Skip to content

Fix slot-migration-max-failover-repl-bytes unable to accept -1 #3443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
enjoy-binbin merged 4 commits into from
Apr 8, 2026
Merged

Fix slot-migration-max-failover-repl-bytes unable to accept -1 #3443

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e2aa60
Fix slot-migration-max-failover-repl-bytes unable to accept -1
enjoy-binbin Apr 4, 2026
b6b0bfc
Add test and update the log
enjoy-binbin Apr 6, 2026
9781ace
code review from viktor
enjoy-binbin Apr 8, 2026
442125d
Update src/config.c
enjoy-binbin Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions src/config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2168,6 +2168,13 @@ static int numericParseString(standardConfig *config, sds value, const char **er
int memerr;
*res = memtoull(value, &memerr);
if (!memerr) return 1;

/* memtoull rejects negative values, but some memory configs accept
* special negative values (e.g. -1 to disable a limit). Fall back
* to plain integer parsing for those. */
if (config->data.numeric.flags & SIGNED_MEMORY_CONFIG) {
if (string2ll(value, sdslen(value), res)) return 1;
}
}

/* Attempt to parse as percent */
Expand Down Expand Up @@ -2242,7 +2249,11 @@ static sds numericConfigGet(standardConfig *config) {
buf[len] = '%';
buf[len + 1] = '\0';
} else if (config->data.numeric.flags & MEMORY_CONFIG) {
ull2string(buf, sizeof(buf), value);
if (config->data.numeric.flags & SIGNED_MEMORY_CONFIG && value < 0) {
ll2string(buf, sizeof(buf), value);
} else {
ull2string(buf, sizeof(buf), value);
}
Comment thread
enjoy-binbin marked this conversation as resolved.
Outdated
} else if (config->data.numeric.flags & OCTAL_CONFIG) {
snprintf(buf, sizeof(buf), "%llo", value);
} else if (config->data.numeric.flags & UNSIGNED_CONFIG) {
Expand All @@ -2261,7 +2272,11 @@ static void numericConfigRewrite(standardConfig *config, const char *name, struc
if (config->data.numeric.flags & PERCENT_CONFIG && value < 0) {
rewriteConfigPercentOption(state, name, -value, config->data.numeric.default_value);
} else if (config->data.numeric.flags & MEMORY_CONFIG) {
rewriteConfigBytesOption(state, name, value, config->data.numeric.default_value);
if (config->data.numeric.flags & SIGNED_MEMORY_CONFIG && value < 0) {
rewriteConfigNumericalOption(state, name, value, config->data.numeric.default_value);
} else {
rewriteConfigBytesOption(state, name, value, config->data.numeric.default_value);
}
} else if (config->data.numeric.flags & OCTAL_CONFIG) {
rewriteConfigOctalOption(state, name, value, config->data.numeric.default_value);
} else if (config->data.numeric.flags & UNSIGNED_CONFIG) {
Expand Down Expand Up @@ -3452,7 +3467,7 @@ standardConfig static_configs[] = {
createSizeTConfig("tracking-table-max-keys", NULL, MODIFIABLE_CONFIG, 0, LONG_MAX, server.tracking_table_max_keys, 1000000, INTEGER_CONFIG, NULL, NULL), /* Default: 1 million keys max. */
createSizeTConfig("client-query-buffer-limit", NULL, DEBUG_CONFIG | MODIFIABLE_CONFIG, 1024 * 1024, LONG_MAX, server.client_max_querybuf_len, 1024 * 1024 * 1024, MEMORY_CONFIG, NULL, NULL), /* Default: 1GB max query buffer. */
createSSizeTConfig("maxmemory-clients", NULL, MODIFIABLE_CONFIG, -100, SSIZE_MAX, server.maxmemory_clients, 0, MEMORY_CONFIG | PERCENT_CONFIG, NULL, applyClientMaxMemoryUsage),
createSSizeTConfig("slot-migration-max-failover-repl-bytes", NULL, MODIFIABLE_CONFIG, -1, SSIZE_MAX, server.slot_migration_max_failover_repl_bytes, 0, MEMORY_CONFIG, NULL, NULL),
createSSizeTConfig("slot-migration-max-failover-repl-bytes", NULL, MODIFIABLE_CONFIG, -1, SSIZE_MAX, server.slot_migration_max_failover_repl_bytes, 0, MEMORY_CONFIG | SIGNED_MEMORY_CONFIG, NULL, NULL),
Comment thread
enjoy-binbin marked this conversation as resolved.

/* Other configs */
createTimeTConfig("repl-backlog-ttl", NULL, MODIFIABLE_CONFIG, 0, LONG_MAX, server.repl_backlog_time_limit, 60 * 60, INTEGER_CONFIG, NULL, NULL), /* Default: 1 hour */
Expand Down Expand Up @@ -3519,6 +3534,14 @@ void initConfigValues(void) {
dictExpand(configs, sizeof(static_configs) / sizeof(standardConfig));
for (standardConfig *config = static_configs; config->name != NULL; config++) {
if (config->interface.init) config->interface.init(config);

/* PERCENT_CONFIG and SIGNED_MEMORY_CONFIG both use negative values
* with different semantics, so they must not be combined. */
if (config->type == NUMERIC_CONFIG) {
serverAssert(!((config->data.numeric.flags & PERCENT_CONFIG) &&
(config->data.numeric.flags & SIGNED_MEMORY_CONFIG)));
}
Comment thread
enjoy-binbin marked this conversation as resolved.
Outdated

/* Add the primary config to the dictionary. */
int ret = registerConfigValue(config->name, config, 0);
serverAssert(ret);
Expand Down
12 changes: 7 additions & 5 deletions src/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3640,11 +3640,13 @@ sds keyspaceEventsFlagsToString(int flags);
* to apply the configuration change even if the new config value is the same as \
* the old. */

#define INTEGER_CONFIG 0 /* No flags means a simple integer configuration */
#define MEMORY_CONFIG (1 << 0) /* Indicates if this value can be loaded as a memory value */
#define PERCENT_CONFIG (1 << 1) /* Indicates if this value can be loaded as a percent (and stored as a negative int) */
#define OCTAL_CONFIG (1 << 2) /* This value uses octal representation */
#define UNSIGNED_CONFIG (1 << 3) /* This value uses unsigned representation */
/* Numeric Flags */
#define INTEGER_CONFIG 0 /* No flags means a simple integer configuration */
#define MEMORY_CONFIG (1 << 0) /* Indicates if this value can be loaded as a memory value */
#define PERCENT_CONFIG (1 << 1) /* Indicates if this value can be loaded as a percent (and stored as a negative int) */
#define OCTAL_CONFIG (1 << 2) /* This value uses octal representation */
#define UNSIGNED_CONFIG (1 << 3) /* This value uses unsigned representation */
#define SIGNED_MEMORY_CONFIG (1 << 4) /* A MEMORY_CONFIG that also accepts plain negative integers */

/* Enum Configs contain an array of configEnum objects that match a string with an integer. */
typedef struct configEnum {
Expand Down
14 changes: 13 additions & 1 deletion tests/unit/introspection.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1964,10 +1964,22 @@ test {CONFIG REWRITE handles alias config properly} {
}
} {} {external:skip}

test {SIGNED MEMORY CONFIG allows negative number} {
start_server {tags {"introspection"}} {
r config set slot-migration-max-failover-repl-bytes -1
assert_equal [lindex [r config get slot-migration-max-failover-repl-bytes] 1] -1
assert_error {*argument must be between -1 and *} {r config set slot-migration-max-failover-repl-bytes -2}

r config rewrite
restart_server 0 true false
assert_equal [lindex [r config get slot-migration-max-failover-repl-bytes] 1] -1
}
} {} {external:skip}

test {CONFIG hash-seed is immutable and settable at startup} {
start_server {tags {"introspection"} overrides {hash-seed aabbccddeeffgghh}} {
assert_error "ERR CONFIG SET failed (possibly related to argument 'hash-seed') - can't set immutable config*" {
r config set hash-seed newseed
}
}
} {} {external:skip}
} {} {external:skip}
Loading