MXS-2329 Use durations with auth_[connect|read|write]_timeout

Documentation/Getting-Started/Configuration-Guide.md

@@ -336,9 +336,15 @@ backend server before aborting the authentication process. The default is 3
 seconds.
 
 ```
-auth_connect_timeout=10
+auth_connect_timeout=10s
 ```
 
+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.
+
 #### `auth_read_timeout`
 
 The read timeout in seconds for the MySQL connection to the backend database
@@ -350,9 +356,15 @@ connection to the backend servers is slow, it is a good idea to increase this
 value. The default is 1 second.
 
 ```
-auth_read_timeout=10
+auth_read_timeout=10s
 ```
 
+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.
+
 #### `auth_write_timeout`
 
 The write timeout in seconds for the MySQL connection to the backend database
@@ -360,9 +372,15 @@ when user authentication data is fetched. Currently MariaDB MaxScale does not
 write or modify the data in the backend server. The default is 2 seconds.
 
 ```
-auth_write_timeout=10
+auth_write_timeout=10s
 ```
 
+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.
+
 #### `query_retries`
 
 The number of times an interrupted internal query will be retried. The default

include/maxscale/config.hh

@@ -673,7 +673,7 @@ inline bool config_is_valid_name(const std::string& name, std::string* reason =
 bool check_path_parameter(const MXS_MODULE_PARAM* params, const char* value);
 
 /**
- * Converts a string into a duration, intepreting in a case-insensitive manner
+ * Converts a string into milliseconds, intepreting in a case-insensitive manner
  * an 'h'-suffix to indicate hours, an 'm'-suffix to indicate minutes, an
  * 's'-suffix to indicate seconds and an 'ms'-suffix to indicate milliseconds.
  *
@@ -690,3 +690,56 @@ bool get_suffixed_duration(const char* zValue,
                            mxs::config::DurationInterpretation interpretation,
                            std::chrono::milliseconds* pDuration,
                            mxs::config::DurationUnit* pUnit = nullptr);
+
+/**
+ * Converts a string into milliseconds, intepreting in a case-insensitive manner
+ * an 'h'-suffix to indicate hours, an 'm'-suffix to indicate minutes, an
+ * 's'-suffix to indicate seconds and an 'ms'-suffix to indicate milliseconds.
+ *
+ * A value lacking a specific suffix will be interpreted as milliseconds.
+ *
+ * @param zValue     A numerical string, possibly suffixed by 'h', 'm',
+ *                   's' or 'ms'.
+ * @param pDuration  Pointer, if non-NULL, where the result is stored.
+ * @param pUnit      Pointer, if non-NULL, where the detected unit is stored.
+ *
+ * @return True on success, false on invalid input in which case @c pUnit and
+ *         @c pDuration will not be modified.
+ */
+inline bool get_suffixed_duration(const char* zValue,
+                                  std::chrono::milliseconds* pDuration,
+                                  mxs::config::DurationUnit* pUnit = nullptr)
+{
+    return get_suffixed_duration(zValue, mxs::config::INTERPRET_AS_MILLISECONDS, pDuration, pUnit);
+}
+
+/**
+ * Converts a string into seconds, intepreting in a case-insensitive manner
+ * an 'h'-suffix to indicate hours, an 'm'-suffix to indicate minutes, an
+ * 's'-suffix to indicate seconds and an 'ms'-suffix to indicate milliseconds.
+ *
+ * A value lacking a specific suffix will be interpreted as seconds.
+ *
+ * @param zValue     A numerical string, possibly suffixed by 'h', 'm',
+ *                   's' or 'ms'.
+ * @param pDuration  Pointer, if non-NULL, where the result is stored.
+ * @param pUnit      Pointer, if non-NULL, where the detected unit is stored.
+ *
+ * @return True on success, false on invalid input in which case @c pUnit and
+ *         @c pDuration will not be modified.
+ */
+inline bool get_suffixed_duration(const char* zValue,
+                                  std::chrono::seconds* pDuration,
+                                  mxs::config::DurationUnit* pUnit = nullptr)
+{
+    std::chrono::milliseconds ms;
+
+    bool rv = get_suffixed_duration(zValue, mxs::config::INTERPRET_AS_SECONDS, &ms, pUnit);
+
+    if (rv)
+    {
+        *pDuration = std::chrono::duration_cast<std::chrono::seconds>(ms);
+    }
+
+    return rv;
+}

server/core/config.cc

@@ -237,6 +237,8 @@ static pcre2_code* compile_regex_string(const char* regex_string,
                                         uint32_t options,
                                         uint32_t* output_ovector_size);
 static bool duration_is_valid(const char* zValue, mxs::config::DurationUnit* pUnit);
+static bool get_seconds(const char* zName, const char* zValue, std::chrono::seconds* pSeconds);
+static bool get_seconds(const char* zName, const char* zValue, unsigned int* pSeconds);
 
 
 int         config_get_ifaddr(unsigned char* output);
@@ -2392,43 +2394,22 @@ static int handle_global_item(const char* name, const char* value)
     }
     else if (strcmp(name, CN_AUTH_CONNECT_TIMEOUT) == 0)
     {
-        char* endptr;
+        if (!get_seconds(name, value, &gateway.auth_conn_timeout))
-        int intval = strtol(value, &endptr, 0);
-        if (*endptr == '\0' && intval > 0)
         {
-            gateway.auth_conn_timeout = intval;
-        }
-        else
-        {
-            MXS_ERROR("Invalid timeout value for 'auth_connect_timeout': %s", value);
             return 0;
         }
     }
     else if (strcmp(name, CN_AUTH_READ_TIMEOUT) == 0)
     {
-        char* endptr;
+        if (!get_seconds(name, value, &gateway.auth_read_timeout))
-        int intval = strtol(value, &endptr, 0);
-        if (*endptr == '\0' && intval > 0)
         {
-            gateway.auth_read_timeout = intval;
-        }
-        else
-        {
-            MXS_ERROR("Invalid timeout value for 'auth_read_timeout': %s", value);
             return 0;
         }
     }
     else if (strcmp(name, CN_AUTH_WRITE_TIMEOUT) == 0)
     {
-        char* endptr;
+        if (!get_seconds(name, value, &gateway.auth_write_timeout))
-        int intval = strtol(value, &endptr, 0);
-        if (*endptr == '\0' && intval > 0)
         {
-            gateway.auth_write_timeout = intval;
-        }
-        else
-        {
-            MXS_ERROR("Invalid timeout value for 'auth_write_timeout': %s", value);
             return 0;
         }
     }
