From 609912ae90fbe12d578dc63713614762748ba587 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Thu, 3 Mar 2016 23:22:16 +0200
Subject: [PATCH] Fixed readwritesplit server weight calculations

The server weights were ignored for the first connection and servers with a
weight of 0 would get connections if the connection count was high enough.

This fixes the weighting behavior so that when connections are created,
servers with a weight of 0 will be ignored as long as there is a server
with a positive weight available.

With this change, three servers configured with weights 2, 1 and 0 would
get connections balanced in the following way:

weight = 2, 66% of connections
weight = 1, 33% of connections
weight = 0, 0% of connections

If the server with the weight of 2 would go down, the server with the weight
of 1 would get 100% of the connections. If both servers with positive weights
go down, the server with the weight of 0 would be used.
---
 .../routing/readwritesplit/readwritesplit.c   | 61 ++++++++++++++-----
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c
index 7f249176f..642bf2975 100644
--- a/server/modules/routing/readwritesplit/readwritesplit.c
+++ b/server/modules/routing/readwritesplit/readwritesplit.c
@@ -660,11 +660,11 @@ createInstance(SERVICE *service, char **options)
 
                     if (perc == 0)
                     {
-                        perc = 1;
                         MXS_ERROR("Weighting parameter '%s' with a value of %d for"
                                   " server '%s' rounds down to zero with total weight"
                                   " of %d for service '%s'. No queries will be "
-                                  "routed to this server.", weightby, wght,
+                                  "routed to this server as long as a server with"
+                                  " positive weight is available.", weightby, wght,
                                   backend->backend_server->unique_name, total,
                                   service->name);
                     }
@@ -2964,8 +2964,21 @@ int bref_cmp_router_conn(
         BACKEND* b1 = ((backend_ref_t *)bref1)->bref_backend;
         BACKEND* b2 = ((backend_ref_t *)bref2)->bref_backend;
 
-        return ((1000 * b1->backend_conn_count) / b1->weight)
-			  - ((1000 * b2->backend_conn_count) / b2->weight);
+        if (b1->weight == 0 && b2->weight == 0)
+        {
+            return b1->backend_server->stats.n_current - b2->backend_server->stats.n_current;
+        }
+        else if (b1->weight == 0)
+        {
+            return 1;
+        }
+        else if (b2->weight == 0)
+        {
+            return -1;
+        }
+
+        return ((1000 + 1000 * b1->backend_conn_count) / b1->weight)
+			  - ((1000 + 1000 * b2->backend_conn_count) / b2->weight);
 }
 
 /** Compare nunmber of global connections in backend servers */
@@ -2975,11 +2988,23 @@ int bref_cmp_global_conn(
 {
         BACKEND* b1 = ((backend_ref_t *)bref1)->bref_backend;
         BACKEND* b2 = ((backend_ref_t *)bref2)->bref_backend;
-        
-        return ((1000 * b1->backend_server->stats.n_current) / b1->weight)
-		  - ((1000 * b2->backend_server->stats.n_current) / b2->weight);
-}
 
+        if (b1->weight == 0 && b2->weight == 0)
+        {
+            return b1->backend_server->stats.n_current - b2->backend_server->stats.n_current;
+        }
+        else if (b1->weight == 0)
+        {
+            return 1;
+        }
+        else if (b2->weight == 0)
+        {
+            return -1;
+        }
+
+        return ((1000 + 1000 * b1->backend_server->stats.n_current) / b1->weight)
+		  - ((1000 + 1000 * b2->backend_server->stats.n_current) / b2->weight);
+}
 
 /** Compare relication lag between backend servers */
 int bref_cmp_behind_master(
@@ -3002,7 +3027,20 @@ int bref_cmp_current_load(
         SERVER*  s2 = ((backend_ref_t *)bref2)->bref_backend->backend_server;
         BACKEND* b1 = ((backend_ref_t *)bref1)->bref_backend;
         BACKEND* b2 = ((backend_ref_t *)bref2)->bref_backend;
-        
+
+        if (b1->weight == 0 && b2->weight == 0)
+        {
+            return b1->backend_server->stats.n_current - b2->backend_server->stats.n_current;
+        }
+        else if (b1->weight == 0)
+        {
+            return 1;
+        }
+        else if (b2->weight == 0)
+        {
+            return -1;
+        }
+
         return ((1000 * s1->stats.n_current_ops) - b1->weight)
 			- ((1000 * s2->stats.n_current_ops) - b2->weight);
 }
@@ -3291,11 +3329,6 @@ static bool select_connect_backend_servers(
         {
                 BACKEND* b = backend_ref[i].bref_backend;
 
-		if (router->servers[i]->weight == 0)
-		{
-			continue;
-		}
-		
                 if (SERVER_IS_RUNNING(b->backend_server) &&
                         ((b->backend_server->status & router->bitmask) ==
                         router->bitvalue))