Wow, thank you for the patch. > > > 0001: Allows WaitLatchOrSocket(WL_WRITABLE) without WL_READABLE. I've > > > tested the poll() and select() implementations on linux and > > > blindly patched up windows. > > > 0002: Put the socket the backend uses to communicate with the client > > > into nonblocking mode as soon as latches are ready and use latches > > > to wait. This probably doesn't work correctly without 0003, but > > > seems easier to review separately. > > > 0003: Don't do sinval catchup and notify processing in signal > > > handlers. It's quite cool that it worked that well so far, but it > > > requires some complicated code and is rather fragile. 0002 allows > > > to move that out of signal handlers and just use a latch > > > there. This seems remarkably simpler: > > > 4 files changed, 69 insertions(+), 229 deletions(-) > > > > > > These aren't ready for commit, especially not 0003, but I think they are > > > quite a good foundation for getting rid of the blocking in send(). I > > > haven't added any interrupt processing after interrupted writes, but > > > marked the relevant places with XXXs. > > > > > > With regard to 0002, I dislike the current need to do interrupt > > > processing both in be-secure.c and be-secure-openssl.c. I guess we could > > > solve that by returning something like EINTR from the ssl routines when > > > they need further reads/writes and do all the processing in one place in > > > be-secure.c. > > > > > > There's also some cleanup in 0002/0003 needed: > > > prepare_for_client_read()/client_read_ended() aren't needed in that form > > > anymore and should probably rather be something like > > > CHECK_FOR_READ_INTERRUPT() or similar. Similarly the > > > EnableCatchupInterrupt()/DisableCatchupInterrupt() in autovacuum.c is > > > pretty ugly. > > > > > > Btw, be-secure.c is really not a good name anymore... > > > > > > What do you think? > > > > I've invested some more time in this: > > 0002 now makes sense on its own and doesn't change anything around the > > interrupt handling. Oh, and it compiles without 0003. > > 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. > > Kyatoro, could you check whether you can achieve what you want using > 0004? > > It's imo pretty clear that a fair amount of base work needs to be done > and there's been a fair amount of progress made this fest. I think this > can now be marked returned with feedback.
Myself is satisfied by Heikki's solution, and it seems ready for commit. But I agree with the temporarily blocked state is seen often and it breaks even non-blocked socket. If we want to/should avoid breaking *temporarily or not* blocked socket even for SIGTERM, this mechanism should be used. Which way should we take? regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers