Re: AIO v2.5

From: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Antonin Houska <ah(at)cybertec(dot)at>, Noah Misch <noah(at)leadboat(dot)com>, pgsql-hackers(at)postgresql(dot)org, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>
Subject: Re: AIO v2.5
Date: 2025-07-09 11:26:09
Message-ID: CAEze2Wj-43JV4YufW23gm=Uwr7Lkj+p0yKctKHxNm1rwFC+_DQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I've been going through the new AIO code as an effort to rebase and
adapt Neon to PG18. In doing so, I found the following
items/curiosities:

1. In aio/README.md, the following code snippet is found:

[...]
pgaio_io_set_handle_data_32(ioh, (uint32 *) buffer, 1);
[...]

I believe it would be clearer if it took a reference to the buffer:

pgaio_io_set_handle_data_32(ioh, (uint32 *) &buffer, 1);

The main reason here is that common practice is to have a `Buffer
buffer;` whereas a Buffer * is more commonly plural.

2. In aio.h, PgAioHandleCallbackID is checked to fit in
PGAIO_RESULT_ID_BITS (though the value of PGAIO_HCB_MAX).
However, the check is off by 1:

```
#define PGAIO_HCB_MAX PGAIO_HCB_LOCAL_BUFFER_READV
StaticAssertDecl(PGAIO_HCB_MAX <= (1 << PGAIO_RESULT_ID_BITS), [...])
```

This static assert will not trigger when PGAIO_HCB_MAX is equal to
2^PGAIO_RESULT_ID_BITS, but its value won't fit in those 6 bits and
instead overflow to 0.
To fix this, I suggest the `<=` is replaced with `<` to make that
work, or the definition of PGAIO_HCB_MAX to be updated to
PGAIO_HCB_LOCAL_BUFFER_READV + 1.

Note that this won't have caused bugs yet because we're not nearing
this limit of 64 callback IDs, but it's just a matter of good code
hygiene.

3. I noticed that there is AIO code for writev-related operations
(specifically, pgaio_io_start_writev is exposed, as is
PGAIO_OP_WRITEV), but no practical way to excercise that code: it's
not called from anywhere in the project, and there is no way for
extensions to register the relevant callbacks required to make writev
work well on buffered contents. Is that intentional?

Relatedly, the rationale for using enum PgAioHandleCallbackID rather
than function pointers in the documentation above its definition says
that function pointer instability across backends is one of the 2 main
reasons.
Is there any example of OS or linker behaviour that does not start
PostgreSQL with stable function pointer addresses across backends of
the same PostgreSQL binary? Or is this designed with extensibility
and/or cross-version EXEC_BACKEND in mind, but with extensibility not
yet been implemented due to $constraints?

I've attached a patch that solves the issues of (1) and (2).

Kind regards,

Matthias van de Meent
Databricks

Attachment Content-Type Size
v1-0001-AIO-README-clarification-assertion-fix.patch application/octet-stream 1.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Florents Tselai 2025-07-09 11:27:22 Re: like pg_shmem_allocations, but fine-grained for DSM registry ?
Previous Message Hannu Krosing 2025-07-09 11:14:06 Re: What is a typical precision of gettimeofday()?