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-w412ff89303686022a9f16aa3...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
