Re: Extended Prefetching using Asynchronous IO - proposal and patch - Mailing list pgsql-hackers

On 06/24/2014 06:08 PM, John Lumby wrote:
>> The question is, if you receive the notification of the I/O completion
>> using a signal or a thread, is it safe to release the lwlock from the
>> signal handler or a separate thread?
>
> In the forthcoming  new version of the patch that uses sigevent,
> the originator locks a LWlock associated with that BAaiocb eXclusive,
> and ,   when signalled,  in the signal handler it places that LWlock
> on a process-local queue of LWlocks awaiting release.
> (No, It cannot be safely released inside the signal handler or in a
> separate thread).     Whenever the mainline passes a CHECK_INTERRUPTS macro
> and at a few additional points in bufmgr,  the backend walks this process-local
> queue and releases those LWlocks.    This is also done if the originator
> itself issues a ReadBuffer,  which is the most frequent case in which it
> is released.
>
> Meanwhile,  any other backend will simply acquire Shared and release.

Ok, doing the work in CHECK_FOR_INTERRUPTS sounds safe. But is that fast 
enough? We have never made any hard guarantees on how often 
CHECK_FOR_INTERRUPTS() is called. In particular, if you're running 3rd 
party C code or PL code, there might be no CHECK_FOR_INTERRUPTS() calls 
for many seconds, or even more. That's a long time to hold onto a buffer 
I/O lock. I don't think that's acceptable :-(.

> I think you are right that the existing io_in_progress_lock LWlock in the
> buf_header  could be used for this,  because if there is a aio in progress,
> then that lock cannot be in use for synchronous IO.  I chose not to use it
> because I preferred to keep the wait/post for asynch io separate,
>   but they could both use the same LWlock.   However,   the way the LWlock
> is acquired and released would still be a bit different because of the need
> to have the originator release it in its mainline.

It would be nice to use the same LWLock.

However, if releasing a regular LWLock in a signal handler is not safe, 
and cannot be made safe, perhaps we should, after all, invent a whole 
new mechanism. One that would make it safe to release the lock in a 
signal handler.

>>> By the way, on the "will it actually work though?" question which several folks
>>> have raised, I should mention that this patch has been in semi-production
>>> use for almost 2 years now in different stages of completion on all postgresql
>>> releases from 9.1.4 to 9.5 devel. I would guess it has had around
>>> 500 hours of operation by now. I'm sure there are bugs still to be
>>> found but I am confident it is fundamentally sound.
>>
>> Well, a committable version of this patch is going to look quite
>> different from the first version that you posted, so I don't put much
>> weight on how long you've tested the first version.
>
> Yes,  I am quite willing to change it,  time permitting.
> I take the works "committable version" as a positive sign ...

BTW, sorry if I sound negative, I'm actually quite excited about this 
feature. A patch like this take a lot of work, and usually several 
rewrites, until it's ready ;-). But I'm looking forward for it.

- Heikki