Re: Extended Prefetching using Asynchronous IO - proposal and patch

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