Using round robin for load balancing if hash key is empty

[Roman Arutyunyan]

Hello Niklas,

On Wed, May 22, 2019 at 09:59:46PM +0200, Niklas Keller wrote:
> Hey,
> 
> I'd like to propose falling back to round robin if the hash key is
> empty. This allows using hashed cookie values for sticky sessions
> while using round robin for all requests that do not carry any session
> information and thus do not need the stickiness. While Nginx Plus
> allows to use the learning mode, this patch avoids the need for
> synchronization between multiple instances.
> 
> Instead of falling back to round robin automatically, this could also
> be changed to offer a configuration option, but I think it's a
> sensible default.
> 
> An additional configuration option could be added in the future to
> change the fallback from round robin to another node selection
> strategy.
> 
> You can find the patch at the end of this email.
> 
> Kind regards,
> Niklas
> 
> # HG changeset patch
> # User Niklas Keller <me at kelunik.com>
> # Date 1558554036 -7200
> #      Wed May 22 21:40:36 2019 +0200
> # Node ID 34015c26b7fe9bda83390d25d989acb109c8d1ea
> # Parent  234373adb2ce6023e69e527cbf2b60adf70b1130
> Use round-robin if hash key is empty
> 
> diff -r 234373adb2ce -r 34015c26b7fe
> src/http/modules/ngx_http_upstream_hash_module.c
> --- a/src/http/modules/ngx_http_upstream_hash_module.c  Tue May 21
> 17:23:57 2019 +0300
> +++ b/src/http/modules/ngx_http_upstream_hash_module.c  Wed May 22
> 21:40:36 2019 +0200
> @@ -178,7 +178,7 @@
> 
>      ngx_http_upstream_rr_peers_rlock(hp->rrp.peers);
> 
> -    if (hp->tries > 20 || hp->rrp.peers->single) {
> +    if (hp->tries > 20 || hp->rrp.peers->single || hp->key.len == 0) {
>          ngx_http_upstream_rr_peers_unlock(hp->rrp.peers);
>          return hp->get_rr_peer(pc, &hp->rrp);
>      }

The proposition sounds reasonable.  As for the patch, it makes sense to add
the same condition to the consistent hash balancer and also make similar
changes in the stream module.

Attached is a modified patch with those changes.  I also slightly changed
the commit log.  Please confirm the patch looks good for you.

-- 
Roman Arutyunyan
-------------- next part --------------
# HG changeset patch
# User Niklas Keller <me at kelunik.com>
# Date 1558614458 -10800
#      Thu May 23 15:27:38 2019 +0300
# Node ID 34dff919b948dd9652b24459e6a40c303058df09
# Parent  234373adb2ce6023e69e527cbf2b60adf70b1130
Upstream hash: fall back to round-robin if hash key is empty.

diff --git a/src/http/modules/ngx_http_upstream_hash_module.c b/src/http/modules/ngx_http_upstream_hash_module.c
--- a/src/http/modules/ngx_http_upstream_hash_module.c
+++ b/src/http/modules/ngx_http_upstream_hash_module.c
@@ -178,7 +178,7 @@ ngx_http_upstream_get_hash_peer(ngx_peer
 
     ngx_http_upstream_rr_peers_rlock(hp->rrp.peers);
 
-    if (hp->tries > 20 || hp->rrp.peers->single) {
+    if (hp->tries > 20 || hp->rrp.peers->single || hp->key.len == 0) {
         ngx_http_upstream_rr_peers_unlock(hp->rrp.peers);
         return hp->get_rr_peer(pc, &hp->rrp);
     }
@@ -509,7 +509,7 @@ ngx_http_upstream_get_chash_peer(ngx_pee
 
     ngx_http_upstream_rr_peers_wlock(hp->rrp.peers);
 
-    if (hp->tries > 20 || hp->rrp.peers->single) {
+    if (hp->tries > 20 || hp->rrp.peers->single || hp->key.len == 0) {
         ngx_http_upstream_rr_peers_unlock(hp->rrp.peers);
         return hp->get_rr_peer(pc, &hp->rrp);
     }
diff --git a/src/stream/ngx_stream_upstream_hash_module.c b/src/stream/ngx_stream_upstream_hash_module.c
--- a/src/stream/ngx_stream_upstream_hash_module.c
+++ b/src/stream/ngx_stream_upstream_hash_module.c
@@ -178,7 +178,7 @@ ngx_stream_upstream_get_hash_peer(ngx_pe
 
     ngx_stream_upstream_rr_peers_rlock(hp->rrp.peers);
 
-    if (hp->tries > 20 || hp->rrp.peers->single) {
+    if (hp->tries > 20 || hp->rrp.peers->single || hp->key.len == 0) {
         ngx_stream_upstream_rr_peers_unlock(hp->rrp.peers);
         return hp->get_rr_peer(pc, &hp->rrp);
     }
@@ -511,7 +511,7 @@ ngx_stream_upstream_get_chash_peer(ngx_p
 
     ngx_stream_upstream_rr_peers_wlock(hp->rrp.peers);
 
-    if (hp->tries > 20 || hp->rrp.peers->single) {
+    if (hp->tries > 20 || hp->rrp.peers->single || hp->key.len == 0) {
         ngx_stream_upstream_rr_peers_unlock(hp->rrp.peers);
         return hp->get_rr_peer(pc, &hp->rrp);
     }