Hi,
Thank you for looking into this!
On Thu, 3 Sept 2026 at 16:15, Yuhang Qiu <iamqyh@gmail.com> wrote:
>
> 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.
Good point. I tested recovery_init_sync_method=syncfs and it seems it
is better than or on par with fsync and AIO. Here are two different
benchmark results:
io_method | recovery_init_sync_method | startup (ms) | speedup
-----------+---------------------------+----------------------+----------------
sync | fsync | 609.4 | 1.00x
worker | fsync | 610.8 | 1.00x
io_uring | fsync | 308.6 | 1.97x
sync | syncfs | 210.7 | 2.89x
io_method | recovery_init_sync_method | startup (ms) | speedup
-----------+---------------------------+----------------------+----------------
sync | fsync | 609.2 | 1.00x
worker | fsync | 610.0 | 1.00x
io_uring | fsync | 211.0 | 2.89x
sync | syncfs | 209.0 | 2.91x
> > 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.
I played with that a bit but there were some problems; it was more
complicated than I thought. If we want to return -errno, we lose quite
a bit of error information because we don't reach the actual error
message. We short-circuit these error messages to return -errno and
continue without terminating the worker process.
For now, ordinary reopen failures for both relation and SLRU fsyncs
return -errno. This allows the checkpointer to apply its cancellation,
retry, and error-handling policy without terminating the I/O worker.
Read and write IO reopening retains its existing error handling. I
will continue working to improve this.
> > 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.
I think this is a valid concern. With io_method=worker, fsyncs also
share the worker pool with reads so that could cause a performance
problems. However, I think increasing the number of worker processes
should solve this problem.
> 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()`?
Done. Relation fsync requests now use io_max_concurrency as their
limit. For handlers that hold transient descriptors in the
issuingprocess, the limit is also capped at max_safe_fds / 6.
Here are some additional updates compared to v1:
- 0001 enables fsync on tests so CI will run tests with fsync enabled.
- 0002 clears SMGR caches for the worker processes. These caches are
created on the smgr_reopen path and not get destroyed later so these
remain for the lifetime of worker processes. Patch clears the SMGR
cache after a checkpoint and when the worker is not busy. I created
another thread for this [1].
- 0005 for reopen error changes.
- 0008 is for doc changes.
I am still working on the tests; no tests are included yet.
[1] https://postgr.es/m/CAN55FZ2BesKUnajdgpw1fPSe3S6_CHOugryaUEtD7vdP%3DdRKEQ%40mail.gmail.com
--
Regards,
Nazir Bilal Yavuz
Microsoft
