mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-22 14:21:25 +02:00
Code Issues Projects Releases Wiki Activity

MINOR: queue: use a distinct variable for the assigned server and the queue

Browse Source 
The pendconn struct uses ->px and ->srv to designate where the element is
queued. There is something confusing regarding threads though, because we
have to lock the appropriate queue before inserting/removing elements, and
this queue may only be determined by looking at ->srv (if it's not NULL
it's the server, otherwise use the proxy). But pendconn_grab_from_px() and
pendconn_process_next_strm() both assign this ->srv field, making it
complicated to know what queue to lock before manipulating the element,
which is exactly why we have the pendconn_lock in the first place.

This commit introduces pendconn->target which is the target server that
the two aforementioned functions will set when assigning the server.
Thanks to this, the server pointer may always be relied on to determine
what queue to use.
This commit is contained in:
Willy Tarreau 2018-07-26 07:38:54 +02:00
parent c1a60d6218
commit 88930dd364
2 changed files with 13 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
include/types/queue.h
View File

@ -34,7 +34,8 @@ struct pendconn {
int strm_flags; /* stream flags */ int strm_flags; /* stream flags */
struct stream *strm; struct stream *strm;
struct proxy *px; struct proxy *px;
struct server *srv; /* the server we are waiting for, may be NULL */ struct server *srv; /* the server we are waiting for, may be NULL if don't care */
struct server *target; /* the server that was assigned, = srv except if srv==NULL */
struct list list; /* next pendconn */ struct list list; /* next pendconn */
__decl_hathreads(HA_SPINLOCK_T lock); __decl_hathreads(HA_SPINLOCK_T lock);
}; };

20
src/queue.c
View File

@ -25,8 +25,10 @@
* otherwise it necessarily belongs to one of the other lists ; this may * otherwise it necessarily belongs to one of the other lists ; this may
* not be atomically checked under threads though ; * not be atomically checked under threads though ;
* - pendconn->px is never NULL if pendconn->list is not empty * - pendconn->px is never NULL if pendconn->list is not empty
* - pendconn->sv is never NULL if pendconn->list is in the server's queue, * - pendconn->srv is never NULL if pendconn->list is in the server's queue,
* and is always NULL if pendconn->list is in the backend's queue or empty. * and is always NULL if pendconn->list is in the backend's queue or empty.
* - pendconn->target is NULL while the element is queued, and points to the
* assigned server when the pendconn is picked.
* *
* Threads complicate the design a little bit but rules remain simple : * Threads complicate the design a little bit but rules remain simple :
* - a lock is present in the pendconn (pendconn->lock). This lock is * - a lock is present in the pendconn (pendconn->lock). This lock is
@ -65,10 +67,9 @@
* - no single operation except the pendconn initialisation prior to the * - no single operation except the pendconn initialisation prior to the
* insertion are performed without a queue lock held. * insertion are performed without a queue lock held.
* *
* - pendconn_grab_from_px() and pendconn_process_next_strm() assign ->srv so * - pendconn_grab_from_px() and pendconn_process_next_strm() assign ->target
* that the stream knows what server to work with (via pendconn_dequeue()). * so that the stream knows what server to work with (via
* It's worth noting that this assignment complicates the detection of the * pendconn_dequeue() which sets it on strm->target).
* queue to be locked/unlocked.
* *
* - a pendconn doesn't switch between queues, it stays where it is. * - a pendconn doesn't switch between queues, it stays where it is.
*/ */
@ -239,7 +240,7 @@ static int pendconn_process_next_strm(struct server *srv, struct proxy *px)
pendconn_found: pendconn_found:
__pendconn_unlink(p); __pendconn_unlink(p);
p->strm_flags |= SF_ASSIGNED; p->strm_flags |= SF_ASSIGNED;
p->srv = srv; p->target = srv;
HA_ATOMIC_ADD(&srv->served, 1); HA_ATOMIC_ADD(&srv->served, 1);
HA_ATOMIC_ADD(&srv->proxy->served, 1); HA_ATOMIC_ADD(&srv->proxy->served, 1);
@ -301,6 +302,7 @@ struct pendconn *pendconn_add(struct stream *strm)
srv = objt_server(strm->target); srv = objt_server(strm->target);
px = strm->be; px = strm->be;
p->target = NULL;
p->srv = NULL; p->srv = NULL;
p->px = px; p->px = px;
p->strm = strm; p->strm = strm;
@ -393,7 +395,7 @@ int pendconn_grab_from_px(struct server *s)
continue; continue;
__pendconn_unlink(p); __pendconn_unlink(p);
p->srv = s; p->target = s;
remote |= !(p->strm->task->thread_mask & tid_bit); remote |= !(p->strm->task->thread_mask & tid_bit);
task_wakeup(p->strm->task, TASK_WOKEN_RES); task_wakeup(p->strm->task, TASK_WOKEN_RES);
@ -439,8 +441,8 @@ int pendconn_dequeue(struct stream *strm)
} }
/* the pendconn must be dequeued now */ /* the pendconn must be dequeued now */
if (p->srv) if (p->target)
strm->target = &p->srv->obj_type; strm->target = &p->target->obj_type;
strm->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET); strm->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
strm->flags |= p->strm_flags & (SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET); strm->flags |= p->strm_flags & (SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);