Re: Cygwin cleanup

Hi,

On 2023-01-11 22:39:49 -0600, Justin Pryzby wrote:
> Thomas raised a good question, which was how the tests were passing when
> SnapBuildSerialize() was raising an error, which is what it would've
> been doing when I used data_sync_retry=no.

Presumably some test not checking for failures in a part of the test.

> So .. why is wal_sync_method being used to control fsync for things
> other than WAL?

Historical raisins, I think. The problem is that macOS lies about fsync, and
one needs special magic to make it behave like a real fsync. Somebody thought
that instead of inventing a separate GUC to control whether the "real" fsync
is used for other subsystems, it'd be better to reuse wal_sync_method.

Note that this isn't the function that is actually used for WAL (that's
issue_xlog_fsync()), and that pg_fsync() only uses the GUC to know whether to
use pg_fsync_writethrough() or pg_fsync_no_writethrough(fd).

> See 6dc7760ac (c. 2005) which added wal_fsync_writethrough, at which
> point (since 9b178555f, c. 2004) wal_sync_method was already being used
> for SLOG.
>
> Now, it's also being used for logical decoding (since b89e1510 and
> 858ec1185, c. 2014) in rewriteheap.c/snapbuild.c. And pidfiles (since
> ee0e525bf, 2010). And the control file (8b938d36f7, 2019). Note that
> data_sync_retry wasn't added until 9ccdd7f66 (c. 2018)
>
> It looks like logical decoding may be the "most wrong" place that
> wal_sync_method is being used, so maybe my change is reasonable to
> consider, and not just a workaround.

I don't follow. What does using fsync_fname() vs fsync_fname_ext() have to do
with pg_fsync() using wal_sync_method? fsync_fname() is just a wrapper around
fsync_fname_ext(). Both end up in pg_fsync().

Are you actually proposing that we don't PANIC after an fsync for the category
of files that you list here, even with data_sync_retry set?

Greetings,

Andres Freund