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
>
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
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
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:
> > > > > + {
> > > > > +
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,
>> > > >
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
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
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
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
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
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
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
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)
> +
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
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
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
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
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
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
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
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
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
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
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.
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
> > >
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
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
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
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
> 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
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
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,
>
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
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
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
> >
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
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
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
48 matches
Mail list logo
