Skip to content

Commit

Permalink
Fix default/explicit "save" parameter loading. (redis#7767)
Browse files Browse the repository at this point in the history
Save parameters should either be default or whatever specified in the
config file. This fixes an issue introduced in redis#7092 which causes
configuration file settings to be applied on top of the defaults.
  • Loading branch information
yossigo authored Sep 9, 2020
1 parent ce15620 commit 818a746
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ void loadServerConfigFromString(char *config) {
int linenum = 0, totlines, i;
int slaveof_linenum = 0;
sds *lines;
int save_loaded = 0;

lines = sdssplitlen(config,strlen(config),"\n",1,&totlines);

Expand Down Expand Up @@ -425,6 +426,14 @@ void loadServerConfigFromString(char *config) {
err = "Invalid socket file permissions"; goto loaderr;
}
} else if (!strcasecmp(argv[0],"save")) {
/* We don't reset save params before loading, because if they're not part
* of the file the defaults should be used.
*/
if (!save_loaded) {
save_loaded = 1;
resetServerSaveParams();
}

if (argc == 3) {
int seconds = atoi(argv[1]);
int changes = atoi(argv[2]);
Expand Down
5 changes: 5 additions & 0 deletions tests/assets/minimal.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Minimal configuration for testing.
always-show-logo yes
daemonize no
pidfile /var/run/redis.pid
loglevel verbose
13 changes: 13 additions & 0 deletions tests/unit/introspection.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,19 @@ start_server {tags {"introspection"}} {
}
}

test {CONFIG save params special case handled properly} {
# No "save" keyword - defaults should apply
start_server {config "minimal.conf"} {
assert_match [r config get save] {save {3600 1 300 100 60 10000}}
}

# First "save" keyword overrides defaults
start_server {config "minimal.conf" overrides {save {100 100}}} {
# Defaults
assert_match [r config get save] {save {100 100}}
}
}

test {CONFIG sanity} {
# Do CONFIG GET, CONFIG SET and then CONFIG GET again
# Skip immutable configs, one with no get, and other complicated configs
Expand Down

0 comments on commit 818a746

Please sign in to comment.