From 083a5151cfe2cf4d1d4c53233b0c989a642ea0c2 Mon Sep 17 00:00:00 2001 From: Luis Rojas Date: Wed, 17 Jun 2020 09:24:05 -0400 Subject: [PATCH 1/4] dispatcher : better distribution when using hash and destination is not available When a destination is selected using one of the hash algorithms (over Call-ID, From, etc.) and destination is not available, then the next is selected. This causes the following problem : All messages to the unavailable destination will be sent to the same unique destination. This will cause that one destination will receive double load and the rest will continue with the same. Solution : To better distribute messages, while also guaranteeing the same distribution of the hash, a re-hash of the hash is made. As the rehash could select the same or other failed destination, it can rehash several times. The number of rehashes made is controlled by : modparam("dispatcher", "ds_rehash_max", -1) Which can take the following values : -1 : The total count of destinations is used 0 : Disabled. Reverts to original implementation. Positive number. Maximum number of rehashes. --- src/modules/dispatcher/dispatch.c | 49 ++++++++++++++++++++++++++++- src/modules/dispatcher/dispatcher.c | 2 ++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/modules/dispatcher/dispatch.c b/src/modules/dispatcher/dispatch.c index 66b35742e54..23adc0efc4c 100644 --- a/src/modules/dispatcher/dispatch.c +++ b/src/modules/dispatcher/dispatch.c @@ -107,6 +107,7 @@ extern int ds_ping_latency_stats; extern float ds_latency_estimator_alpha; extern int ds_attrs_none; extern param_t *ds_db_extra_attrs_list; +extern int ds_rehash_max; extern int ds_load_mode; static db_func_t ds_dbf; @@ -2099,30 +2100,40 @@ int ds_manage_routes(sip_msg_t *msg, ds_select_state_t *rstate) LM_DBG("set [%d]\n", rstate->setid); hash = 0; + int maxRehash = 0; + int fullHash; switch(rstate->alg) { case DS_ALG_HASHCALLID: /* 0 - hash call-id */ if(ds_hash_callid(msg, &hash) != 0) { LM_ERR("can't get callid hash\n"); return -1; } + maxRehash = ds_rehash_max; + fullHash = hash; break; case DS_ALG_HASHFROMURI: /* 1 - hash from-uri */ if(ds_hash_fromuri(msg, &hash) != 0) { LM_ERR("can't get From uri hash\n"); return -1; } + maxRehash = ds_rehash_max; + fullHash = hash; break; case DS_ALG_HASHTOURI: /* 2 - hash to-uri */ if(ds_hash_touri(msg, &hash) != 0) { LM_ERR("can't get To uri hash\n"); return -1; } + maxRehash = ds_rehash_max; + fullHash = hash; break; case DS_ALG_HASHRURI: /* 3 - hash r-uri */ if(ds_hash_ruri(msg, &hash) != 0) { LM_ERR("can't get ruri hash\n"); return -1; } + maxRehash = ds_rehash_max; + fullHash = hash; break; case DS_ALG_ROUNDROBIN: /* 4 - round robin */ hash = idx->last; @@ -2145,6 +2156,8 @@ int ds_manage_routes(sip_msg_t *msg, ds_select_state_t *rstate) LM_ERR("can't get authorization hash\n"); return -1; } + maxRehash = ds_rehash_max; + fullHash = hash; break; case DS_ALG_RANDOM: /* 6 - random selection */ hash = kam_rand(); @@ -2154,6 +2167,8 @@ int ds_manage_routes(sip_msg_t *msg, ds_select_state_t *rstate) LM_ERR("can't get PV hash\n"); return -1; } + maxRehash = ds_rehash_max; + fullHash = hash; break; case DS_ALG_SERIAL: /* 8 - use always first entry */ hash = 0; @@ -2215,7 +2230,39 @@ int ds_manage_routes(sip_msg_t *msg, ds_select_state_t *rstate) if(ds_use_default != 0 && idx->nr != 1) i = (i + 1) % (idx->nr - 1); else - i = (i + 1) % idx->nr; + { /* Should try to rehash, and at most maxRehash times*/ + if ( maxRehash < 0 ) maxRehash = idx->nr; + if ( maxRehash > 0 ) + { + char fullhashStr[3*sizeof(int) + 2]; + do + { + str cid; + cid.len = snprintf (fullhashStr, sizeof(fullhashStr), "%d", fullHash ); + if ( cid.len >= sizeof(fullhashStr)) + { + cid.len = sizeof(fullhashStr); + } + cid.s = fullhashStr; + fullHash = ds_get_hash(&cid, NULL); + maxRehash--; + } + while (( maxRehash > 0 ) && ds_skip_dst(idx->dlist[fullHash % idx->nr].flags ) ); + if ( 0 == maxRehash ) /* Acts as if no rehash had been done */ + { + i = (i + 1) % idx->nr; + } + else + { + i = fullHash % idx->nr; + } + + } + else + { + i = (i + 1) % idx->nr; + } + } if(i == hash) { /* back to start -- looks like no active dst */ if(ds_use_default != 0) { diff --git a/src/modules/dispatcher/dispatcher.c b/src/modules/dispatcher/dispatcher.c index 836038442f8..3ecb7c29f77 100644 --- a/src/modules/dispatcher/dispatcher.c +++ b/src/modules/dispatcher/dispatcher.c @@ -121,6 +121,7 @@ int ds_timer_mode = 0; int ds_attrs_none = 0; int ds_load_mode = 0; +int ds_rehash_max = -1; /* Number of times trying rehash, to find an active destination. 0 : disable, -1 : Total number of destinations */ str ds_outbound_proxy = STR_NULL; /* tm */ @@ -289,6 +290,7 @@ static param_export_t params[]={ {"ds_db_extra_attrs", PARAM_STR, &ds_db_extra_attrs}, {"ds_load_mode", PARAM_INT, &ds_load_mode}, {"reload_delta", PARAM_INT, &ds_reload_delta }, + {"ds_rehash_max", PARAM_INT, &ds_rehash_max }, {0,0,0} }; From fdd623a50fedd4312f8324cf8796ef00eed12af5 Mon Sep 17 00:00:00 2001 From: Luis Rojas Date: Wed, 17 Jun 2020 11:04:04 -0400 Subject: [PATCH 2/4] dispatcher: Fixs rehash of has when ds_rehash_max is 1. In the rehash loop, when it exits because the counter maxRehash reaches 0. it was ignoring the current value of fullHash, which could be successful. Now it check status of destination related to current fullHash, not the counter --- src/modules/dispatcher/dispatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/dispatcher/dispatch.c b/src/modules/dispatcher/dispatch.c index 23adc0efc4c..139212b66a5 100644 --- a/src/modules/dispatcher/dispatch.c +++ b/src/modules/dispatcher/dispatch.c @@ -2248,7 +2248,7 @@ int ds_manage_routes(sip_msg_t *msg, ds_select_state_t *rstate) maxRehash--; } while (( maxRehash > 0 ) && ds_skip_dst(idx->dlist[fullHash % idx->nr].flags ) ); - if ( 0 == maxRehash ) /* Acts as if no rehash had been done */ + if ( ds_skip_dst(idx->dlist[fullHash % idx->nr].flags ) ) /* Acts as if no rehash had been done */ { i = (i + 1) % idx->nr; } From ce177c134f00cd1509313b50163849f736b6e0a9 Mon Sep 17 00:00:00 2001 From: Luis Rojas Date: Wed, 17 Jun 2020 13:27:54 -0400 Subject: [PATCH 3/4] dispatcher : FIX. Previous implementation of rehash did not consider case when "use_default" flag is active. --- src/modules/dispatcher/dispatch.c | 71 +++++++++++++++---------------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/src/modules/dispatcher/dispatch.c b/src/modules/dispatcher/dispatch.c index 139212b66a5..9afbc2cd5a6 100644 --- a/src/modules/dispatcher/dispatch.c +++ b/src/modules/dispatcher/dispatch.c @@ -2219,50 +2219,49 @@ int ds_manage_routes(sip_msg_t *msg, ds_select_state_t *rstate) LM_DBG("using alg [%d] hash [%u]\n", rstate->alg, hash); + int listSize; if(ds_use_default != 0 && idx->nr != 1) - hash = hash % (idx->nr - 1); + listSize = idx->nr - 1; else - hash = hash % idx->nr; + listSize = idx->nr; + hash = hash % listSize; i = hash; - /* if selected address is inactive, find next active */ - while(ds_skip_dst(idx->dlist[i].flags)) { - if(ds_use_default != 0 && idx->nr != 1) - i = (i + 1) % (idx->nr - 1); + if ( maxRehash < 0 ) { + if(ds_use_default != 0 ) + maxRehash = listSize - 1; else - { /* Should try to rehash, and at most maxRehash times*/ - if ( maxRehash < 0 ) maxRehash = idx->nr; - if ( maxRehash > 0 ) - { - char fullhashStr[3*sizeof(int) + 2]; - do - { - str cid; - cid.len = snprintf (fullhashStr, sizeof(fullhashStr), "%d", fullHash ); - if ( cid.len >= sizeof(fullhashStr)) - { - cid.len = sizeof(fullhashStr); - } - cid.s = fullhashStr; - fullHash = ds_get_hash(&cid, NULL); - maxRehash--; - } - while (( maxRehash > 0 ) && ds_skip_dst(idx->dlist[fullHash % idx->nr].flags ) ); - if ( ds_skip_dst(idx->dlist[fullHash % idx->nr].flags ) ) /* Acts as if no rehash had been done */ - { - i = (i + 1) % idx->nr; - } - else - { - i = fullHash % idx->nr; - } - - } - else + maxRehash = listSize; + } + + /* if selected address is inactive, find one active */ + + while(ds_skip_dst(idx->dlist[i].flags)) { + /* if alg was a hash, it should try to rehash first, and at most maxRehash times*/ + if ( maxRehash > 0 ) { + char fullhashStr[3*sizeof(int) + 2]; + do + { + str cid; + cid.len = snprintf (fullhashStr, sizeof(fullhashStr), "%d", fullHash ); + if ( cid.len >= sizeof(fullhashStr)) { - i = (i + 1) % idx->nr; + cid.len = sizeof(fullhashStr); } + cid.s = fullhashStr; + fullHash = ds_get_hash(&cid, NULL); + maxRehash--; } + while (( maxRehash > 0 ) && ds_skip_dst(idx->dlist[fullHash % listSize].flags ) ); + if ( ds_skip_dst(idx->dlist[fullHash % listSize].flags ) ) /* Acts as if no rehash had been done */ + /* original implementaion : find next active */ + i = (i + 1) % listSize; + else + i = fullHash % listSize; + } + else /* original implementaion : find next active */ + i = (i + 1) % listSize; + if(i == hash) { /* back to start -- looks like no active dst */ if(ds_use_default != 0) { From 33bdabc77905b6b056022fa7eec560c0163f09ef Mon Sep 17 00:00:00 2001 From: Luis Rojas Date: Sun, 28 Jun 2020 01:35:30 -0400 Subject: [PATCH 4/4] Default mode is fully compatible with previous implementation. Default mode "Doesn nothing" --- src/modules/dispatcher/dispatcher.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/modules/dispatcher/dispatcher.c b/src/modules/dispatcher/dispatcher.c index 3ecb7c29f77..7793b164798 100644 --- a/src/modules/dispatcher/dispatcher.c +++ b/src/modules/dispatcher/dispatcher.c @@ -121,7 +121,9 @@ int ds_timer_mode = 0; int ds_attrs_none = 0; int ds_load_mode = 0; -int ds_rehash_max = -1; /* Number of times trying rehash, to find an active destination. 0 : disable, -1 : Total number of destinations */ +int ds_rehash_max = 0; /* Number of times trying rehash, to find an active destination. +/* Default, 0 : disabled. to ensure backward compatibility with running enviroments +-1 : Total number of destinations */ str ds_outbound_proxy = STR_NULL; /* tm */