Re: Extended Prefetching using Asynchronous IO - proposal and patch

From: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
To: John Lumby <johnlumby(at)hotmail(dot)com>, Claudio Freire <klaussfreire(at)gmail(dot)com>, pgsql hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Greg Stark <stark(at)mit(dot)edu>, "andres(at)2ndquadrant(dot)com" <andres(at)2ndquadrant(dot)com>
Subject: Re: Extended Prefetching using Asynchronous IO - proposal and patch
Date: 2014-08-19 22:27:39
Message-ID: 53F3CF5B.4000607@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general pgsql-hackers

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(at)phx(dot)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

In response to

Responses

Browse pgsql-general by date

  From Date Subject
Next Message Adrian Klaver 2014-08-19 22:33:17 Re: pgbadger download
Previous Message Joe Van Dyk 2014-08-19 22:20:10 Re: adding a nullable column of type domain w/ check constraint runs checks?

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2014-08-19 22:31:03 Re: PQgetssl() and alternative SSL implementations
Previous Message Heikki Linnakangas 2014-08-19 21:58:22 Re: PQgetssl() and alternative SSL implementations