Re: AIO v2.5

Hi,

Attached v2.12, with the following changes:

- Pushed the max_files_per_process change

I plan to look at what parts of Jelte's change is worth doing ontop.

Thanks for the review Noah.

- Rebased over Thomas' commit of the remaining read stream changes

Yay!

- Addressed Noah's review comments

- Added another test to test_aio/, to test that changing io_workers while
running works, and that workers are restarted if terminated

Written by Bilal

- Made InvalidateLocalBuffer wait for IO if necessary

As reported / suggested by Noah

- Added tests for dropping tables with ongoing IO

This failed, as Noah predicted, without the InvalidateLocalBuffer() change.

- Added a commit to explicitly hold interrupts in workers after
pgaio_io_reopen()

As suggested by Noah.

- Added a commit to fix a logic error around what gets passed to
ioh->report_return - this lead to temporary buffer validation errors not
being reported

Discovered while extending the tests, as noted in the next point.

I could see a few different "formulations" of this change (e.g. the
report_return stuff could be populated by pgaio_io_call_complete_local()
instead), but I don't think it matters much.

- Add temporary table coverage to test_aio

This required hanged test_aio.c to cope with temporary tables as well.

- io_uring tests don't run anymore when built with EXEC_BACKEND and liburing
enabled

- Split the read stream patch into two

Noah, quite rightly, pointed out that it's not safe to use batching if the
next-block callback may block (or start its own batch). The best idea seems
to be to make users of read stream opt-in to batching. I've done that in a
patch that uses where it seems safe without doing extra work. See also the
commit message.

- Added a commit to add I/O, Asynchronous I/O glossary and acronym entries

- Docs for pg_aios

- Renamed pg_aios.offset to off, to avoid use of a keyword

- Updated the io_uring wait event name while waiting for IOs to complete to
AIO_IO_URING_COMPLETION and updated the description of AIO_IO_COMPLETION to
"Waiting for another process to complete IO."

I think this is a mix of different suggestions by Noah.

TODO:

- There are more tests in test_aio that should be expanded to run for temp
tables as well, not just normal tables

- Add an explicit test for the checksum verification in the completion callback

There is an existing test for testing an invalid page due to page header
verification in test_aio, but not for checksum failures.

I think it's indirectly covered (e.g. in amcheck), but seems better to test
it explicitly.

Wonder if it's worth adding some coverage for when checksums are disabled?
Probably not necessary?

Greetings,

Andres Freund