On 09/28/2014 01:54 AM, Andres Freund wrote:
I've invested some more time in this:
Thanks!
In 0001, the select() codepath will not return (WL_SOCKET_READABLE |
WL_SOCKET_WRITEABLE) on EOF or error, like the comment says and like the
poll() path does. It only sets WL_SOCKET_READABLE if WL_SOCKET_READABLE
was requested as a wake-event, and likewise for writeable, while the
poll() codepath returns (WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE)
regardless of the requested wake-events. I'm not sure which is actually
better - a separate WL_SOCKET_ERROR code might be best - but it's
inconsistent as it is.
0002 now makes sense on its own and doesn't change anything around the
interrupt handling. Oh, and it compiles without 0003.
WaitLatchOrSocket() can throw an error, so it's not totally safe to call
that underneath OpenSSL. Admittedly the cases where it throws an error
are "shouldn't happen" cases like "poll() failed" or "read() on
self-pipe failed", but still. Perhaps those errors should be
reclassified as FATAL; it's not clear you can just roll back and expect
to continue running if any of them happens.
In secure_raw_write(), you need to save and restore errno, as
WaitLatchOrSocket will not preserve it. If secure_raw_write() calls
WaitLatchOrSocket(), and it returns because the latch was set, and we
fall out of secure_raw_write, we will return -1 but the errno might not
be set to anything sensible anymore.
0003 Sinval/notify processing got simplified further. There really isn't
any need for DisableNotifyInterrupt/DisableCatchupInterrupt
anymore. Also begin_client_read/client_read_ended don't make much
sense anymore. Instead introduce ProcessClientReadInterrupt (which
wants a better name).
There's also a very WIP
0004 Allows secure_read/write be interrupted when ProcDiePending is
set. All of that happens via the latch mechanism, nothing happens
inside signal handlers. So I do think it's quite an improvement
over what's been discussed in this thread.
But it (and the other approaches) do noticeably increase the
likelihood of clients not getting the error message if the client
isn't actually dead. The likelihood of write() being blocked
*temporarily* due to normal bandwidth constraints is quite high
when you consider COPY FROM and similar. Right now we'll wait till
we can put the error message into the socket afaics.
1-3 need some serious comment work, but I think the approach is
basically sound. I'm much, much less sure about allowing send() to be
interrupted.
Yeah, 1-3 seem sane. 4 also looks OK to me at a quick glance. It
basically enables handling the "die" interrupt immediately, if we're
blocked in a read or write. It won't be handled in the signal handler,
but within the secure_read/write call anyway.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
