On Wed, Sep 29, 2021 at 07:43:41PM -0500, Justin Pryzby wrote:
> Forking this thread in which Thomas implemented syncfs for the startup process
> (61752afb2).
> https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BSG9jSW3ekwib0cSdC0yD-jReJ21X4bZAmqxoWTLTc2A%40mail.gmail.com
>
> Is there any reas
On Thu, Sep 30, 2021 at 4:49 PM Michael Paquier wrote:
> fsync_pgdata() is going to manipulate many inodes anyway, because
> that's a code path designed to do so. If we know that syncfs() is
> just going to be better, I'd rather just call it by default if
> available and not add new switches to a
On Thu, Sep 30, 2021 at 05:08:24PM +1300, Thomas Munro wrote:
> If we want this it should be an option, because it flushes out data
> other than the pgdata dir, and it doesn't report errors on old
> kernels.
Oh, OK, thanks. That's the part about 5.8. The only option
controlling if sync is used n
On Thu, Sep 30, 2021 at 05:08:24PM +1300, Thomas Munro wrote:
> On Thu, Sep 30, 2021 at 4:49 PM Michael Paquier wrote:
> > fsync_pgdata() is going to manipulate many inodes anyway, because
> > that's a code path designed to do so. If we know that syncfs() is
> > just going to be better, I'd rathe
On Thu, Sep 30, 2021 at 12:49:36PM +0900, Michael Paquier wrote:
> On Wed, Sep 29, 2021 at 07:43:41PM -0500, Justin Pryzby wrote:
> > Forking this thread in which Thomas implemented syncfs for the startup
> > process
> > (61752afb2).
> > https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BSG9jS
On Fri, 6 Oct 2023 at 22:35, Nathan Bossart
wrote:
> From a quick skim, this one looks pretty good to me. Would you mind adding
> it to the commitfest so that it doesn't get lost? I will aim to take a
> closer look at it next week.
>
Sounds good, thanks a lot!
https://commitfest.postgresql.or
On Mon, Oct 09, 2023 at 01:12:16PM +0300, Maxim Orlov wrote:
> On Fri, 6 Oct 2023 at 22:35, Nathan Bossart
> wrote:
>> From a quick skim, this one looks pretty good to me. Would you mind adding
>> it to the commitfest so that it doesn't get lost? I will aim to take a
>> closer look at it next we
On Mon, Oct 09, 2023 at 11:14:39AM -0500, Nathan Bossart wrote:
> Thanks. I've made a couple of small changes, but otherwise I think this
> one is just about ready.
I forgot to rename one thing. Here's a v13 with that fixed.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From b
On Wed, Oct 04, 2023 at 10:29:07AM -0500, Nathan Bossart wrote:
> On Wed, Sep 27, 2023 at 01:56:08PM +0100, Peter Eisentraut wrote:
>> We have a collection of platform-specific notes in chapter 19, including
>> file-system-related notes in section 19.2. Maybe it could be put there?
>
> I will giv
On Mon, Oct 09, 2023 at 02:34:27PM -0500, Nathan Bossart wrote:
> On Mon, Oct 09, 2023 at 11:14:39AM -0500, Nathan Bossart wrote:
>> Thanks. I've made a couple of small changes, but otherwise I think this
>> one is just about ready.
>
> I forgot to rename one thing. Here's a v13 with that fixed.
On Thu, Aug 17, 2023 at 12:50:31PM +0900, Michael Paquier wrote:
> On Wed, Aug 16, 2023 at 08:17:05AM -0700, Nathan Bossart wrote:
>> The patch does have the following note:
>>
>> +On Linux, syncfs may be used instead to ask the
>> +operating system to synchronize the whole file sy
On Wed, Aug 16, 2023 at 11:50 PM Michael Paquier wrote:
> >> Do we actually need --no-sync at all if --sync-method is around? We
> >> could have an extra --sync-method=none at option level with --no-sync
> >> still around mainly for compatibility? Or perhaps that's just
> >> over-designing thing
On Mon, Aug 21, 2023 at 04:08:46PM -0400, Robert Haas wrote:
> Doesn't seem worth it to me. I think --no-sync is more intuitive than
> --sync-method=none, it's certainly shorter, and it's a pretty
> important setting because we use it when running the regression tests.
No arguments against that ;)
On Fri, Aug 18, 2023 at 09:01:11AM -0700, Nathan Bossart wrote:
> On Thu, Aug 17, 2023 at 12:50:31PM +0900, Michael Paquier wrote:
>> SyncMethod may be a bit too generic as name for the option structure.
>> How about a PGSyncMethod or pg_sync_method?
>
> In v4, I renamed this to DataDirSyncMethod
On Tue, Aug 22, 2023 at 08:56:26AM +0900, Michael Paquier wrote:
> --- a/src/include/storage/fd.h
> +++ b/src/include/storage/fd.h
> @@ -43,15 +43,11 @@
> #ifndef FD_H
> #define FD_H
>
> +#ifndef FRONTEND
> +
> #include
> #include
>
> Ugh. So you need this part because pg_rewind's filema
On Mon, Aug 21, 2023 at 06:44:07PM -0700, Nathan Bossart wrote:
> I'm hoping there's a simpler path forward here. pg_rewind only needs the
> following lines from fd.h:
>
> /* Filename components */
> #define PG_TEMP_FILES_DIR "pgsql_tmp"
> #define PG_TEMP_FILE_PREFIX "pgsql_tmp"
On Tue, Aug 22, 2023 at 10:50:01AM +0900, Michael Paquier wrote:
> On Mon, Aug 21, 2023 at 06:44:07PM -0700, Nathan Bossart wrote:
>> I'm hoping there's a simpler path forward here. pg_rewind only needs the
>> following lines from fd.h:
>>
>> /* Filename components */
>> #define PG_TEMP
On Mon, Aug 21, 2023 at 07:06:32PM -0700, Nathan Bossart wrote:
> This would look something like the attached patch. I think this is nicer.
> With this patch, we don't have to choose between including fd.h or
> redefining the macros in the frontend code.
Yes, this one is moving the needle in the
On Tue, Aug 22, 2023 at 12:53:53PM +0900, Michael Paquier wrote:
> On Mon, Aug 21, 2023 at 07:06:32PM -0700, Nathan Bossart wrote:
>> This would look something like the attached patch. I think this is nicer.
>> With this patch, we don't have to choose between including fd.h or
>> redefining the ma
rebased
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 1bb28cfe5d40670386ae663e14c8854dc1b5486d Mon Sep 17 00:00:00 2001
From: Nathan Bossart
Date: Mon, 21 Aug 2023 19:05:14 -0700
Subject: [PATCH v6 1/2] move PG_TEMP_FILE* macros to file_utils.h
---
src/backend/backup/base
On Tue, Aug 29, 2023 at 08:45:59AM -0700, Nathan Bossart wrote:
> rebased
0001 looks OK, worth its own, independent, commit.
I understand that I'm perhaps sounding pedantic about fsync_pgdata()..
But, after thinking more about it, I would still make this code fail
hard with an exit(EXIT_FAILURE)
On Wed, Aug 30, 2023 at 09:10:47AM +0900, Michael Paquier wrote:
> I understand that I'm perhaps sounding pedantic about fsync_pgdata()..
> But, after thinking more about it, I would still make this code fail
> hard with an exit(EXIT_FAILURE) to let any C code calling directly
> this routine with s
On Tue, Aug 29, 2023 at 06:14:08PM -0700, Nathan Bossart wrote:
> That seems fair enough. I did this in v7. I restructured fsync_pgdata()
> and fsync_dir_recurse() so that any new sync methods should cause compiler
> warnings until they are implemented.
That's pretty cool and easier to maintain
On Thu, Aug 31, 2023 at 02:30:33PM +0900, Michael Paquier wrote:
> - Should we have some regression tests? We should only need one test
> in one of the binaries to be able to stress the new code paths of
> file_utils.c with syncfs. The cheapest one may be pg_dump with a
> dump in directory forma
On Thu, Aug 31, 2023 at 08:48:58AM -0700, Nathan Bossart wrote:
> On Thu, Aug 31, 2023 at 02:30:33PM +0900, Michael Paquier wrote:
> > - Should we have some regression tests? We should only need one test
> > in one of the binaries to be able to stress the new code paths of
> > file_utils.c with sy
On Fri, Sep 01, 2023 at 10:40:12AM +0900, Michael Paquier wrote:
> That should be OK this way. The extra running time is not really
> visible, right?
AFAICT it is negligible. Presumably it could take a little longer if there
is a lot to sync on the file system, but I don't know if that's worth
w
> + if (!user_opts.sync_method)
> + user_opts.sync_method = pg_strdup("fsync");
why pstrdup?
> +parse_sync_method(const char *optarg, SyncMethod *sync_method)
> +{
> + if (strcmp(optarg, "fsync") == 0)
> + *sync_method = SYNC_METHOD_FSYNC;
> +#ifdef HAVE_SYNCFS
> +
Thanks for taking a look.
On Fri, Sep 01, 2023 at 12:58:10PM -0500, Justin Pryzby wrote:
>> +if (!user_opts.sync_method)
>> +user_opts.sync_method = pg_strdup("fsync");
>
> why pstrdup?
I believe I was just following the precedent set by some of the other
options.
>> +parse_sync
On Fri, Sep 01, 2023 at 11:08:51AM -0700, Nathan Bossart wrote:
> > This should probably give a distinct error when syncfs is not supported
> > than when it's truely recognized.
>
> Later versions of the patch should have this.
Oops, right.
> > The patch should handle pg_dumpall, too.
>
> It lo
On Fri, Sep 01, 2023 at 01:19:13PM -0500, Justin Pryzby wrote:
> What about (per git grep no-sync doc) pg_receivewal?
I don't think it's applicable there, either. IIUC that option specifies
whether to sync the data as it is streamed over.
--
Nathan Bossart
Amazon Web Services: https://aws.amazo
I've committed 0001 for now. I plan to commit the rest in the next couple
of days.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Tue, Sep 05, 2023 at 05:08:53PM -0700, Nathan Bossart wrote:
> I've committed 0001 for now. I plan to commit the rest in the next couple
> of days.
Committed.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Thu, 7 Sept 2023 at 03:34, Nathan Bossart
wrote:
> Committed.
>
Hi! Great job!
But here is one problem I've encountered during working on some unrelated
stuff.
How we have two different things call the same name – sync_method. One in
xlog:
intsync_method = DEFAULT_SYNC_METHOD;
..
On Wed, Sep 20, 2023 at 03:12:56PM +0300, Maxim Orlov wrote:
> As a solution, I suggest renaming sync_method in xlog module to
> wal_sync_method. In fact,
> appropriate GUC for this variable, called "wal_sync_method" and I see no
> reason not to use
> the exact same name for a variable in xlog modu
On Wed, 20 Sept 2023 at 22:08, Nathan Bossart
wrote:
> I think we should also consider renaming things like SYNC_METHOD_FSYNC to
> WAL_SYNC_METHOD_FSYNC, and sync_method_options to wal_sync_method_options.
>
I've already rename sync_method_options in previous patch.
34 @@ -171,7 +171,7 @@ stati
On 17.08.23 04:50, Michael Paquier wrote:
Yeah, this crossed my mind. Do you know of any existing examples of
options with links to a common section? One problem with this approach is
that there are small differences in the wording for some of the frontend
utilities, so it might be difficult to
On Wed, Sep 27, 2023 at 01:56:08PM +0100, Peter Eisentraut wrote:
> I think it's a bit much to add a whole appendix for that little content.
I'm inclined to agree.
> We have a collection of platform-specific notes in chapter 19, including
> file-system-related notes in section 19.2. Maybe it cou
Back to the patch v11. I don’t understand a bit, what we should do next?
Make a separate thread or put this one on commitfest?
--
Best regards,
Maxim Orlov.
On Fri, Oct 06, 2023 at 10:50:11AM +0300, Maxim Orlov wrote:
> Back to the patch v11. I don’t understand a bit, what we should do next?
> Make a separate thread or put this one on commitfest?
>From a quick skim, this one looks pretty good to me. Would you mind adding
it to the commitfest so that
On Wed, Apr 13, 2022 at 06:54:12AM -0500, Justin Pryzby wrote:
> I didn't pursue this patch, as it's easier for me to use /bin/sync -f.
> Someone
> should adopt it if interested.
I was about to start a new thread, but I found this one with some good
preliminary discussion. I came to the same co
On Sat, Jul 29, 2023 at 02:40:10PM -0700, Nathan Bossart wrote:
> I was about to start a new thread, but I found this one with some good
> preliminary discussion. I came to the same conclusion about introducing a
> new option instead of using syncfs() by default wherever it is available.
> The att
On Mon, Jul 31, 2023 at 10:51:38AM -0700, Nathan Bossart wrote:
> Here is a new version of the patch with documentation updates and a couple
> other small improvements.
I just realized I forgot to update the --help output for these utilities.
I'll do that in the next version of the patch.
--
Nat
On Mon, Jul 31, 2023 at 11:39:46AM -0700, Nathan Bossart wrote:
> I just realized I forgot to update the --help output for these utilities.
> I'll do that in the next version of the patch.
Done in v3. Sorry for the noise.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From be52e
I ran a couple of tests for pg_upgrade with 100k tables (created using the
script here [0]) in order to demonstrate the potential benefits of this
patch.
pg_upgrade --sync-method fsync
real5m50.072s
user0m10.606s
sys 0m40.298s
pg_upgrade --sync-method syncfs
real3m
On Tue, Aug 08, 2023 at 01:06:06PM -0700, Nathan Bossart wrote:
> I ran a couple of tests for pg_upgrade with 100k tables (created using the
> script here [0]) in order to demonstrate the potential benefits of this
> patch.
That shows some nice numbers with many files, indeed. How does the
size o
Thanks for taking a look.
On Wed, Aug 16, 2023 at 08:10:10AM +0900, Michael Paquier wrote:
> On Tue, Aug 08, 2023 at 01:06:06PM -0700, Nathan Bossart wrote:
>> I ran a couple of tests for pg_upgrade with 100k tables (created using the
>> script here [0]) in order to demonstrate the potential benef
On Wed, Aug 16, 2023 at 08:17:05AM -0700, Nathan Bossart wrote:
> On Wed, Aug 16, 2023 at 08:10:10AM +0900, Michael Paquier wrote:
>> + pg_log_error("could not synchronize file system for file \"%s\":
>> %m", path);
>> + (void) close(fd);
>> + exit(EXIT_FAILURE);
>>
>> walkdir()
On Wed, Aug 16, 2023 at 08:23:25AM -0700, Nathan Bossart wrote:
> Ah, it looks like this code used to treat fsync() errors as non-fatal, but
> it was changed in commit 1420617. I still find it a bit strange that some
> errors that prevent a file from being sync'd are non-fatal while others
> _are_
On Wed, Aug 16, 2023 at 08:17:05AM -0700, Nathan Bossart wrote:
> On Wed, Aug 16, 2023 at 08:10:10AM +0900, Michael Paquier wrote:
>> On Tue, Aug 08, 2023 at 01:06:06PM -0700, Nathan Bossart wrote:
>> +else
>> +{
>> +while (errno = 0, (de = readdir(dir)) != NULL)
>> +
49 matches
Mail list logo
