From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, David Steele <david(at)pgmasters(dot)net>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Paul Guo <guopa(at)vmware(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Brown <michael(dot)brown(at)discourse(dot)org>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: should frontend tools use syncfs() ? |
Date: | 2023-09-01 18:08:51 |
Message-ID: | 20230901180851.GC3180181@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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_method(const char *optarg, SyncMethod *sync_method)
>> +{
>> + if (strcmp(optarg, "fsync") == 0)
>> + *sync_method = SYNC_METHOD_FSYNC;
>> +#ifdef HAVE_SYNCFS
>> + else if (strcmp(optarg, "syncfs") == 0)
>> + *sync_method = SYNC_METHOD_SYNCFS;
>> +#endif
>> + else
>> + {
>> + pg_log_error("unrecognized sync method: %s", optarg);
>> + return false;
>> + }
>
> 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.
> The patch should handle pg_dumpall, too.
It looks like pg_dumpall only ever fsyncs a single file, so I don't think
it is really needed there.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2023-09-01 18:19:13 | Re: should frontend tools use syncfs() ? |
Previous Message | Justin Pryzby | 2023-09-01 17:58:10 | Re: should frontend tools use syncfs() ? |