Sentinel: return an error if configuration save fails (#10151)

When performing `SENTINEL SET`, Sentinel updates the local configuration file. Before this commit, failure to update the file would still result with an `+OK` reply. Now, a `-ERR Failed to save config file` error will be returned.

Co-authored-by: Yossi Gottlieb <yossigo@gmail.com>
This commit is contained in:
Wen Hui 2022-02-03 06:20:35 -05:00 committed by GitHub
parent 53c43fcc84
commit 65ef543f8c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 18 deletions

View File

@ -2281,6 +2281,16 @@ werr:
return C_ERR;
}
/* Call sentinelFlushConfig() produce a success/error reply to the
* calling client.
*/
static void sentinelFlushConfigAndReply(client *c) {
if (sentinelFlushConfig() == C_ERR)
addReplyErrorFormat(c,"Failed to save config file");
else
addReply(c, shared.ok);
}
/* ====================== hiredis connection handling ======================= */
/* Send the AUTH command with the specified master password if needed.
@ -3174,8 +3184,7 @@ void sentinelConfigSetCommand(client *c) {
return;
}
sentinelFlushConfig();
addReply(c, shared.ok);
sentinelFlushConfigAndReply(c);
/* Drop Sentinel connections to initiate a reconnect if needed. */
if (drop_conns)
@ -3929,14 +3938,12 @@ NULL
if (ri == NULL) {
addReplyError(c,sentinelCheckCreateInstanceErrors(SRI_MASTER));
} else {
sentinelFlushConfig();
sentinelFlushConfigAndReply(c);
sentinelEvent(LL_WARNING,"+monitor",ri,"%@ quorum %d",ri->quorum);
addReply(c,shared.ok);
}
} else if (!strcasecmp(c->argv[1]->ptr,"flushconfig")) {
if (c->argc != 2) goto numargserr;
sentinelFlushConfig();
addReply(c,shared.ok);
sentinelFlushConfigAndReply(c);
return;
} else if (!strcasecmp(c->argv[1]->ptr,"remove")) {
/* SENTINEL REMOVE <name> */
@ -3947,8 +3954,7 @@ NULL
== NULL) return;
sentinelEvent(LL_WARNING,"-monitor",ri,"%@");
dictDelete(sentinel.masters,c->argv[2]->ptr);
sentinelFlushConfig();
addReply(c,shared.ok);
sentinelFlushConfigAndReply(c);
} else if (!strcasecmp(c->argv[1]->ptr,"ckquorum")) {
/* SENTINEL CKQUORUM <name> */
sentinelRedisInstance *ri;
@ -4348,22 +4354,16 @@ void sentinelSetCommand(client *c) {
break;
}
}
if (changes && sentinelFlushConfig() == C_ERR) {
addReplyErrorFormat(c,"Failed to save Sentinel new configuration on disk");
return;
}
addReply(c,shared.ok);
if (changes) sentinelFlushConfigAndReply(c);
return;
badfmt: /* Bad format errors */
addReplyErrorFormat(c,"Invalid argument '%s' for SENTINEL SET '%s'",
(char*)c->argv[badarg]->ptr,option);
seterr:
if (changes && sentinelFlushConfig() == C_ERR) {
addReplyErrorFormat(c,"Failed to save Sentinel new configuration on disk");
return;
}
return;
/* TODO: Handle the case of both bad input and save error, possibly handling
* SENTINEL SET atomically. */
if (changes) sentinelFlushConfig();
}
/* Our fake PUBLISH command: it is actually useful only to receive hello messages

View File

@ -44,5 +44,17 @@ test "Sentinel Set with other error situations" {
# unknown parameter option
assert_error "ERR Unknown option or number of arguments for SENTINEL SET 'fakeoption'" {S 0 SENTINEL SET mymaster fakeoption fakevalue}
# save new config to disk failed
set info [S 0 SENTINEL master mymaster]
set origin_quorum [dict get $info quorum]
set update_quorum [expr $origin_quorum+1]
set sentinel_id 0
set configfilename [file join "sentinel_$sentinel_id" "sentinel.conf"]
exec chmod 000 $configfilename
catch {[S 0 SENTINEL SET mymaster quorum $update_quorum]} err
exec chmod 644 $configfilename
assert_equal "ERR Failed to save config file" $err
}