Re: AIO v2.2

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@postgresql.org
Date: 2025-01-09T00:26:36Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. aio: Fix assertion, clarify README

  2. aio: Fix reference to outdated name

  3. aio: Fix possible state confusions due to interrupt processing

  4. aio: Improve debug logging around waiting for IOs

  5. aio: Fix crash potential for pg_aios views due to late state update

  6. Increase BAS_BULKREAD based on effective_io_concurrency

  7. localbuf: Add Valgrind buffer access instrumentation

  8. aio: Make AIO more compatible with valgrind

  9. aio: Avoid spurious coverity warning

  10. tests: Fix incompatibility of test_aio with *_FORCE_RELEASE

  11. tests: Cope with WARNINGs during failed CREATE DB on windows

  12. aio: Add errcontext for processing I/Os for another backend

  13. aio: Add README.md explaining higher level design

  14. aio: Minor comment improvements

  15. aio: Add test_aio module

  16. aio: Add pg_aios view

  17. docs: Add acronym and glossary entries for I/O and AIO

  18. Enable IO concurrency on all systems

  19. read_stream: Introduce and use optional batchmode support

  20. docs: Reframe track_io_timing related docs as wait time

  21. bufmgr: Use AIO in StartReadBuffers()

  22. bufmgr: Implement AIO read support

  23. aio: Add WARNING result status

  24. Let caller of PageIsVerified() control ignore_checksum_failure

  25. pgstat: Allow checksum errors to be reported in critical sections

  26. Add errhint_internal()

  27. localbuf: Track pincount in BufferDesc as well

  28. aio, bufmgr: Comment fixes/improvements

  29. Fix mis-attribution of checksum failure stats to the wrong database

  30. aio: Implement support for reads in smgr/md/fd

  31. aio: Add io_method=io_uring

  32. aio: Add liburing dependency

  33. aio: Rename pgaio_io_prep_* to pgaio_io_start_*

  34. aio: Pass result of local callbacks to ->report_return

  35. aio: Be more paranoid about interrupts

  36. Redefine max_files_per_process to control additionally opened files

  37. aio: Change prefix of PgAioResultStatus values to PGAIO_RS_

  38. bufmgr: Improve stats when a buffer is read in concurrently

  39. aio: Add io_method=worker

  40. aio: Infrastructure for io_method=worker

  41. aio: Add core asynchronous I/O infrastructure

  42. aio: Basic subsystem initialization

  43. tests: Expand temp table tests to some pin related matters

  44. localbuf: Introduce FlushLocalBuffer()

  45. localbuf: Introduce TerminateLocalBufferIO()

  46. localbuf: Fix dangerous coding pattern in GetLocalVictimBuffer()

  47. localbuf: Introduce StartLocalBufferIO()

  48. localbuf: Introduce InvalidateLocalBuffer()

  49. Allow lwlocks to be disowned

  50. Make jsonb casts to scalar types translate JSON null to SQL NULL.

  51. bufmgr/smgr: Don't cross segment boundaries in StartReadBuffers()

  52. Use aux process resource owner in walsender

  53. bufmgr: Return early in ScheduleBufferTagForWriteback() if fsync=off

Hi,

On 2025-01-07 14:59:58 -0500, Robert Haas wrote:
> On Tue, Jan 7, 2025 at 11:11 AM Andres Freund <andres@anarazel.de> wrote:
> > The difference between a handle and a reference is useful right now, to have
> > some separation between the functions that can be called by anyone (taking a
> > PgAioHandleRef) and only by the issuer (PgAioHandle). That might better be
> > solved by having a PgAioHandleIssuerRef ref or something.
>
> To me, those names don't convey that.

I'm certainly not wedded to these names - I went back and forth between
different names a fair bit, because I wasn't quite happy. I am however certain
that the current names are better than what it used to be (PgAioInProgress and
because that's long, a bunch of PgAioIP* names) :)


To make sure were talking about the same things, I am thinking of the
following "entities" needing names:


1) Shared memory representation of an IO, for the AIO subsystem internally

   Currently: PgAioHandle

   Because shared memory is limited, we need to reuse this entity. This reuse
   needs to be possible "immediately" after completion, to avoid a bunch of
   nasty scenarios.

   To distinguish a reused PgAioHandle from its "prior" incarnation, each
   PgAioHandle has a 64bit "generation counter.

   In addition to being referenceable via pointer, it's also possible to
   assign a 32bit integer to each PgAioHandle, as there is a fixed number of
   them.


2) A way for the issuer of an IO to reference 1), to attach information to the
   IO

   Currently: PgAioHandle*

   As long as the issuer hasn't yet staged the IO, it can't be
   reused. Therefore it's OK to just point to the PgAioHandle.

   One disadvantage of just using a pointer to PgAioHandle* is that it's
   harder to distinguish subystem-internal functions that accept PgAioHandle*
   from "public" functions that accept the "issuer reference".


3) A way for any backend to wait for a specific IO to complete

   Currently: PgAioHandleRef

   This references 1) using a 32 bit ID and the 64bit generation.

   This is used to allow any backend to wait for a specific IO to
   complete. E.g. by including it in the BufferDesc so that WaitIO can wait
   for it.

   Because it includes the generation it's trivial to detect whether the
   PgAioHandle was reused.



> I would perhaps call the thing that supports issuer-only operations a
> "PgAio" and the thing other people can use a "PgAioHandle". Or
> "PgAioRequest" and "PgAioHandle" or something like that. With
> PgAioHandleRef, IMHO you've got two words that both imply a layer of
> indirection -- "handle" and "ref" -- which doesn't seem quite as nice,
> because then the other thing -- "PgAioHandle" still sort of implies one
> layer of indirection and the whole thing seems a bit less clear.

It's indirections all the way down. The PG representation of "one IO" in the
end is just an indirection for a kernel operation :)


I would like to split 1) and 2) above.

1) PgAio{Handle,Request,} (a large struct)  - used internally by AIO subsystem,
   "pointed to" by the following
2) PgAioIssuerRef (an ID or pointer) - used by the issuer to incrementally
   define the IO
3) PgAioWaitRef - (an ID and generation) - used to wait for a specific IO to
   complete, not affected by reuse of PgAioHandle





> > > REAPED feels like a bad name. It sounds like a later stage than COMPLETED,
> > > but it's actually vice versa.
> >
> > What would you call having gotten "completion notifications" from the kernel,
> > but not having processed them?
>
> The Linux kernel calls those zombie processes, so we could call it a ZOMBIE
> state, but that seems like it might be a bit of inside baseball.

ZOMBIE feels even later than REAPED to me :)


> I do agree with Heikki that REAPED sounds later than COMPLETED, because you
> reap zombie processes by collecting their exit status. Maybe you could have
> AHS_COMPLETE or AHS_IO_COMPLETE for the state where the I/O is done but
> there's still completion-related work to be done, and then the other state
> could be AHS_DONE or AHS_FINISHED or AHS_FINAL or AHS_REAPED or something.

How about

AHS_COMPLETE_KERNEL or AHS_COMPLETE_RAW - raw syscall completed
AHS_COMPLETE_SHARED_CB - shared callback completed
AHS_COMPLETE_LOCAL_CB - local callback completed

?

Greetings,

Andres Freund