Hi Nazir,

The overall direction looks good to me. A few points:

> This patch implements async fsyncs for crash recovery and checkpoints by
> using the AIO system.

For crash recovery, Linux already has `recovery_init_sync_method=syncfs` as an
alternative to per-file fsyncs. I think it would be useful to include that in
the `SyncDataDirectory()` benchmark.

> I will change the reopen interface so that ordinary failures return
> `-errno`.

Agreed. I think all AIO target reopen callbacks should follow this convention.
Relation fsyncs can hit the same problem through `smgr_aio_reopen()`, not only
SLRUs.

> It uses both io_max_concurrency and the file descriptor budget to determine
> max number of available in-flight fsync I/Os.

I wonder whether submitting a batch of concurrent fsyncs could cause an I/O
storm and hurt foreground I/O latency.

The `max_safe_fds / 6` cap protects the `AllocateDesc` budget used by
`OpenTransientFile()`, while relation fsyncs use VFDs. Should the descriptor
cap apply only to handlers that use `OpenTransientFile()`?

Best regards,
Yuhang Qiu



Reply via email to