[feat] make min_reserved_fds runtime-configurable

src/config.c

@@ -33,6 +33,7 @@
 
 #include <fcntl.h>
 #include <sys/stat.h>
+#include <sys/resource.h>
 
 /*-----------------------------------------------------------------------------
  * Config file name-value maps.
@@ -2774,6 +2775,62 @@ static int updateSighandlerEnabled(int val, int prev, const char **err) {
     return 1;
 }
 
+static int tryResizeSetAeSize(const char** err) {
+    if ((unsigned int) aeGetSetSize(server.el) <
+            server.maxclients + server.min_reserved_fds + CONFIG_FDSET_INCR)
+    {
+        if (aeResizeSetSize(server.el,
+            server.maxclients + server.min_reserved_fds + CONFIG_FDSET_INCR) == AE_ERR)
+        {
+            *err = "The event loop API used by Redis is not able to handle the specified number of clients";
+            return 0;
+        }
+    }
+    return 1;
+}
+
+/* Try to raise RLIMIT_NOFILE to fit server.maxclients + server.min_reserved_fds.
+ * Returns 1 on success (or no-op), 0 on failure with *err set.
+ * MUST NOT exit() — called from CONFIG SET hot path. */
+static int tryRaiseOpenFilesLimit(const char **err) {
+    rlim_t wanted = (rlim_t)server.maxclients + (rlim_t)server.min_reserved_fds;
+    struct rlimit limit;
+
+    if (getrlimit(RLIMIT_NOFILE, &limit) == -1) { //get limit
+        return 1;
+    }
+    if (limit.rlim_cur >= wanted) return 1;
+
+    struct rlimit new_limit = limit;
+    new_limit.rlim_cur = wanted;
+    new_limit.rlim_max = wanted;
+    if (setrlimit(RLIMIT_NOFILE, &new_limit) == -1) { //try update limit
+        static char msg[160];
+        snprintf(msg, sizeof(msg),
+            "Unable to set RLIMIT_NOFILE to %llu (current %llu): %s. "
+            "min-reserved-fds would exceed available file descriptors.",
+            (unsigned long long)wanted,
+            (unsigned long long)limit.rlim_cur,
+            strerror(errno));
+        *err = msg;
+        return 0;
+    }
+    return 1;
+}
+
+
+static int updateMinReservedFds(long long val, long long prev, const char **err) {
+    if (val > prev) {
+        if (tryRaiseOpenFilesLimit(err) == 0) {
+            return 0;
+        }
+        if (tryResizeSetAeSize(err) == 0) {
+            return 0;
+        }
+    }
+    return 1;
+}
+
 static int updateMaxclients(long long val, long long prev, const char **err) {
     /* Try to check if the OS is capable of supporting so many FDs. */
     if (val > prev) {
@@ -2788,15 +2845,8 @@ static int updateMaxclients(long long val, long long prev, const char **err) {
             }
             return 0;
         }
-        if ((unsigned int) aeGetSetSize(server.el) <
-            server.maxclients + CONFIG_FDSET_INCR)
-        {
-            if (aeResizeSetSize(server.el,
-                server.maxclients + CONFIG_FDSET_INCR) == AE_ERR)
-            {
-                *err = "The event loop API used by Redis is not able to handle the specified number of clients";
-                return 0;
-            }
+        if (tryResizeSetAeSize(err) == 0) {
+            return 0;
         }
     }
     return 1;
@@ -3119,6 +3169,7 @@ standardConfig configs[] = {
     /* Unsigned int configs */
     createUIntConfig("max-tracking-clients-to-write", NULL, MODIFIABLE_CONFIG, 1, UINT_MAX, server.max_tracking_clients_to_write, 16, INTEGER_CONFIG, NULL, NULL),
     createUIntConfig("maxclients", NULL, MODIFIABLE_CONFIG, 1, UINT_MAX, server.maxclients, 10000, INTEGER_CONFIG, NULL, updateMaxclients),
+    createUIntConfig("min-reserved-fds", NULL, MODIFIABLE_CONFIG, 32, UINT_MAX, server.min_reserved_fds, CONFIG_MIN_RESERVED_FDS, INTEGER_CONFIG, NULL, updateMinReservedFds),
 #ifdef ENABLE_SWAP
     createUIntConfig("swap-ttl-compact-expire-percentile", NULL, MODIFIABLE_CONFIG, 1, 100, server.swap_ttl_compact_expire_percentile, 99, INTEGER_CONFIG, NULL, NULL),
 #endif

src/ctrip_swap_batch.c

@@ -578,7 +578,7 @@ int swapBatchTest(int argc, char *argv[], int accurate) {
         swapRequest *out_req1, *out_req2, *utils_req;
 
         swapThreadsInit();
-        server.el = aeCreateEventLoop(server.maxclients+CONFIG_FDSET_INCR);
+        server.el = aeCreateEventLoop(server.maxclients+ server.min_reserved_fds + CONFIG_FDSET_INCR);
         asyncCompleteQueueInit();
 
         /* flush empty ctx => nop */

src/server.c

@@ -3230,13 +3230,13 @@ int setOOMScoreAdj(int process_class) {
  * max number of clients, the function will do the reverse setting
  * server.maxclients to the value that we can actually handle. */
 void adjustOpenFilesLimit(void) {
-    rlim_t maxfiles = server.maxclients+CONFIG_MIN_RESERVED_FDS;
+    rlim_t maxfiles = server.maxclients+server.min_reserved_fds;
     struct rlimit limit;
 
     if (getrlimit(RLIMIT_NOFILE,&limit) == -1) {
         serverLog(LL_WARNING,"Unable to obtain the current NOFILE limit (%s), assuming 1024 and setting the max clients configuration accordingly.",
             strerror(errno));
-        server.maxclients = 1024-CONFIG_MIN_RESERVED_FDS;
+        server.maxclients = 1024-server.min_reserved_fds;
     } else {
         rlim_t oldlimit = limit.rlim_cur;
 
@@ -3269,11 +3269,11 @@ void adjustOpenFilesLimit(void) {
 
             if (bestlimit < maxfiles) {
                 unsigned int old_maxclients = server.maxclients;
-                server.maxclients = bestlimit-CONFIG_MIN_RESERVED_FDS;
+                server.maxclients = bestlimit-server.min_reserved_fds;
                 /* maxclients is unsigned so may overflow: in order
                  * to check if maxclients is now logically less than 1
                  * we test indirectly via bestlimit. */
-                if (bestlimit <= CONFIG_MIN_RESERVED_FDS) {
+                if (bestlimit <= server.min_reserved_fds) {
                     serverLog(LL_WARNING,"Your current 'ulimit -n' "
                         "of %llu is not enough for the server to start. "
                         "Please increase your open file limit to at least "
@@ -3538,7 +3538,7 @@ void initServer(void) {
     adjustOpenFilesLimit();
     const char *clk_msg = monotonicInit();
     serverLog(LL_NOTICE, "monotonic clock: %s", clk_msg);
-    server.el = aeCreateEventLoop(server.maxclients+CONFIG_FDSET_INCR);
+    server.el = aeCreateEventLoop(server.maxclients + server.min_reserved_fds + CONFIG_FDSET_INCR);
     if (server.el == NULL) {
         serverLog(LL_WARNING,
             "Failed creating the event loop. Error message: '%s'",

src/server.h

@@ -174,7 +174,7 @@ typedef long long ustime_t; /* microsecond time type. */
  * of file descriptors we can handle are server.maxclients + RESERVED_FDS +
  * a few more to stay safe. Since RESERVED_FDS defaults to 32, we add 96
  * in order to make sure of not over provisioning more than 128 fds. */
-#define CONFIG_FDSET_INCR (CONFIG_MIN_RESERVED_FDS+96)
+#define CONFIG_FDSET_INCR 96
 
 /* OOM Score Adjustment classes. */
 #define CONFIG_OOM_MASTER 0
@@ -1578,6 +1578,7 @@ struct redisServer {
     int get_ack_from_slaves;            /* If true we send REPLCONF GETACK. */
     /* Limits */
     unsigned int maxclients;            /* Max number of simultaneous clients */
+    unsigned int min_reserved_fds;
     unsigned long long maxmemory;   /* Max number of memory bytes to use */
     ssize_t maxmemory_tracking_clients;       /* Memory limit for total tracking client buffers */
     int maxmemory_policy;           /* Policy for key eviction */

tests/swap/unit/monitor.tcl

@@ -39,4 +39,32 @@ start_server [list overrides [list ctrip-monitor-port $mport] tags "ctrip_monito
         assert_match [$r3 ping] "PONG"
     }
 
+    test {CONFIG SET min-reserved-fds accepts a valid value at runtime} {
+        # Boundary: a valid value can be set and CONFIG GET returns the same value
+        # Monitor port still serves connections (verifies the change does not break existing behavior)
+        r config set min-reserved-fds 64
+        assert_equal 64 [lindex [r config get min-reserved-fds] 1]
+        set m [redis $redis_host $mport 0 0]
+        assert_match [$m ping] "PONG"
+        # Restore
+        r config set min-reserved-fds 32
+    }
+
+    test {CONFIG SET min-reserved-fds huge value rejected without killing server} {
+        # Key regression (BUG 1): when CONFIG SET min-reserved-fds exceeds current RLIMIT_NOFILE,
+        # it must return an error and the CONFIG framework must roll back to the previous value;
+        # the server must NOT be killed by exit(1).
+        # UINT_MAX (4294967295) is used to trigger the failure path;
+        # catch + if-else keeps the test stable in the rare case where ulimit is unusually high.
+        if {[catch {r config set min-reserved-fds 4294967295} err]} {
+            # Failure path: server must still respond to PING and the previous value must remain
+            assert_equal PONG [r ping]
+            assert_equal 32 [lindex [r config get min-reserved-fds] 1]
+        } else {
+            # Rare environments where ulimit >= UINT_MAX: configuration succeeds and value must match
+            assert_equal 4294967295 [lindex [r config get min-reserved-fds] 1]
+            r config set min-reserved-fds 32
+        }
+    }
+
 }

tests/unit/limits.tcl

@@ -18,4 +18,98 @@ start_server {tags {"limits network"} overrides {maxclients 10}} {
         assert {$c > 8 && $c <= 10}
         set e
     } $expected_code
+
+    # === min-reserved-fds boundary tests ===
+    # Purpose: cover input validation and runtime regression points for CONFIG SET min-reserved-fds
+    # Defaults: default 32, lower_bound 32, upper_bound UINT_MAX
+
+    test {CONFIG SET min-reserved-fds accepts a valid value} {
+        # Boundary 1: a valid value can be set and CONFIG GET returns the same value
+        assert_equal OK [r config set min-reserved-fds 64]
+        assert_equal 64 [lindex [r config get min-reserved-fds] 1]
+        # Restore to default to avoid polluting subsequent tests
+        r config set min-reserved-fds 32
+    }
+
+    test {CONFIG SET min-reserved-fds rejects value below lower bound (32)} {
+        # Boundary 2: values below 32 must be rejected and the existing value must remain
+        assert_error "*inclusive*" {r config set min-reserved-fds 16}
+        assert_equal 32 [lindex [r config get min-reserved-fds] 1]
+    }
+
+    test {CONFIG SET min-reserved-fds rejects non-integer value} {
+        # Boundary 3: non-integer values must be rejected ("argument couldn't be parsed into an integer")
+        assert_error "*integer*" {r config set min-reserved-fds abc}
+    }
+
+    test {CONFIG SET min-reserved-fds does not modify maxclients} {
+        # Boundary 4: changing min-reserved-fds must not silently change maxclients
+        # Regression point: the original adjustOpenFilesLimit path used to mutate maxclients
+        set original_maxclients [lindex [r config get maxclients] 1]
+        r config set min-reserved-fds 64
+        assert_equal $original_maxclients [lindex [r config get maxclients] 1]
+        # Restore
+        r config set min-reserved-fds 32
+    }
+
+    test {CONFIG SET min-reserved-fds shrink succeeds} {
+        # Boundary 5: grow then shrink; CONFIG stays consistent (does not depend on ae setsize shrinking)
+        r config set min-reserved-fds 96
+        assert_equal OK [r config set min-reserved-fds 32]
+        assert_equal 32 [lindex [r config get min-reserved-fds] 1]
+    }
+
+    test {CONFIG SET min-reserved-fds huge value rejected without killing server} {
+        # Key regression (BUG 1): when CONFIG SET min-reserved-fds exceeds current RLIMIT_NOFILE,
+        # it must return an error and the CONFIG framework must roll back to the previous value;
+        # the server must NOT be killed by exit(1).
+        # UINT_MAX (4294967295) is used to trigger the failure path (far beyond any common ulimit);
+        # catch + if-else keeps the test stable in the rare case where ulimit is unusually high.
+        if {[catch {r config set min-reserved-fds 4294967295} err]} {
+            # Failure path: server must still respond to PING and the previous value must remain
+            assert_equal PONG [r ping]
+            assert_equal 32 [lindex [r config get min-reserved-fds] 1]
+        } else {
+            # Rare environments where ulimit >= UINT_MAX: configuration succeeds and value must match
+            assert_equal 4294967295 [lindex [r config get min-reserved-fds] 1]
+            r config set min-reserved-fds 32
+        }
+    }
+
+    # === maxclients boundary tests ===
+    # Purpose: cover the same risk classes that min-reserved-fds guards against,
+    # because updateMaxclients and updateMinReservedFds both rely on adjustOpenFilesLimit
+    # and share tryResizeSetAeSize.
+
+    test {CONFIG SET maxclients huge value rejected without silently reducing maxclients} {
+        # Regression for the same BUG 1 class as min-reserved-fds:
+        # adjustOpenFilesLimit may silently shrink server.maxclients when ulimit is too small,
+        # and (in extreme cases) call exit(1) when bestlimit <= server.min_reserved_fds.
+        # updateMaxclients guards against silent shrinkage by rolling server.maxclients back to prev,
+        # and the CONFIG framework also rolls back when update_fn returns 0.
+        # UINT_MAX (4294967295) triggers the failure path on any common ulimit;
+        # catch + if-else keeps the test stable in the rare case where ulimit is unusually high.
+        set original [lindex [r config get maxclients] 1]
+        if {[catch {r config set maxclients 4294967295} err]} {
+            # Failure path: maxclients must remain at its previous value
+            assert_equal $original [lindex [r config get maxclients] 1]
+            assert_match "*not able to handle*" $err
+        } else {
+            # Rare environments where ulimit >= UINT_MAX: configuration succeeds and value must match
+            assert_equal 4294967295 [lindex [r config get maxclients] 1]
+            r config set maxclients $original
+        }
+        # Server must still be alive
+        assert_equal PONG [r ping]
+    }
+
+    test {CONFIG SET maxclients smaller value accepted} {
+        # val < prev path: updateMaxclients returns 1 without touching adjustOpenFilesLimit
+        # or tryResizeSetAeSize (the ae array is not shrunk — same behavior as min-reserved-fds shrink).
+        # Verify CONFIG consistency and that the server still responds.
+        assert_equal OK [r config set maxclients 5]
+        assert_equal 5 [lindex [r config get maxclients] 1]
+        # Restore
+        r config set maxclients 10
+    }
 }