On 2013/8/6 20:36, Willy Tarreau wrote: > Hi Godbach, > > Sorry for replying late, I don't have a regular internet access at the > moment. > > On Tue, Aug 06, 2013 at 07:48:46PM +0800, Godbach wrote: >> It seems that static-rr should also use the same check condition for the >> server after being selected as roundrobin and leastconn as below: >> >> if (!srv->maxconn || (!srv->nbpend && srv->served < >> srv_dynamic_maxconn(srv))) >> >> not the following: >> >> if (!srv->maxconn || srv->cur_sess < srv_dynamic_maxconn(srv)) { > > Yes I agree with you, I think the reason is that the code was > extracted from the hash codes (which also fall back to RR if > the content to hash is not here), and which initially did not > need to handle this situation. > > I don't remember if the hash algorithms now fall back to the > generic RR code or if they use their own, but it is possible > that more code needs fixing in the end. > > Care to send a patch ? :-) > > Best regards, > Willy > >
Hi Willy, Thank you for your replying. I have checked all the hash algorithms, if the hash type is consistent, they will call chash_get_server_hash(), and map_get_server_hash() will be called with hash type is map-based. Both functions will return server directly without checking server limition. The check will be executed in assign_server_and_queue() as below: err = assign_server(s); ... switch (err) { ... case SRV_STATUS_OK: if (srv->maxconn && (srv->nbpend || srv->served >= srv_dynamic_maxconn(srv))) { if (srv->maxqueue > 0 && srv->nbpend >= srv->maxqueue) return SRV_STATUS_FULL; p = pendconn_add(s); if (p) return SRV_STATUS_QUEUED; else return SRV_STATUS_INTERNAL; } ... So in my opinion, it is not neccessary to worry about hash algorithms. The attachment is the patch which fix the check condition for static-rr in map_get_server_rr() for your information. This patch will only affect static-rr. Though all hash algorithms with type map-based will use the same server map as static-rr, they call another function map_get_server_hash() to get server. By the way, it seems that hash type avalanche was not used anymore since there are only definition and parsing for it in lastest snapshot as below: src/cfgparse.c 4101: curproxy->lbprm.algo |= BE_LB_HASH_AVAL; include/types/backend.h 108:#define BE_LB_HASH_AVAL 0x200000 /* run an avalanche hash before a map */ Is there anything I missed? -- Best Regards, Godbach
From 370a74e89af0153a96ed8b7ebd4648258c89109e Mon Sep 17 00:00:00 2001 From: Godbach <nylzhao...@gmail.com> Date: Wed, 7 Aug 2013 09:48:23 +0800 Subject: [PATCH] BUG/MINOR: use the same check condition for server as other algorithms Such load balance algorithms as roundrobin, leastconn and first will check the server after being selected with the following condition: if (!s->maxconn || (!s->nbpend && s->served < srv_dynamic_maxconn(s))) But static-rr uses the different one in map_get_server_rr() as below: if (!srv->maxconn || srv->cur_sess < srv_dynamic_maxconn(srv)) After viewing this difference, it is a better choice for static-rr to use the same check condition as other algorithms. This change will only affect static-rr. Though all hash algorithms with type map-based will use the same server map as static-rr, they call another function map_get_server_hash() to get server. Signed-off-by: Godbach <nylzhao...@gmail.com> --- src/lb_map.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/lb_map.c b/src/lb_map.c index 49805ad..9858249 100644 --- a/src/lb_map.c +++ b/src/lb_map.c @@ -229,7 +229,7 @@ struct server *map_get_server_rr(struct proxy *px, struct server *srvtoavoid) avoididx = 0; /* shut a gcc warning */ do { srv = px->lbprm.map.srv[newidx++]; - if (!srv->maxconn || srv->cur_sess < srv_dynamic_maxconn(srv)) { + if (!srv->maxconn || (!srv->nbpend && srv->served < srv_dynamic_maxconn(srv))) { /* make sure it is not the server we are try to exclude... */ if (srv != srvtoavoid) { px->lbprm.map.rr_idx = newidx; -- 1.7.7