Hi Frederik, On Thu, Mar 31, 2016 at 12:37:03PM -0700, Frederik Deweerdt wrote: > >> It seems that we would be a bit more efficient if we also aborted when > >> si_b->state was SI_ST_INI: that is, don't even try to open a connection > >> to the backend if we're shutting down the frontend. > > > > No, we should not do this. You can already force this behaviour with > > "option abortonclose". > > Mmm, adding "option abortonclose" does work in "mode http", but not in > "mode tcp", which I've been using.
Why are you saying this ? I'm not seeing anything restricting abortonclose to HTTP mode only. > I can however confirm that if the > check becomes: > (s->be->options & PR_O_ABRT_CLOSE || channel_is_empty(req)) > rather than > channel_is_empty(req) > it does close the connection there. Then you definitely need to enable the option :-) > > I'm thinking about two possibilities : > > - either we consider that if we can't retrieve a connection's address > > for a proxy protocol line we must fail and abort the connection ; > > - or we consider that when we're closing a front connection early > > (with "early" still to be defined, maybe with something in the > > request buffer), then we retrieve the destination address prior > > to closing. Or maybe we should retrieve this each time the client > > closes first (read 0 or error caught) except when the session is > > idle. > > > > I guess we should do both. First ensure that we always have the > > socket's addresses before closing on error, and then cover the > > possible remaining cases by aborting outgoing connections with > > an incomplete proxy proto. > > > > What do you think ? > > I hadn't considered this - I was trying to address the "don't open > a connection if the peer fd is closed" case - but I think that > both fixes sound good, with a preference for retrieving each > time the client closes first, since that has simpler semantics. > At the very least, it sounds like the proxy code should be looking > the CO_FL_ADDR_*_SET flags. It's always what is looked at before performing the address lookup, it's just that we want to avoid doing it if we don't need it. I guess we can find a way to mark that a stream is idle and thus that the client may safely close without causing its address to be retrieved first. This state would happen after logging or after a stream is recycled while waiting for a new request over a connection. I'll need to think about it a bit more, because it might become easier with the next changes needed to progress towards HTTP/2. Regards, Willy
