Hi again Willy,
Le 21/07/2013 20:09, Willy Tarreau a écrit :
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.
Ah right, I forgot to check splicing.
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.
It's not heavily tested but I was thinking about such a patch (in
attachment). It looks to work with/without splicing.
Originally, the feature claimed to impact the performance about 0.5%,
but as haproxy 1.5 introduced stick tables, I'm not sure about this
(session_process_counters does more than it was in 1.3).
Downloading a 1GB file across a gigabit ethernet lan with splicing
enabled and option contstats, counters were updated 68043 times.
The same without splicing, counters were updated 86798 times.
The drawback is that stream_interface has to know about the session
(wich is already provided in the structures).
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
--
Cyril Bonté
diff --git a/src/session.c b/src/session.c
index 0a6b130..057cb13 100644
--- a/src/session.c
+++ b/src/session.c
@@ -693,6 +693,10 @@ void session_process_counters(struct session *s)
void *ptr;
int i;
+#ifdef DEBUG_CONTSTATS
+ static int _spc = 0;
+ fprintf(stderr, "session_process_counters : %d\n", ++_spc);
+#endif
if (s->req) {
bytes = s->req->total - s->logs.bytes_in;
s->logs.bytes_in = s->req->total;
@@ -2346,9 +2350,6 @@ struct task *process_session(struct task *t)
if (likely((s->rep->cons->state != SI_ST_CLO) ||
(s->req->cons->state > SI_ST_INI && s->req->cons->state < SI_ST_CLO))) {
- if ((s->fe->options & PR_O_CONTSTATS) && (s->flags & SN_BE_ASSIGNED))
- session_process_counters(s);
-
if (s->rep->cons->state == SI_ST_EST && obj_type(s->rep->cons->conn->target) != OBJ_TYPE_APPLET)
si_update(s->rep->cons);
diff --git a/src/stream_interface.c b/src/stream_interface.c
index 8a21d39..4246efe 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -30,6 +30,7 @@
#include <proto/connection.h>
#include <proto/fd.h>
#include <proto/pipe.h>
+#include <proto/session.h>
#include <proto/stream_interface.h>
#include <proto/task.h>
@@ -615,6 +616,8 @@ static int si_conn_wake_cb(struct connection *conn)
static int si_conn_send_loop(struct connection *conn)
{
struct stream_interface *si = conn->owner;
+ struct task *t = si->owner;
+ struct session *s = t->context;
struct channel *chn = si->ob;
int ret;
@@ -636,7 +639,7 @@ static int si_conn_send_loop(struct connection *conn)
* in the normal buffer.
*/
if (!chn->buf->o)
- return 0;
+ goto out;
/* when we're in this loop, we already know that there is no spliced
* data left, and that there are sendable buffered data.
@@ -678,9 +681,14 @@ static int si_conn_send_loop(struct connection *conn)
break;
} /* while */
+
if (conn->flags & CO_FL_ERROR)
return -1;
+ out:
+ if (s && s->fe->options & PR_O_CONTSTATS && (s->flags & SN_BE_ASSIGNED))
+ session_process_counters(s);
+
return 0;
}
