Re: Escaping from blocked send() reprised.

On 2014-09-03 15:09:54 +0300, Heikki Linnakangas wrote:
> On 09/03/2014 12:23 AM, Andres Freund wrote:
> >On 2014-09-02 17:21:03 -0400, Tom Lane wrote:
> >>Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> writes:
> >>>I was going to suggest using WaitLatchOrSocket instead of sleeping in 1
> >>>second increment, but I see that WaitLatchOrSocket() doesn't currently
> >>>support waiting for a socket to become writeable, without also waiting
> >>>for it to become readable. I wonder how difficult it would be to lift
> >>>that restriction.
> >>
> >>My recollection is that there was a reason for that, but I don't recall
> >>details any more.
> >
> >http://git.postgresql.org/pg/commitdiff/e42a21b9e6c9b9e6346a34b62628d48ff2fc6ddf
> >
> >In my prototype I've changed the API that errors set both
> >READABLE/WRITABLE. Seems to work....
>
> Andres, would you mind posting the WIP patch you have? That could be a
> better foundation for this patch.

Sorry, I missed this message and only cought up when reading your CF
status mail. I've attached three patches:

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?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services