[BUG] do not dequeue the backend's pending connections on a dead server

Kai Krueger found that previous patch was incomplete, because there is
an unconditionnal call to process_srv_queue() in session_free() which
still causes a dead server to consume pending connections from the
backend.

This call was made unconditionnal so that we don't leave unserved
connections in the server queue, for instance connections coming
in with "option persist" which can bypass the server status check.
However, the server must not touch the backend's queue if it is down.

Another fear was that some connections might remain unserved when
the server is using a dynamic maxconn if the number of connections
to the backend is too low. Right now, srv_dynamic_maxconn() ensures
this cannot happen, so the call can remain conditionnal.

The fix consists in allowing a server to process it own queue whatever
its state, but not to touch the backend's queue if it is down. Its
queue should normally be empty when the server is down because it is
redistributed when the server goes down. The only remaining cases are
precisely the persistent connections with "option persist" set, coming
in after the queue has been redispatched. Those ones must still be
processed when a connection terminates.
(cherry picked from commit cd485c44807bfcdb4928dd83c1907636b4e1b6f3)

diff --git a/include/proto/queue.h b/include/proto/queue.h
index be78b535612ced7d2b873fede759eafb4f8be653..77964feb3783167ae01f5b1f1dfb41997ce8f4b1 100644 (file)
--- a/include/proto/queue.h
+++ b/include/proto/queue.h
@@ -64,11 +64,11 @@ static inline struct pendconn *pendconn_from_px(const struct proxy *px) {
 }
 
 /* returns 0 if nothing has to be done for server <s> regarding queued connections,
- * and non-zero otherwise. If the server is down, we always return zero. Suited for
- * and if/else usage.
+ * and non-zero otherwise. If the server is down, we only check its own queue. Suited
+ * for and if/else usage.
  */
 static inline int may_dequeue_tasks(const struct server *s, const struct proxy *p) {
-       return (s && (s->state & SRV_RUNNING) && (s->nbpend || p->nbpend) &&
+       return (s && (s->nbpend || (p->nbpend && (s->state & SRV_RUNNING))) &&
                (!s->maxconn || s->cur_sess < srv_dynamic_maxconn(s)));
 }
 

diff --git a/src/queue.c b/src/queue.c
index 172e0560d05f79da35a284772a0a1f0342132604..fe940436433b8a888e8e3646f4ea139f9d023287 100644 (file)
--- a/src/queue.c
+++ b/src/queue.c
@@ -86,6 +86,10 @@ void process_srv_queue(struct server *s)
  * returned. Note that neither <srv> nor <px> may be NULL.
  * Priority is given to the oldest request in the queue if both <srv> and <px>
  * have pending requests. This ensures that no request will be left unserved.
+ * The <px> queue is not considered if the server is not RUNNING. The <srv>
+ * queue is still considered in this case, because if some connections remain
+ * there, it means that some requests have been forced there after it was seen
+ * down (eg: due to option persist).
  * The session is immediately marked as "assigned", and both its <srv> and
  * <srv_conn> are set to <srv>,
  */
@@ -97,7 +101,7 @@ struct session *pendconn_get_next_sess(struct server *srv, struct proxy *px)
        ps = pendconn_from_srv(srv);
        pp = pendconn_from_px(px);
        /* we want to get the definitive pendconn in <ps> */
-       if (!pp) {
+       if (!pp || !(srv->state & SRV_RUNNING)) {
                if (!ps)
                        return NULL;
        } else {

diff --git a/src/session.c b/src/session.c
index c0df6c68509d0959ac2cd3dda3fade3088479b43..5046e04264421f3d3431a158cd8fff2e1204784e 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -46,13 +46,16 @@ void session_free(struct session *s)
 
        if (s->pend_pos)
                pendconn_free(s->pend_pos);
+
        if (s->srv) { /* there may be requests left pending in queue */
                if (s->flags & SN_CURR_SESS) {
                        s->flags &= ~SN_CURR_SESS;
                        s->srv->cur_sess--;
                }
-               process_srv_queue(s->srv);
+               if (may_dequeue_tasks(s->srv, s->be))
+                       process_srv_queue(s->srv);
        }
+
        if (unlikely(s->srv_conn)) {
                /* the session still has a reserved slot on a server, but
                 * it should normally be only the same as the one above,