Hi Cyril, On Sun, Jul 21, 2013 at 05:03:19PM +0200, Cyril Bonté wrote: > Hi Willy and Krzysztof, > > It appears that "option contstats" added in haproxy 1.3.14 is broken > since 1.3.16 and is still broken in 1.4 and 1.5 branches.
Yes I know, it is only updated when the processing is given back to process_session(), so basically between multiple HTTP requests, or at the end of a TCP connection. It's been that way as you say, since 1.3.16 (the one which introduced forwarding between stream interfaces). I never really considered that as a bug but rather as a limitation. > It's easier to show the issue with long tcp connections : counters are > updated only on connection close. In my case, it prevents tracking mysql > activity when a connection pool is used on the client side. > > Examle : > listen stats :4000 > mode http > stats enable > > listen test :3307 > mode tcp > option contstats > server mysql localhost:3306 > > > Before trying to fix this, I'd like to discuss about it. > In haproxy 1.5, a stream interface contains a reference to the current > session by walking thru si->owner->context. > I wonder if it shouldn't be the role of the stream interface to update > the counters, maybe at the end of si_conn_send_loop(). > > In the same way, for haproxy 1.4, counters could be updated at the end > of stream_sock_read() (then we should always initialize si->private to > provide a pointer to the current session). > > Do you see a better way to do this ? Tell me if you prefer an initial > patch, it's sometimes better than explanations ;-) I remember having tried several times to address this thing in 1.3 and 1.4 with no elegant solution (eg: some would cause difficulties with the CLI socket and such things, other ways would make the lower layers far too aware of the upper ones). I even thought about having an option to disable the fast-forwarding (and splicing) to force the counters to be valid. This can make sense for low loads as it would basically work the same way as it used to before 1.3.16. First, I think that we should only focus on 1.5 at the moment, as it changed too much from 1.4 to have a general solution. If we find a way to make this work nice in 1.5, then we'll more easily consider whether we want to try to use the same for 1.4 or if we'd rather leave it as it is now. And if we find no elegant enough solution (I mean none which does not make the lower layers aware of the upper), then maybe the solution will be to have a specific option for this behaviour. We may also decide to break contstat and make it disable fast forwarding but I'm really worried about deployed setups, as I've seen this option used a lot, including in large deployments where calling process_session for each small chunk of data might not be an option. Maybe we should try to define a very generic stats counter type which could be updated from many places, always the same way. I must say that at the moment I have no idea yet what the best solution is :-/ Regards, Willy
