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