[BUG] fix the dequeuing logic to ensure that all requests get served
The dequeuing logic was completely wrong. First, a task was assigned
to all servers to process the queue, but this task was never scheduled
and was only woken up on session free. Second, there was no reservation
of server entries when a task was assigned a server. This means that
as long as the task was not connected to the server, its presence was
not accounted for. This was causing trouble when detecting whether or
not a server had reached maxconn. Third, during a redispatch, a session
could lose its place at the server's and get blocked because another
session at the same moment would have stolen the entry. Fourth, the
redispatch option did not work when maxqueue was reached for a server,
and it was not possible to do so without indefinitely hanging a session.
The root cause of all those problems was the lack of pre-reservation of
connections at the server's, and the lack of tracking of servers during
a redispatch. Everything relied on combinations of flags which could
appear similarly in quite distinct situations.
This patch is a major rework but there was no other solution, as the
internal logic was deeply flawed. The resulting code is cleaner, more
understandable, uses less magics and is overall more robust.
As an added bonus, "option redispatch" now works when maxqueue has
been reached on a server.
diff --git a/src/proto_http.c b/src/proto_http.c
index c863ae0..e366ee5 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -1,7 +1,7 @@
/*
* HTTP protocol analyzer
*
- * Copyright 2000-2007 Willy Tarreau <w@1wt.eu>
+ * Copyright 2000-2008 Willy Tarreau <w@1wt.eu>
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -2656,9 +2656,9 @@
* to any other session to release it and wake us up again.
*/
if (t->pend_pos) {
- if (!tv_isle(&req->cex, &now))
+ if (!tv_isle(&req->cex, &now)) {
return 0;
- else {
+ } else {
/* we've been waiting too long here */
tv_eternity(&req->cex);
t->logs.t_queue = tv_ms_elapsed(&t->logs.tv_accept, &now);
@@ -2747,8 +2747,7 @@
fd_delete(t->srv_fd);
if (t->srv) {
t->srv->cur_sess--;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
}
@@ -2776,8 +2775,7 @@
fd_delete(t->srv_fd);
if (t->srv) {
t->srv->cur_sess--;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
if (!(req->flags & BF_WRITE_STATUS))
@@ -2808,10 +2806,11 @@
*/
/* let's try to offer this slot to anybody */
if (may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
/* it's left to the dispatcher to choose a server */
t->flags &= ~(SN_DIRECT | SN_ASSIGNED | SN_ADDR_SET);
+ t->prev_srv = t->srv;
/* first, get a connection */
if (srv_redispatch_connect(t))
@@ -2971,8 +2970,7 @@
if (t->srv) {
t->srv->cur_sess--;
t->srv->failed_resp++;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
t->be->failed_resp++;
t->srv_state = SV_STCLOSE;
@@ -2986,7 +2984,7 @@
* we have to inform the server that it may be used by another session.
*/
if (t->srv && may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}
@@ -3015,8 +3013,7 @@
if (t->srv) {
t->srv->cur_sess--;
t->srv->failed_resp++;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
t->be->failed_resp++;
t->srv_state = SV_STCLOSE;
@@ -3030,7 +3027,7 @@
* we have to inform the server that it may be used by another session.
*/
if (t->srv && may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}
@@ -3185,8 +3182,7 @@
if (t->srv) {
t->srv->cur_sess--;
t->srv->failed_resp++;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
cur_proxy->failed_resp++;
return_srv_prx_502:
@@ -3204,7 +3200,7 @@
* we have to inform the server that it may be used by another session.
*/
if (t->srv && may_dequeue_tasks(t->srv, cur_proxy))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}
}
@@ -3214,8 +3210,7 @@
if (t->srv) {
t->srv->cur_sess--;
t->srv->failed_secu++;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
cur_proxy->denied_resp++;
goto return_srv_prx_502;
@@ -3354,8 +3349,7 @@
if (t->srv) {
t->srv->cur_sess--;
t->srv->failed_secu++;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
t->be->denied_resp++;
@@ -3443,8 +3437,7 @@
if (t->srv) {
t->srv->cur_sess--;
t->srv->failed_resp++;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
t->be->failed_resp++;
t->srv_state = SV_STCLOSE;
@@ -3456,7 +3449,7 @@
* we have to inform the server that it may be used by another session.
*/
if (may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}
@@ -3552,8 +3545,7 @@
if (t->srv) {
t->srv->cur_sess--;
t->srv->failed_resp++;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
t->be->failed_resp++;
//close(t->srv_fd);
@@ -3566,7 +3558,7 @@
* we have to inform the server that it may be used by another session.
*/
if (may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}
@@ -3576,8 +3568,7 @@
fd_delete(t->srv_fd);
if (t->srv) {
t->srv->cur_sess--;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
//close(t->srv_fd);
t->srv_state = SV_STCLOSE;
@@ -3585,7 +3576,7 @@
* we have to inform the server that it may be used by another session.
*/
if (may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}
@@ -3595,8 +3586,7 @@
fd_delete(t->srv_fd);
if (t->srv) {
t->srv->cur_sess--;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
//close(t->srv_fd);
t->srv_state = SV_STCLOSE;
@@ -3608,7 +3598,7 @@
* we have to inform the server that it may be used by another session.
*/
if (may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}
@@ -3635,8 +3625,7 @@
if (t->srv) {
t->srv->cur_sess--;
t->srv->failed_resp++;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
t->be->failed_resp++;
//close(t->srv_fd);
@@ -3649,7 +3638,7 @@
* we have to inform the server that it may be used by another session.
*/
if (may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}
@@ -3659,8 +3648,7 @@
fd_delete(t->srv_fd);
if (t->srv) {
t->srv->cur_sess--;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
//close(t->srv_fd);
t->srv_state = SV_STCLOSE;
@@ -3668,7 +3656,7 @@
* we have to inform the server that it may be used by another session.
*/
if (may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}
@@ -3678,8 +3666,7 @@
fd_delete(t->srv_fd);
if (t->srv) {
t->srv->cur_sess--;
- if (t->srv->proxy->lbprm.server_drop_conn)
- t->srv->proxy->lbprm.server_drop_conn(t->srv);
+ sess_change_server(t, NULL);
}
//close(t->srv_fd);
t->srv_state = SV_STCLOSE;
@@ -3691,7 +3678,7 @@
* we have to inform the server that it may be used by another session.
*/
if (may_dequeue_tasks(t->srv, t->be))
- task_wakeup(t->srv->queue_mgt);
+ process_srv_queue(t->srv);
return 1;
}