@@ -5114,6 +5095,53 @@ static bool duration_is_valid(const char* zValue, mxs::config::DurationUnit* pUn
     return get_suffixed_duration(zValue, mxs::config::INTERPRET_AS_SECONDS, nullptr, pUnit);
 }
 
+static bool get_seconds(const char* zName, const char* zValue, std::chrono::seconds* pSeconds)
+{
+    bool valid = false;
+
+    mxs::config::DurationUnit unit;
+    std::chrono::seconds seconds;
+    if (get_suffixed_duration(zValue, &seconds, &unit))
+    {
+        switch (unit)
+        {
+        case mxs::config::DURATION_IN_MILLISECONDS:
+            MXS_ERROR("Currently the granularity of `%s` is seconds. The value cannot "
+                      "be specified in milliseconds.", zName);
+            valid = false;
+            break;
+
+        case mxs::config::DURATION_IN_DEFAULT:
+            MXS_WARNING("Specifying durations without a suffix denoting the unit "
+                        "has been deprecated: %s=%s. Use the suffixes 'h' (hour), "
+                        "'m' (minute) 's' (second) or 'ms' (milliseconds).", zName, zValue);
+        default:
+            *pSeconds = seconds;
+            valid = true;
+        }
+    }
+    else
+    {
+        MXS_ERROR("Invalid timeout value for '%s': %s", zName, zValue);
+    }
+
+    return valid;
+}
+
+static bool get_seconds(const char* zName, const char* zValue, unsigned int* pSeconds)
+{
+    std::chrono::seconds seconds;
+
+    bool valid = get_seconds(zName, zValue, &seconds);
+
+    if (valid)
+    {
+        *pSeconds = seconds.count();
+    }
+
+    return valid;
+}
+
 bool config_parse_disk_space_threshold(SERVER::DiskSpaceLimits* pDisk_space_threshold,
                                        const char* zDisk_space_threshold)
 {