From 8bf0e00b1c618b0af1d4c27e31957f34da33cd15 Mon Sep 17 00:00:00 2001
From: Johan Wikman <johan.wikman@mariadb.com>
Date: Fri, 26 Apr 2019 17:12:39 +0300
Subject: [PATCH] MXS-2329 Use duration with users_refresh_time

---
 .../Getting-Started/Configuration-Guide.md    |  8 ++++-
 server/core/config.cc                         | 36 ++++++++++---------
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/Documentation/Getting-Started/Configuration-Guide.md b/Documentation/Getting-Started/Configuration-Guide.md
index 29b3e0575..fb5177a7d 100644
--- a/Documentation/Getting-Started/Configuration-Guide.md
+++ b/Documentation/Getting-Started/Configuration-Guide.md
@@ -871,9 +871,15 @@ that can be changed. The minimum allowed value is 10 seconds. A negative
 value disables the refreshing entirelly. Note that using `maxadmin` it is
 possible to explicitly cause the users of a service to be reloaded.
 ```
-users_refresh_time=120
+users_refresh_time=120s
 ```
 
+The value is specified as documented [here](#durations). If no explicit unit
+is provided, the value is interpreted as seconds in MaxScale 2.4. In subsequent
+versions a value without a unit may be rejected. Note that since the granularity
+of the timeout is seconds, a timeout specified in milliseconds will be rejected,
+even if the duration is longer than a second.
+
 #### `retain_last_statements`
 
 How many statements MaxScale should store for each session. This is for
diff --git a/server/core/config.cc b/server/core/config.cc
index 6a69eb4da..329dda1b9 100644
--- a/server/core/config.cc
+++ b/server/core/config.cc
@@ -2609,19 +2609,26 @@ static int handle_global_item(const char* name, const char* value)
     else if (strcmp(name, CN_USERS_REFRESH_TIME) == 0)
     {
         char* endptr;
-        long users_refresh_time = strtol(value, &endptr, 0);
-        if (*endptr == '\0')
+        time_t users_refresh_time = strtol(value, &endptr, 0);
+
+        if (*endptr == '\0' && users_refresh_time < 0)
         {
-            if (users_refresh_time < 0)
+            MXS_NOTICE("Value of '%s' is less than 0, users will "
+                       "not be automatically refreshed.",
+                       CN_USERS_REFRESH_TIME);
+            // Strictly speaking they will be refreshed once every 68 years,
+            // but I just don't beleave the uptime will be that long.
+            users_refresh_time = INT32_MAX;
+        }
+        else
+        {
+            // Have to "parse" the value anew in case a suffix has been used.
+            if (!get_seconds(name, value, &users_refresh_time))
             {
-                MXS_NOTICE("Value of '%s' is less than 0, users will "
-                           "not be automatically refreshed.",
-                           CN_USERS_REFRESH_TIME);
-                // Strictly speaking they will be refreshed once every 68 years,
-                // but I just don't beleave the uptime will be that long.
-                users_refresh_time = INT32_MAX;
+                return 0;
             }
-            else if (users_refresh_time < USERS_REFRESH_TIME_MIN)
+
+            if (users_refresh_time < USERS_REFRESH_TIME_MIN)
             {
                 MXS_WARNING("%s is less than the allowed minimum value of %d for the "
                             "configuration option '%s', using the minimum value.",
@@ -2637,14 +2644,9 @@ static int handle_global_item(const char* name, const char* value)
                 // we later do arithmetic.
                 users_refresh_time = INT32_MAX;
             }
+        }
 
-            gateway.users_refresh_time = users_refresh_time;
-        }
-        else
-        {
-            MXS_ERROR("%s is an invalid value for '%s'.", value, CN_USERS_REFRESH_TIME);
-            return 0;
-        }
+        gateway.users_refresh_time = users_refresh_time;
     }
     else if (strcmp(name, CN_WRITEQ_HIGH_WATER) == 0)
     {