summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOran Agra <oran@redislabs.com>2022-05-09 13:37:49 +0300
committerGitHub <noreply@github.com>2022-05-09 13:37:49 +0300
commit2bcd890d8aa645cab0d8fd0ed765c52a997de4f5 (patch)
tree85d2602858f6bb1538626d7fbcf7a58bb28d41f2
parenteb915a82a5f4d7fd6c36ad1710b35e2b0c2abd31 (diff)
downloadredis-2bcd890d8aa645cab0d8fd0ed765c52a997de4f5.tar.gz
Fix --save command line regression in redis 7.0.0 (#10690)
Unintentional change in #9644 (since RC1) meant that an empty `--save ""` config from command line, wouldn't have clear any setting from the config file Added tests to cover that, and improved test infra to take additional command line args for redis-server
-rw-r--r--src/config.c4
-rw-r--r--tests/assets/default.conf3
-rw-r--r--tests/support/server.tcl22
-rw-r--r--tests/unit/introspection.tcl17
4 files changed, 36 insertions, 10 deletions
diff --git a/src/config.c b/src/config.c
index f7252ca6e..04f0dbcd8 100644
--- a/src/config.c
+++ b/src/config.c
@@ -2595,8 +2595,10 @@ static int setConfigSaveOption(standardConfig *config, sds *argv, int argc, cons
int j;
/* Special case: treat single arg "" as zero args indicating empty save configuration */
- if (argc == 1 && !strcasecmp(argv[0],""))
+ if (argc == 1 && !strcasecmp(argv[0],"")) {
+ resetServerSaveParams();
argc = 0;
+ }
/* Perform sanity check before setting the new config:
* - Even number of args
diff --git a/tests/assets/default.conf b/tests/assets/default.conf
index 227ab5e7f..4ae420790 100644
--- a/tests/assets/default.conf
+++ b/tests/assets/default.conf
@@ -13,9 +13,8 @@ databases 16
latency-monitor-threshold 1
repl-diskless-sync-delay 0
+# Note the infrastructure in server.tcl uses a dict, we can't provide several save directives
save 900 1
-save 300 10
-save 60 10000
rdbcompression yes
dbfilename dump.rdb
diff --git a/tests/support/server.tcl b/tests/support/server.tcl
index 9d0c4510d..4a993f552 100644
--- a/tests/support/server.tcl
+++ b/tests/support/server.tcl
@@ -262,16 +262,22 @@ proc create_server_config_file {filename config} {
close $fp
}
-proc spawn_server {config_file stdout stderr} {
+proc spawn_server {config_file stdout stderr args} {
+ set cmd [list src/redis-server $config_file]
+ set args {*}$args
+ if {[llength $args] > 0} {
+ lappend cmd {*}$args
+ }
+
if {$::valgrind} {
- set pid [exec valgrind --track-origins=yes --trace-children=yes --suppressions=[pwd]/src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full src/redis-server $config_file >> $stdout 2>> $stderr &]
+ set pid [exec valgrind --track-origins=yes --trace-children=yes --suppressions=[pwd]/src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full {*}$cmd >> $stdout 2>> $stderr &]
} elseif ($::stack_logging) {
- set pid [exec /usr/bin/env MallocStackLogging=1 MallocLogFile=/tmp/malloc_log.txt src/redis-server $config_file >> $stdout 2>> $stderr &]
+ set pid [exec /usr/bin/env MallocStackLogging=1 MallocLogFile=/tmp/malloc_log.txt {*}$cmd >> $stdout 2>> $stderr &]
} else {
# ASAN_OPTIONS environment variable is for address sanitizer. If a test
# tries to allocate huge memory area and expects allocator to return
# NULL, address sanitizer throws an error without this setting.
- set pid [exec /usr/bin/env ASAN_OPTIONS=allocator_may_return_null=1 src/redis-server $config_file >> $stdout 2>> $stderr &]
+ set pid [exec /usr/bin/env ASAN_OPTIONS=allocator_may_return_null=1 {*}$cmd >> $stdout 2>> $stderr &]
}
if {$::wait_server} {
@@ -398,6 +404,7 @@ proc start_server {options {code undefined}} {
set overrides {}
set omit {}
set tags {}
+ set args {}
set keep_persistence false
# parse options
@@ -409,6 +416,9 @@ proc start_server {options {code undefined}} {
"overrides" {
set overrides $value
}
+ "args" {
+ set args $value
+ }
"omit" {
set omit $value
}
@@ -518,7 +528,7 @@ proc start_server {options {code undefined}} {
send_data_packet $::test_server_fd "server-spawning" "port $port"
- set pid [spawn_server $config_file $stdout $stderr]
+ set pid [spawn_server $config_file $stdout $stderr $args]
# check that the server actually started
set port_busy [wait_server_started $config_file $stdout $pid]
@@ -721,7 +731,7 @@ proc restart_server {level wait_ready rotate_logs {reconnect 1} {shutdown sigter
set config_file [dict get $srv "config_file"]
- set pid [spawn_server $config_file $stdout $stderr]
+ set pid [spawn_server $config_file $stdout $stderr {}]
# check that the server actually started
wait_server_started $config_file $stdout $pid
diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl
index 4b4b8f56b..41d20e20a 100644
--- a/tests/unit/introspection.tcl
+++ b/tests/unit/introspection.tcl
@@ -166,11 +166,26 @@ start_server {tags {"introspection"}} {
assert_match [r config get save] {save {3600 1 300 100 60 10000}}
}
- # First "save" keyword overrides defaults
+ # First "save" keyword overrides hard coded defaults
start_server {config "minimal.conf" overrides {save {100 100}}} {
# Defaults
assert_match [r config get save] {save {100 100}}
}
+
+ # First "save" keyword in default config file
+ start_server {config "default.conf"} {
+ assert_match [r config get save] {save {900 1}}
+ }
+
+ # First "save" keyword appends default from config file
+ start_server {config "default.conf" args {--save 100 100}} {
+ assert_match [r config get save] {save {900 1 100 100}}
+ }
+
+ # Empty "save" keyword resets all
+ start_server {config "default.conf" args {--save {}}} {
+ assert_match [r config get save] {save {}}
+ }
} {} {external:skip}
test {CONFIG sanity} {