mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2025-02-17 19:16:56 +00:00
BUG/MEDIUM: stream: fix client-fin/server-fin handling
A tcp half connection can cause 100% CPU on expiration. First reproduced with this haproxy configuration : global tune.bufsize 10485760 defaults timeout server-fin 90s timeout client-fin 90s backend node2 mode tcp timeout server 900s timeout connect 10s server def 127.0.0.1:3333 frontend fe_api mode tcp timeout client 900s bind :1990 use_backend node2 Ie timeout server-fin shorter than timeout server, the backend server sends data, this package is left in the cache of haproxy, the backend server continue sending fin package, haproxy recv fin package. this time the session information is as follows: time the session information is as follows: 0x2373470: proto=tcpv4 src=127.0.0.1:39513 fe=fe_api be=node2 srv=def ts=08 age=1s calls=3 rq[f=848000h,i=0,an=00h,rx=14m58s,wx=,ax=] rp[f=8004c020h,i=0,an=00h,rx=,wx=14m58s,ax=] s0=[7,0h,fd=6,ex=] s1=[7,18h,fd=7,ex=] exp=14m58s rp has set the CF_SHUTR state, next, the client sends the fin package, session information is as follows: 0x2373470: proto=tcpv4 src=127.0.0.1:39513 fe=fe_api be=node2 srv=def ts=08 age=38s calls=4 rq[f=84a020h,i=0,an=00h,rx=,wx=,ax=] rp[f=8004c020h,i=0,an=00h,rx=1m11s,wx=14m21s,ax=] s0=[7,0h,fd=6,ex=] s1=[9,10h,fd=7,ex=] exp=1m11s After waiting 90s, session information is as follows: 0x2373470: proto=tcpv4 src=127.0.0.1:39513 fe=fe_api be=node2 srv=def ts=04 age=4m11s calls=718074391 rq[f=84a020h,i=0,an=00h,rx=,wx=,ax=] rp[f=8004c020h,i=0,an=00h,rx=?,wx=10m49s,ax=] s0=[7,0h,fd=6,ex=] s1=[9,10h,fd=7,ex=] exp=? run(nice=0) cpu information: 6899 root 20 0 112224 21408 4260 R 100.0 0.7 3:04.96 haproxy Buffering is set to ensure that there is data in the haproxy buffer, and haproxy can receive the fin package, set the CF_SHUTR flag, If the CF_SHUTR flag has been set, The following code does not clear the timeout message, causing cpu 100%: stream.c:process_stream: if (unlikely((res->flags & (CF_SHUTR|CF_READ_TIMEOUT)) == CF_READ_TIMEOUT)) { if (si_b->flags & SI_FL_NOHALF) si_b->flags |= SI_FL_NOLINGER; si_shutr(si_b); } If you have closed the read, set the read timeout does not make sense. With or without cf_shutr, read timeout is set: if (tick_isset(s->be->timeout.serverfin)) { res->rto = s->be->timeout.serverfin; res->rex = tick_add(now_ms, res->rto); } After discussion on the mailing list, setting half-closed timeouts the hard way here doesn't make sense. They should be set only at the moment the shutdown() is performed. It will also solve a special case which was already reported of some half-closed timeouts not working when the shutw() is performed directly at the stream-interface layer (no analyser involved). Since the stream interface layer cannot know the timeout values, we'll have to store them directly in the stream interface so that they are used upon shutw(). This patch does this, fixing the problem. An easier reproducer to validate the fix is to keep the huge buffer and shorten all timeouts, then call it under tcploop server and client, and wait 3 seconds to see haproxy run at 100% CPU : global tune.bufsize 10485760 listen px bind :1990 timeout client 90s timeout server 90s timeout connect 1s timeout server-fin 3s timeout client-fin 3s server def 127.0.0.1:3333 $ tcploop 3333 L W N20 A P100 F P10000 & $ tcploop 127.0.0.1:1990 C S10000000 F
This commit is contained in:
parent
aa83a3144b
commit
e39683c4d4
@ -99,6 +99,7 @@ struct stream_interface {
|
||||
/* struct members below are the "remote" part, as seen from the buffer side */
|
||||
unsigned int err_type; /* first error detected, one of SI_ET_* */
|
||||
int conn_retries; /* number of connect retries left */
|
||||
unsigned int hcto; /* half-closed timeout (0 = unset) */
|
||||
};
|
||||
|
||||
/* operations available on a stream-interface */
|
||||
|
@ -9033,6 +9033,7 @@ void http_reset_txn(struct stream *s)
|
||||
s->res.rex = TICK_ETERNITY;
|
||||
s->res.wex = TICK_ETERNITY;
|
||||
s->res.analyse_exp = TICK_ETERNITY;
|
||||
s->si[1].hcto = TICK_ETERNITY;
|
||||
}
|
||||
|
||||
void free_http_res_rules(struct list *r)
|
||||
|
@ -1152,6 +1152,9 @@ int stream_set_backend(struct stream *s, struct proxy *be)
|
||||
if (be->options2 & PR_O2_INDEPSTR)
|
||||
s->si[1].flags |= SI_FL_INDEP_STR;
|
||||
|
||||
if (tick_isset(be->timeout.serverfin))
|
||||
s->si[1].hcto = be->timeout.serverfin;
|
||||
|
||||
/* We want to enable the backend-specific analysers except those which
|
||||
* were already run as part of the frontend/listener. Note that it would
|
||||
* be more reliable to store the list of analysers that have been run,
|
||||
|
10
src/stream.c
10
src/stream.c
@ -168,6 +168,7 @@ struct stream *stream_new(struct session *sess, struct task *t, enum obj_type *o
|
||||
/* this part should be common with other protocols */
|
||||
si_reset(&s->si[0]);
|
||||
si_set_state(&s->si[0], SI_ST_EST);
|
||||
s->si[0].hcto = sess->fe->timeout.clientfin;
|
||||
|
||||
/* attach the incoming connection to the stream interface now. */
|
||||
if (conn)
|
||||
@ -182,6 +183,7 @@ struct stream *stream_new(struct session *sess, struct task *t, enum obj_type *o
|
||||
* callbacks will be initialized before attempting to connect.
|
||||
*/
|
||||
si_reset(&s->si[1]);
|
||||
s->si[1].hcto = TICK_ETERNITY;
|
||||
|
||||
if (likely(sess->fe->options2 & PR_O2_INDEPSTR))
|
||||
s->si[1].flags |= SI_FL_INDEP_STR;
|
||||
@ -2056,10 +2058,6 @@ struct task *process_stream(struct task *t)
|
||||
if (req->flags & CF_READ_ERROR)
|
||||
si_b->flags |= SI_FL_NOLINGER;
|
||||
si_shutw(si_b);
|
||||
if (tick_isset(s->be->timeout.serverfin)) {
|
||||
res->rto = s->be->timeout.serverfin;
|
||||
res->rex = tick_add(now_ms, res->rto);
|
||||
}
|
||||
}
|
||||
|
||||
/* shutdown(write) done on server side, we must stop the client too */
|
||||
@ -2239,10 +2237,6 @@ struct task *process_stream(struct task *t)
|
||||
if (unlikely((res->flags & (CF_SHUTW|CF_SHUTW_NOW)) == CF_SHUTW_NOW &&
|
||||
channel_is_empty(res))) {
|
||||
si_shutw(si_f);
|
||||
if (tick_isset(sess->fe->timeout.clientfin)) {
|
||||
req->rto = sess->fe->timeout.clientfin;
|
||||
req->rex = tick_add(now_ms, req->rto);
|
||||
}
|
||||
}
|
||||
|
||||
/* shutdown(write) done on the client side, we must stop the server too */
|
||||
|
@ -205,6 +205,11 @@ static void stream_int_shutw(struct stream_interface *si)
|
||||
oc->wex = TICK_ETERNITY;
|
||||
si->flags &= ~SI_FL_WAIT_DATA;
|
||||
|
||||
if (tick_isset(si->hcto)) {
|
||||
ic->rto = si->hcto;
|
||||
ic->rex = tick_add(now_ms, ic->rto);
|
||||
}
|
||||
|
||||
switch (si->state) {
|
||||
case SI_ST_EST:
|
||||
/* we have to shut before closing, otherwise some short messages
|
||||
@ -826,6 +831,11 @@ static void stream_int_shutw_conn(struct stream_interface *si)
|
||||
oc->wex = TICK_ETERNITY;
|
||||
si->flags &= ~SI_FL_WAIT_DATA;
|
||||
|
||||
if (tick_isset(si->hcto)) {
|
||||
ic->rto = si->hcto;
|
||||
ic->rex = tick_add(now_ms, ic->rto);
|
||||
}
|
||||
|
||||
switch (si->state) {
|
||||
case SI_ST_EST:
|
||||
/* we have to shut before closing, otherwise some short messages
|
||||
@ -1441,6 +1451,11 @@ static void stream_int_shutw_applet(struct stream_interface *si)
|
||||
oc->wex = TICK_ETERNITY;
|
||||
si->flags &= ~SI_FL_WAIT_DATA;
|
||||
|
||||
if (tick_isset(si->hcto)) {
|
||||
ic->rto = si->hcto;
|
||||
ic->rex = tick_add(now_ms, ic->rto);
|
||||
}
|
||||
|
||||
/* on shutw we always wake the applet up */
|
||||
appctx_wakeup(si_appctx(si));
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user