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 <[email protected]>
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 <[email protected]>
---
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
