Re: should frontend tools use 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_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