diff --git a/src/config.c b/src/config.c index 8071139da10..54f9d31c8df 100644 --- a/src/config.c +++ b/src/config.c @@ -2988,34 +2988,46 @@ static int updateSighandlerEnabled(int val, int prev, const char **err) { 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) { - adjustOpenFilesLimit(); - if (server.maxclients != val) { - static char msg[128]; - sprintf(msg, "The operating system is not able to handle the specified number of clients, try with %d", server.maxclients); - *err = msg; - if (server.maxclients > prev) { - server.maxclients = prev; - adjustOpenFilesLimit(); - } - 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; - } - } + +static int updateFdBudget(const char *name, long long val, long long prev, const char **err) { + static char msg[256]; + unsigned long long setsize; + + if (val <= prev) return 1; + + setsize = (unsigned long long)server.maxclients + server.min_reserved_fds + CONFIG_FDSET_INCR; + if (setsize > INT_MAX) { + snprintf(msg, sizeof(msg), + "The operating system is not able to handle the specified number of %s, " + "maxclients + min-reserved-fds is too large", name); + *err = msg; + return 0; + } + if (adjustOpenFilesLimit(0) == C_ERR) { + snprintf(msg, sizeof(msg), + "The operating system is not able to handle the specified number of %s, " + "increase 'ulimit -n' (see server log for details)", name); + *err = msg; + return 0; + } + if ((unsigned long long)aeGetSetSize(server.el) < setsize && + aeResizeSetSize(server.el, (int)setsize) == AE_ERR) { + snprintf(msg, sizeof(msg), + "The event loop API used by Redis is not able to handle the specified number of %s", name); + *err = msg; + return 0; } return 1; } +static int updateMaxclients(long long val, long long prev, const char **err) { + return updateFdBudget("clients", val, prev, err); +} + +static int updateMinReservedFds(long long val, long long prev, const char **err) { + return updateFdBudget("reserved file descriptors", val, prev, err); +} + static int updateOOMScoreAdj(int val, int prev, const char **err) { UNUSED(prev); @@ -3360,7 +3372,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, IMMUTABLE_CONFIG, 32, UINT_MAX, server.min_reserved_fds, CONFIG_MIN_RESERVED_FDS, INTEGER_CONFIG, NULL, NULL), + 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 diff --git a/src/server.c b/src/server.c index f2594ac7740..d6348aef418 100644 --- a/src/server.c +++ b/src/server.c @@ -3220,85 +3220,57 @@ int setOOMScoreAdj(int process_class) { /* This function will try to raise the max number of open files accordingly to * the configured max number of clients. It also reserves a number of file - * descriptors (CONFIG_MIN_RESERVED_FDS) for extra operations of - * persistence, listening sockets, log files and so forth. + * descriptors (server.min_reserved_fds) for extra operations of + * persistence, listening sockets, log files, rocksdb and so forth. * - * If it will not be possible to set the limit accordingly to the configured - * 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+server.min_reserved_fds; + * server.maxclients is never modified. If the limit can't be raised to + * maxclients + min_reserved_fds, the server exits when 'exit_on_failure' is + * true (startup), otherwise (runtime CONFIG SET) C_ERR is returned and the + * caller is expected to roll back the config. In all the other cases C_OK + * is returned. */ +int adjustOpenFilesLimit(int exit_on_failure) { + /* Compute in 64 bit: both operands are unsigned int and may wrap. */ + rlim_t maxfiles = (rlim_t)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-server.min_reserved_fds; - } else { - rlim_t oldlimit = limit.rlim_cur; - - /* Set the max number of files if the current limit is not enough - * for our needs. */ - if (oldlimit < maxfiles) { - rlim_t bestlimit; - int setrlimit_error = 0; - - /* Try to set the file limit to match 'maxfiles' or at least - * to the higher value supported less than maxfiles. */ - bestlimit = maxfiles; - while(bestlimit > oldlimit) { - rlim_t decr_step = 16; - - limit.rlim_cur = bestlimit; - limit.rlim_max = bestlimit; - if (setrlimit(RLIMIT_NOFILE,&limit) != -1) break; - setrlimit_error = errno; - - /* We failed to set file limit to 'bestlimit'. Try with a - * smaller limit decrementing by a few FDs per iteration. */ - if (bestlimit < decr_step) break; - bestlimit -= decr_step; - } + serverLog(LL_WARNING,"Unable to obtain the current NOFILE limit (%s).%s", + strerror(errno), exit_on_failure ? " Exiting." : ""); + if (!exit_on_failure) return C_ERR; + exit(1); + } - /* Assume that the limit we get initially is still valid if - * our last try was even lower. */ - if (bestlimit < oldlimit) bestlimit = oldlimit; - - if (bestlimit < maxfiles) { - unsigned int old_maxclients = server.maxclients; - 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 <= 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 " - "%llu. Exiting.", - (unsigned long long) oldlimit, - (unsigned long long) maxfiles); - exit(1); - } - serverLog(LL_WARNING,"You requested maxclients of %d " - "requiring at least %llu max file descriptors.", - old_maxclients, - (unsigned long long) maxfiles); - serverLog(LL_WARNING,"Server can't set maximum open files " - "to %llu because of OS error: %s.", - (unsigned long long) maxfiles, strerror(setrlimit_error)); - serverLog(LL_WARNING,"Current maximum open files is %llu. " - "maxclients has been reduced to %d to compensate for " - "low ulimit. " - "If you need higher maxclients increase 'ulimit -n'.", - (unsigned long long) bestlimit, server.maxclients); - } else { - serverLog(LL_NOTICE,"Increased maximum number of open files " - "to %llu (it was originally set to %llu).", - (unsigned long long) maxfiles, - (unsigned long long) oldlimit); - } - } + rlim_t oldlimit = limit.rlim_cur; + rlim_t oldmax = limit.rlim_max; + + /* RLIM_INFINITY is the max rlim_t value, so it never enters here. */ + if (oldlimit >= maxfiles) return C_OK; + + /* Only raise the soft limit. The hard limit is raised only when it is + * lower than needed and is never lowered: an unprivileged process can't + * raise it back, which would block later runtime increases. */ + limit.rlim_cur = maxfiles; + if (oldmax != RLIM_INFINITY && oldmax < maxfiles) limit.rlim_max = maxfiles; + + if (setrlimit(RLIMIT_NOFILE,&limit) == -1) { + serverLog(LL_WARNING,"Can't set maximum open files to %llu " + "(maxclients %u + min-reserved-fds %u), current soft limit is " + "%llu and hard limit is %llu: %s. Please increase 'ulimit -n' " + "or decrease maxclients / min-reserved-fds.%s", + (unsigned long long) maxfiles, + server.maxclients, server.min_reserved_fds, + (unsigned long long) oldlimit, + (unsigned long long) oldmax, + strerror(errno), + exit_on_failure ? " Exiting." : ""); + if (!exit_on_failure) return C_ERR; + exit(1); } + + serverLog(LL_NOTICE,"Increased maximum number of open files " + "to %llu (it was originally set to %llu).", + (unsigned long long) maxfiles, (unsigned long long) oldlimit); + return C_OK; } /* Check that server.tcp_backlog can be actually enforced in Linux according @@ -3532,10 +3504,18 @@ void initServer(void) { } createSharedObjects(); - adjustOpenFilesLimit(); + adjustOpenFilesLimit(1); const char *clk_msg = monotonicInit(); serverLog(LL_NOTICE, "monotonic clock: %s", clk_msg); - server.el = aeCreateEventLoop(server.maxclients + server.min_reserved_fds + CONFIG_FDSET_INCR); + unsigned long long setsize = (unsigned long long)server.maxclients + + server.min_reserved_fds + CONFIG_FDSET_INCR; + if (setsize > INT_MAX) { + serverLog(LL_WARNING, + "maxclients (%u) + min-reserved-fds (%u) is too large for the event loop. Exiting.", + server.maxclients, server.min_reserved_fds); + exit(1); + } + server.el = aeCreateEventLoop((int)setsize); if (server.el == NULL) { serverLog(LL_WARNING, "Failed creating the event loop. Error message: '%s'", diff --git a/src/server.h b/src/server.h index 326a3fbb36e..ee01a76efb8 100644 --- a/src/server.h +++ b/src/server.h @@ -2407,7 +2407,7 @@ int htNeedsResize(dict *dict); void populateCommandTable(void); void resetCommandTableStats(void); void resetErrorTableStats(void); -void adjustOpenFilesLimit(void); +int adjustOpenFilesLimit(int exit_on_failure); void incrementErrorCount(const char *fullerr, size_t namelen); void closeListeningSockets(int unlink_unix_socket); void updateCachedTime(int update_daylight_info); diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl index f8cd72e961f..6ae57b70e40 100644 --- a/tests/unit/introspection.tcl +++ b/tests/unit/introspection.tcl @@ -200,7 +200,6 @@ start_server {tags {"introspection"}} { swap-cuckoo-filter-estimated-keys ctrip-monitor-port swap-persist-enabled - min-reserved-fds } if {!$::tls} { diff --git a/tests/unit/limits.tcl b/tests/unit/limits.tcl index 51122e8f5b3..55cae9606b6 100644 --- a/tests/unit/limits.tcl +++ b/tests/unit/limits.tcl @@ -19,3 +19,196 @@ start_server {tags {"limits network"} overrides {maxclients 10}} { set e } $expected_code } + +start_server {tags {"limits network"} overrides {maxclients 100 min-reserved-fds 64}} { + test {min-reserved-fds is loaded from the config file} { + assert_equal [lindex [r config get min-reserved-fds] 1] 64 + } + + test {CONFIG SET min-reserved-fds same value and decrease keep the server up} { + assert_equal [r config set min-reserved-fds 64] {OK} + assert_equal [lindex [r config get min-reserved-fds] 1] 64 + assert_equal [r config set min-reserved-fds 32] {OK} + assert_equal [lindex [r config get min-reserved-fds] 1] 32 + assert_equal [r ping] {PONG} + } + + test {CONFIG SET min-reserved-fds increase applies when the fd budget allows} { + set soft unlimited + catch {set soft [string trim [exec sh -c {ulimit -Sn}]]} + # 32 -> 64 stays inside the limit established at startup. A further + # increase resizes the event loop and is skipped when the soft limit + # cannot cover maxclients + the new reservation. + if {$soft ne "unlimited" && $soft < (100 + 128)} { + assert_equal [r config set min-reserved-fds 64] {OK} + assert_equal [lindex [r config get min-reserved-fds] 1] 64 + } else { + assert_equal [r config set min-reserved-fds 128] {OK} + assert_equal [lindex [r config get min-reserved-fds] 1] 128 + assert_equal [r config set min-reserved-fds 64] {OK} + } + assert_equal [lindex [r config get min-reserved-fds] 1] 64 + assert_equal [r ping] {PONG} + } + + test {CONFIG SET min-reserved-fds above the event-loop limit rolls back} { + set cur [lindex [r config get min-reserved-fds] 1] + assert_error "*not able to handle*" { + r config set min-reserved-fds 2147483647 + } + assert_equal [lindex [r config get min-reserved-fds] 1] $cur + assert_equal [r ping] {PONG} + } + + test {CONFIG SET min-reserved-fds UINT_MAX does not wrap around} { + set cur [lindex [r config get min-reserved-fds] 1] + assert_error "*not able to handle*" {r config set min-reserved-fds 4294967295} + assert_equal [lindex [r config get min-reserved-fds] 1] $cur + assert_equal [r ping] {PONG} + } + + test {CONFIG SET min-reserved-fds below the minimum is rejected} { + set cur [lindex [r config get min-reserved-fds] 1] + assert_error "*between 32 and*" {r config set min-reserved-fds 31} + assert_equal [lindex [r config get min-reserved-fds] 1] $cur + } + + test {CONFIG SET maxclients to the same or a smaller value keeps the server up} { + assert_equal [r config set maxclients 100] {OK} + assert_equal [lindex [r config get maxclients] 1] 100 + assert_equal [r config set maxclients 50] {OK} + assert_equal [lindex [r config get maxclients] 1] 50 + assert_equal [r ping] {PONG} + } + + test {CONFIG SET maxclients increase applies when the fd budget allows} { + set soft unlimited + catch {set soft [string trim [exec sh -c {ulimit -Sn}]]} + # Startup only clamps RLIMIT_NOFILE when the soft limit is below + # maxclients + min-reserved-fds. Above that, raising back past the + # original maxclients also resizes the event loop. + if {$soft ne "unlimited" && $soft < (200 + 64)} { + assert_equal [r config set maxclients 100] {OK} + assert_equal [lindex [r config get maxclients] 1] 100 + } else { + assert_equal [r config set maxclients 200] {OK} + assert_equal [lindex [r config get maxclients] 1] 200 + } + assert_equal [r ping] {PONG} + } + + test {CONFIG SET maxclients above the event-loop limit rolls back} { + set cur [lindex [r config get maxclients] 1] + assert_error "*not able to handle the specified number of clients*" { + r config set maxclients 2147483647 + } + assert_equal [lindex [r config get maxclients] 1] $cur + assert_equal [r ping] {PONG} + } + + test {CONFIG SET maxclients UINT_MAX does not wrap around} { + set cur [lindex [r config get maxclients] 1] + assert_error "*not able to handle*" {r config set maxclients 4294967295} + assert_equal [lindex [r config get maxclients] 1] $cur + assert_equal [r ping] {PONG} + } + + test {CONFIG SET maxclients below the minimum is rejected} { + set cur [lindex [r config get maxclients] 1] + assert_error "*between 1 and*" {r config set maxclients 0} + assert_equal [lindex [r config get maxclients] 1] $cur + } +} + +# The tests below read /proc//limits and are Linux only. +if {[file exists /proc/self/limits]} { + +proc get_nofile {pid kind} { + # kind: soft | hard + set fd [open /proc/$pid/limits r] + set content [read $fd] + close $fd + foreach line [split $content "\n"] { + if {[string match "Max open files*" $line]} { + set f [regexp -all -inline {\S+} $line] + if {$kind eq "soft"} { return [lindex $f 3] } + return [lindex $f 4] + } + } + return "" +} + +# Uses its own server: prlimit lowers the hard limit, which can't be undone. +start_server {tags {"limits network"} overrides {maxclients 100 min-reserved-fds 64}} { + set pid [srv 0 pid] + + test {CONFIG SET maxclients beyond what setrlimit allows fails and rolls back} { + set cur [lindex [r config get maxclients] 1] + set soft_before [get_nofile $pid soft] + # 2000000 + 64 exceeds fs.nr_open (1048576 by default), so setrlimit + # fails even for root. + assert_error "*increase 'ulimit -n'*" {r config set maxclients 2000000} + assert_equal [lindex [r config get maxclients] 1] $cur + assert_equal [get_nofile $pid soft] $soft_before + assert_equal [r ping] {PONG} + } + + test {CONFIG SET min-reserved-fds beyond what setrlimit allows fails and rolls back} { + set cur [lindex [r config get min-reserved-fds] 1] + set soft_before [get_nofile $pid soft] + assert_error "*increase 'ulimit -n'*" {r config set min-reserved-fds 2000000} + assert_equal [lindex [r config get min-reserved-fds] 1] $cur + assert_equal [get_nofile $pid soft] $soft_before + assert_equal [r ping] {PONG} + } + + if {![catch {exec prlimit --version}]} { + test {CONFIG SET raises only the soft NOFILE limit and keeps the hard limit} { + exec prlimit --pid $pid --nofile=200:100000 + assert_equal [r config set maxclients 1000] {OK} + assert_equal [get_nofile $pid soft] 1064 + assert_equal [get_nofile $pid hard] 100000 + assert_equal [r config set min-reserved-fds 128] {OK} + assert_equal [get_nofile $pid soft] 1128 + assert_equal [get_nofile $pid hard] 100000 + assert_equal [r ping] {PONG} + } + + test {CONFIG SET fails and rolls back when the hard NOFILE limit is too low} { + exec prlimit --pid $pid --nofile=1128:1128 + assert_error "*increase 'ulimit -n'*" {r config set maxclients 5000} + assert_equal [lindex [r config get maxclients] 1] 1000 + assert_error "*increase 'ulimit -n'*" {r config set min-reserved-fds 5000} + assert_equal [lindex [r config get min-reserved-fds] 1] 128 + # Decreasing never needs more fds and leaves the limit untouched. + assert_equal [r config set maxclients 500] {OK} + assert_equal [get_nofile $pid soft] 1128 + assert_equal [get_nofile $pid hard] 1128 + assert_equal [r ping] {PONG} + } + + test {Raised maxclients actually accepts more clients} { + assert_equal [r config set maxclients 300] {OK} + set clients {} + for {set i 0} {$i < 250} {incr i} { + set rd [redis_deferring_client] + $rd ping + assert_equal [$rd read] {PONG} + lappend clients $rd + } + foreach rd $clients { $rd close } + } + } +} + +tags {"limits"} { + test {Server exits at startup when NOFILE can't cover maxclients + min-reserved-fds} { + set log [file join [tmpdir server.nofile] stdout] + catch { + exec sh -c "ulimit -n 1024; exec src/redis-server --port 0 --maxclients 10000 --save '' > $log 2>&1" + } + assert_match "*Can't set maximum open files to 10032*Exiting*" [exec cat $log] + } +} + +}