On 08/20/2014 12:17 AM, John Lumby wrote:
> I am attaching a new version of the patch for consideration in the current commit fest.
Thanks for working on this!
> Relative to the one I submitted on 25 June in BAY175-W412FF89303686022A9F16AA3190@phx.gbl
> the method for handling aio completion using sigevent has been re-written to use
> signals exclusively rather than a composite of signals and LWlocks,
> and this has fixed the problem I mentioned before with the LWlock method.
ISTM the patch is still allocating stuff in shared memory that really
doesn't belong there. Namely, the BufferAiocb structs. Or at least parts
of it; there's also a waiter queue there which probably needs to live in
shared memory, but the rest of it does not.
At least BufAWaitAioCompletion is still calling aio_error() on an AIO
request that might've been initiated by another backend. That's not OK.
Please write the patch without atomic CAS operation. Just use a
spinlock. There's a patch in the commitfest to add support for that, but
it's not committed yet, and all those USE_AIO_ATOMIC_BUILTIN_COMP_SWAP
ifdefs make the patch more difficult to read. Same with all the other
#ifdefs; please just pick a method that works.
Also, please split prefetching of regular index scans into a separate
patch. It's orthogonal to doing async I/O; we could prefetch regular
index scans with posix_fadvise too, and AIO should be useful for
prefetching other stuff.
- Heikki
