Re: AIO v2.5

Hi,

On 2025-03-25 13:18:50 -0700, Noah Misch wrote:
> On Tue, Mar 25, 2025 at 04:07:35PM -0400, Andres Freund wrote:
> > On 2025-03-25 12:39:56 -0700, Noah Misch wrote:
> > > On Tue, Mar 25, 2025 at 02:58:37PM -0400, Andres Freund wrote:
> > > > There are 2 1/2 ways around this:
> > > > 
> > > > 1) Stop using IOSQE_ASYNC heuristic
> > > > 2a) Wait for all in-flight IOs when any FD gets closed
> > > > 2b) Wait for all in-flight IOs using FD when it gets closed
> > > > 
> > > > Given that we have clear evidence that io_uring doesn't completely support
> > > > closing FDs while IOs are in flight, be it a bug or intentional, it seems
> > > > clearly better to go for 2a or 2b.
> > > 
> > > Agreed.  If a workload spends significant time on fd.c closing files, I
> > > suspect that workload already won't have impressive benchmark numbers.
> > > Performance-seeking workloads will already want to tune FD usage high enough
> > > to keep FDs long-lived.  So (1) clearly loses, and neither (2a) nor (2b)
> > > clearly beats the other.  I'd try (2b) first but, if complicated, quickly
> > > abandon it in favor of (2a).  What other considerations could be important?
> > 
> > The only other consideration I can think of is whether this should happen for
> > all io_methods or not.
> 
> Either way is fine, I think.

Here's a draft incremental patch (attached as a .fixup to avoid triggering
cfbot) implementing 2b).


> > I'm inclined to do it via a bool in IoMethodOps, but I guess one could argue
> > it's a bit weird to have a bool in a struct called *Ops.
> 
> That wouldn't bother me.  IndexAmRoutine has many bools, and "Ops" is
> basically a synonym of "Routine".

Cool. Done that way.

The repeated-iteration approach taken in pgaio_closing_fd() isn't the
prettiest, but it's hard to to imagine that ever being a noticeable.


This survives a testrun where I use your torture patch and where I force all
IOs to use ASYNC. Previously that did not get very far.  I also did verify
that, if I allow a small number of FDs, we do not wrongly wait for all IOs.

Greetings,

Andres Freund