On Tue, Jun 22, 2021 at 5:01 PM Thomas Munro wrote:
> On Fri, Jun 18, 2021 at 1:11 PM Justin Pryzby wrote:
> > Thomas, could you comment on this ?
>
> Sorry, I missed that. It is initially a confusing proposal, but after
> trying it out (that is: making recovery_init_sync_method PGC_SIGHUP
> and
On Fri, Jun 18, 2021 at 1:11 PM Justin Pryzby wrote:
> Thomas, could you comment on this ?
Sorry, I missed that. It is initially a confusing proposal, but after
trying it out (that is: making recovery_init_sync_method PGC_SIGHUP
and testing a scenario where you want to make the next crash use it
On Fri, Jun 18, 2021 at 06:18:59PM +0900, Fujii Masao wrote:
> On 2021/06/04 23:39, Justin Pryzby wrote:
>> You said switching to SIGHUP "would have zero effect"; but, actually it
>> allows
>> an admin who's DB took a long time in recovery/startup to change the
>> parameter
>> without shutting do
On 2021/06/04 23:39, Justin Pryzby wrote:
You said switching to SIGHUP "would have zero effect"; but, actually it allows
an admin who's DB took a long time in recovery/startup to change the parameter
without shutting down the service. This mitigates the downtime if it crashes
again. I think
Thomas, could you comment on this ?
On Sat, May 29, 2021 at 02:23:21PM -0500, Justin Pryzby wrote:
> On Tue, May 25, 2021 at 07:13:59PM -0500, Justin Pryzby wrote:
> > On Sat, Mar 20, 2021 at 12:16:27PM +1300, Thomas Munro wrote:
> > > > > + {
> > > > > + {"recovery_init_sync_metho
On Fri, Jun 04, 2021 at 04:24:02PM +0900, Michael Paquier wrote:
> On Sat, May 29, 2021 at 02:23:21PM -0500, Justin Pryzby wrote:
> > On Tue, May 25, 2021 at 07:13:59PM -0500, Justin Pryzby wrote:
> >> On Sat, Mar 20, 2021 at 12:16:27PM +1300, Thomas Munro wrote:
> >> > > > + {
> >> > > > +
On Sat, May 29, 2021 at 02:23:21PM -0500, Justin Pryzby wrote:
> On Tue, May 25, 2021 at 07:13:59PM -0500, Justin Pryzby wrote:
>> On Sat, Mar 20, 2021 at 12:16:27PM +1300, Thomas Munro wrote:
>> > > > + {
>> > > > + {"recovery_init_sync_method", PGC_POSTMASTER,
>> > > > ERROR_HAND
On Tue, May 25, 2021 at 07:13:59PM -0500, Justin Pryzby wrote:
> On Sat, Mar 20, 2021 at 12:16:27PM +1300, Thomas Munro wrote:
> > > > + {
> > > > + {"recovery_init_sync_method", PGC_POSTMASTER,
> > > > ERROR_HANDLING_OPTIONS,
> > > > + gettext_noop("Sets the me
On Tue, May 25, 2021 at 07:13:59PM -0500, Justin Pryzby wrote:
> This one isn't documented as requiring a restart:
> max_logical_replication_workers.
There is much more than meets the eye here, and this is unrelated to
this thread, so let's discuss that on a separate thread. I'll start a
new one
On Sat, Mar 20, 2021 at 12:16:27PM +1300, Thomas Munro wrote:
> > > + {
> > > + {"recovery_init_sync_method", PGC_POSTMASTER,
> > > ERROR_HANDLING_OPTIONS,
> > > + gettext_noop("Sets the method for synchronizing the
> > > data directory before crash recovery.")
On Wed, 10 Mar 2021 at 20:25, Tom Lane wrote:
>
> So this means that in less-than-bleeding-edge kernels, syncfs can
> only be regarded as a dangerous toy. If we expose an option to use
> it, there had better be large blinking warnings in the docs.
Isn't that true for fsync and everything else re
On 3/19/21 7:16 PM, Thomas Munro wrote:
Thanks Justin and David. Replies to two emails inline:
Fair point. Here's what I went with:
When set to fsync, which is the default,
PostgreSQL will recursively open and
synchronize all files in the data directory before crash
Thanks Justin and David. Replies to two emails inline:
On Sat, Mar 20, 2021 at 12:01 AM Justin Pryzby wrote:
> On Fri, Mar 19, 2021 at 06:28:46PM +1300, Thomas Munro wrote:
> > +++ b/doc/src/sgml/config.sgml
>
> > +PostgreSQL will recursively open and
> > fsync
> > +all files in
On 3/19/21 1:28 AM, Thomas Munro wrote:
On Fri, Mar 19, 2021 at 3:23 PM Fujii Masao wrote:
Thanks for updating the patch! It looks good to me!
I have one minor comment for the patch.
+ elog(LOG, "could not open %s: %m", path);
+ return;
+ }
+ if (syncfs(
On Fri, Mar 19, 2021 at 06:28:46PM +1300, Thomas Munro wrote:
> +++ b/doc/src/sgml/config.sgml
> +PostgreSQL will recursively open and fsync
> +all files in the data directory before crash recovery begins. This
Maybe it should say "data, tablespace, and WAL directories", or just
On 2021/03/19 14:28, Thomas Munro wrote:
On Fri, Mar 19, 2021 at 3:23 PM Fujii Masao wrote:
Thanks for updating the patch! It looks good to me!
I have one minor comment for the patch.
+ elog(LOG, "could not open %s: %m", path);
+ return;
+ }
+ if (syn
On Fri, Mar 19, 2021 at 3:23 PM Fujii Masao wrote:
> Thanks for updating the patch! It looks good to me!
> I have one minor comment for the patch.
>
> + elog(LOG, "could not open %s: %m", path);
> + return;
> + }
> + if (syncfs(fd) < 0)
> + elo
On 2021/03/19 11:22, Fujii Masao wrote:
On 2021/03/19 10:37, Thomas Munro wrote:
On Fri, Mar 19, 2021 at 2:16 PM Thomas Munro wrote:
PS: For illustration/discussion, I've also attached a "none" patch. I
also couldn't resist rebasing my "wal" mode patch, which I plan to
propose for PG15 b
On 2021/03/19 10:37, Thomas Munro wrote:
On Fri, Mar 19, 2021 at 2:16 PM Thomas Munro wrote:
PS: For illustration/discussion, I've also attached a "none" patch. I
also couldn't resist rebasing my "wal" mode patch, which I plan to
propose for PG15 because there is not enough time left for th
On Fri, Mar 19, 2021 at 2:16 PM Thomas Munro wrote:
> PS: For illustration/discussion, I've also attached a "none" patch. I
> also couldn't resist rebasing my "wal" mode patch, which I plan to
> propose for PG15 because there is not enough time left for this
> release.
Erm... I attached the wron
On Fri, Mar 19, 2021 at 5:50 AM Fujii Masao wrote:
> On 2021/03/18 23:03, Bruce Momjian wrote:
> >> Are we sure we want to use the word "crash" here? I don't remember
> >> seeing it used anywhere else in our user interface.
>
> We have GUC restart_after_crash.
>
> > I guess it is
> >> "crash rec
On 2021/03/18 23:03, Bruce Momjian wrote:
Are we sure we want to use the word "crash" here? I don't remember
seeing it used anywhere else in our user interface.
We have GUC restart_after_crash.
I guess it is
"crash recovery".
Maybe call it "recovery_sync_method"
+1. This name sounds
On Thu, Mar 18, 2021 at 09:54:11AM -0400, Bruce Momjian wrote:
> On Thu, Mar 18, 2021 at 11:19:13PM +1300, Thomas Munro wrote:
> > On Thu, Mar 18, 2021 at 8:52 PM Paul Guo wrote:
> > > About the syncfs patch, my first impression on the guc name
> > > sync_after_crash
> > > is that it is a boolean
On Thu, Mar 18, 2021 at 11:19:13PM +1300, Thomas Munro wrote:
> On Thu, Mar 18, 2021 at 8:52 PM Paul Guo wrote:
> > About the syncfs patch, my first impression on the guc name sync_after_crash
> > is that it is a boolean type. Not sure about other people's feeling. Do you
> > guys think
> > It is
On 2021/03/18 19:19, Thomas Munro wrote:
On Thu, Mar 18, 2021 at 8:52 PM Paul Guo wrote:
About the syncfs patch, my first impression on the guc name sync_after_crash
is that it is a boolean type. Not sure about other people's feeling. Do you
guys think
It is better to rename it to a clearer
On Wed, Mar 17, 2021 at 11:42 PM Paul Guo wrote:
> I just quickly reviewed the patch (the code part). It looks good. Only
> one concern
> or question is do_syncfs() for symlink opened fd for syncfs() - I'm
> not 100% sure.
Alright, let me try to prove that it works the way we want with an experim
On Thu, Mar 18, 2021 at 8:52 PM Paul Guo wrote:
> About the syncfs patch, my first impression on the guc name sync_after_crash
> is that it is a boolean type. Not sure about other people's feeling. Do you
> guys think
> It is better to rename it to a clearer name like sync_method_after_crash or
About the syncfs patch, my first impression on the guc name sync_after_crash
is that it is a boolean type. Not sure about other people's feeling. Do you
guys think
It is better to rename it to a clearer name like sync_method_after_crash or
others?
On 2021/03/17 12:45, Thomas Munro wrote:
On Tue, Mar 16, 2021 at 9:29 PM Fujii Masao wrote:
On 2021/03/16 8:15, Thomas Munro wrote:
I don't want to add a hypothetical sync_after_crash=none, because it
seems like generally a bad idea. We already have a
running-with-scissors mode you could u
On 2021/03/17 12:02, Thomas Munro wrote:
On Tue, Mar 16, 2021 at 9:10 PM Fujii Masao wrote:
Thanks for the patch!
+When set to fsync, which is the default,
+PostgreSQL will recursively open and fsync
+all files in the data directory before crash recovery begins.
Isn
On Wed, Mar 17, 2021 at 11:45 AM Thomas Munro wrote:
>
> On Tue, Mar 16, 2021 at 9:29 PM Fujii Masao
> wrote:
> > On 2021/03/16 8:15, Thomas Munro wrote:
> > > I don't want to add a hypothetical sync_after_crash=none, because it
> > > seems like generally a bad idea. We already have a
> > > run
On Tue, Mar 16, 2021 at 9:29 PM Fujii Masao wrote:
> On 2021/03/16 8:15, Thomas Munro wrote:
> > I don't want to add a hypothetical sync_after_crash=none, because it
> > seems like generally a bad idea. We already have a
> > running-with-scissors mode you could use for that: fsync=off.
>
> I hear
On Tue, Mar 16, 2021 at 9:10 PM Fujii Masao wrote:
> Thanks for the patch!
>
> +When set to fsync, which is the default,
> +PostgreSQL will recursively open and fsync
> +all files in the data directory before crash recovery begins.
>
> Isn't this a bit misleading? This may
On Tue, Mar 16, 2021 at 4:29 PM Fujii Masao wrote:
>
>
>
> On 2021/03/16 8:15, Thomas Munro wrote:
> > On Tue, Mar 16, 2021 at 3:30 AM Paul Guo wrote:
> >> By the way, there is a usual case that we could skip fsync: A fsync-ed
> >> already standby generated by pg_rewind/pg_basebackup.
> >> The s
On 2021/03/16 8:15, Thomas Munro wrote:
On Tue, Mar 16, 2021 at 3:30 AM Paul Guo wrote:
By the way, there is a usual case that we could skip fsync: A fsync-ed already
standby generated by pg_rewind/pg_basebackup.
The state of those standbys are surely not
DB_SHUTDOWNED/DB_SHUTDOWNED_IN_REC
On 2021/03/15 8:33, Thomas Munro wrote:
On Mon, Mar 15, 2021 at 11:52 AM Thomas Munro wrote:
Time being of the essence, here is the patch I posted last year, this
time with a GUC and some docs. You can set sync_after_crash to
"fsync" (default) or "syncfs" if you have it.
Cfbot told me to
On Tue, Mar 16, 2021 at 3:30 AM Paul Guo wrote:
> By the way, there is a usual case that we could skip fsync: A fsync-ed
> already standby generated by pg_rewind/pg_basebackup.
> The state of those standbys are surely not
> DB_SHUTDOWNED/DB_SHUTDOWNED_IN_RECOVERY, so the
> pgdata directory is fs
> On 2021/3/15, 7:34 AM, "Thomas Munro" wrote:
>> On Mon, Mar 15, 2021 at 11:52 AM Thomas Munro
wrote:
>> Time being of the essence, here is the patch I posted last year, this
>> time with a GUC and some docs. You can set sync_after_crash to
>> "fsync" (default) or "syncfs"
On Mon, Mar 15, 2021 at 11:52 AM Thomas Munro wrote:
> Time being of the essence, here is the patch I posted last year, this
> time with a GUC and some docs. You can set sync_after_crash to
> "fsync" (default) or "syncfs" if you have it.
Cfbot told me to add HAVE_SYNCFS to Solution.pm, and I fix
On Thu, Mar 11, 2021 at 2:32 PM Thomas Munro wrote:
> On Thu, Mar 11, 2021 at 2:25 PM Tom Lane wrote:
> > Trolling the net, I found a newer-looking version of the man page,
> > and behold it says
> >
> >In mainline kernel versions prior to 5.8, syncfs() will fail only
> >when pass
On Thu, Mar 11, 2021 at 2:25 PM Tom Lane wrote:
> Trolling the net, I found a newer-looking version of the man page,
> and behold it says
>
>In mainline kernel versions prior to 5.8, syncfs() will fail only
>when passed a bad file descriptor (EBADF). Since Linux 5.8,
>sync
Thomas Munro writes:
> On Thu, Mar 11, 2021 at 1:16 PM Tom Lane wrote:
>> I'm a little skeptical about the "simple" part. At minimum, you'd
>> have to syncfs() each tablespace, since we have no easy way to tell
>> which of them are on different filesystems. (Although, if we're
>> presuming this
On Thu, Mar 11, 2021 at 2:00 PM Fujii Masao wrote:
> On 2021/03/11 8:30, Thomas Munro wrote:
> > I've run into a couple of users who have just commented that recursive
> > fsync() code out!
>
> BTW, we can skip that recursive fsync() by disabling fsync GUC even without
> commenting out the code?
On 2021/03/11 8:30, Thomas Munro wrote:
On Thu, Mar 11, 2021 at 11:38 AM Thomas Munro wrote:
On Thu, Mar 11, 2021 at 11:01 AM Michael Brown
wrote:
* is there a knob missing we can configure?
* can we get an opt-in knob to use a single sync() call instead of a
recursive fsync()?
* would you
On Thu, Mar 11, 2021 at 1:16 PM Tom Lane wrote:
> Thomas Munro writes:
> > Thinking about this some more, if you were to propose a patch like
> > that syncfs() one but make it a configurable option, I'd personally be
> > in favour of trying to squeeze it into v14. Others might object on
> > comm
Thomas Munro writes:
> Thinking about this some more, if you were to propose a patch like
> that syncfs() one but make it a configurable option, I'd personally be
> in favour of trying to squeeze it into v14. Others might object on
> commitfest procedural grounds, I dunno, but I think this is a r
On Thu, Mar 11, 2021 at 11:38 AM Thomas Munro wrote:
> On Thu, Mar 11, 2021 at 11:01 AM Michael Brown
> wrote:
> > * is there a knob missing we can configure?
> > * can we get an opt-in knob to use a single sync() call instead of a
> > recursive fsync()?
> > * would you be open to merging a patch
On Thu, Mar 11, 2021 at 11:01 AM Michael Brown
wrote:
> * pg_basebackup receives a streaming backup (via [2] fsync_dir_recurse
> or fsync_pgdata) unless --no-sync is specified
> * postgres starts up unclean (via [3] SyncDataDirectory)
>
> We run multiple postgres clusters and some of those cluster
e. It isn't starved for IO but
does have other regular write workloads
[5]:
https://www.postgresql-archive.org/fdatasync-performance-problem-with-large-number-of-DB-files-td6184094.html
--
Michael Brown
Civilized Discourse Construction Kit, Inc.
https://www.discourse.org/
49 matches
Mail list logo
