Allow SENTINEL CONFIG SET and SENTINEL CONFIG GET to handle multiple parameters. (#10362)

Extend SENTINEL CONFIG SET and SENTINEL CONFIG GET to be
compatible with variadic CONFIG SET and CONFIG GET and allow multiple
parameters to be modified in a single call atomically.

Co-authored-by: Oran Agra <oran@redislabs.com>
This commit is contained in:
Wen Hui 2023-05-17 03:26:02 -04:00 committed by GitHub
parent fd566f4050
commit df1890ef7f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 239 additions and 95 deletions

View File

@ -1,12 +1,18 @@
{ {
"CONFIG": { "CONFIG": {
"summary": "Configures Redis Sentinel.", "summary": "Configures Redis Sentinel.",
"complexity": "O(1)", "complexity": "O(N) when N is the number of configuration parameters provided",
"group": "sentinel", "group": "sentinel",
"since": "6.2.0", "since": "6.2.0",
"arity": -4, "arity": -4,
"container": "SENTINEL", "container": "SENTINEL",
"function": "sentinelCommand", "function": "sentinelCommand",
"history": [
[
"7.2.0",
"Added the ability to set and get multiple parameters in one call."
]
],
"command_flags": [ "command_flags": [
"ADMIN", "ADMIN",
"SENTINEL", "SENTINEL",
@ -42,7 +48,7 @@
"type": "string" "type": "string"
}, },
"announce-port": { "announce-port": {
"type": "integer" "type": "string"
}, },
"sentinel-user": { "sentinel-user": {
"type": "string" "type": "string"
@ -87,6 +93,7 @@
"name":"set", "name":"set",
"token":"SET", "token":"SET",
"type":"block", "type":"block",
"multiple": true,
"arguments":[ "arguments":[
{ {
"name":"parameter", "name":"parameter",
@ -101,7 +108,8 @@
{ {
"token":"GET", "token":"GET",
"name":"parameter", "name":"parameter",
"type":"string" "type":"string",
"multiple": true
} }
] ]
} }

View File

@ -3178,6 +3178,13 @@ void sentinelSendPeriodicCommands(sentinelRedisInstance *ri) {
} }
/* =========================== SENTINEL command ============================= */ /* =========================== SENTINEL command ============================= */
static void populateDict(dict *options_dict, char **options) {
for (int i=0; options[i]; i++) {
sds option = sdsnew(options[i]);
if (dictAdd(options_dict, option, NULL)==DICT_ERR)
sdsfree(option);
}
}
const char* getLogLevel(void) { const char* getLogLevel(void) {
switch (server.verbosity) { switch (server.verbosity) {
@ -3189,52 +3196,112 @@ const char* getLogLevel(void) {
return "unknown"; return "unknown";
} }
/* SENTINEL CONFIG SET <option> <value>*/ /* SENTINEL CONFIG SET option value [option value ...] */
void sentinelConfigSetCommand(client *c) { void sentinelConfigSetCommand(client *c) {
robj *o = c->argv[3];
robj *val = c->argv[4];
long long numval; long long numval;
int drop_conns = 0; int drop_conns = 0;
char *option;
robj *val;
char *options[] = {
"announce-ip",
"sentinel-user",
"sentinel-pass",
"resolve-hostnames",
"announce-port",
"announce-hostnames",
"loglevel",
NULL};
static dict *options_dict = NULL;
if (!options_dict) {
options_dict = dictCreate(&stringSetDictType);
populateDict(options_dict, options);
}
dict *set_configs = dictCreate(&stringSetDictType);
if (!strcasecmp(o->ptr, "resolve-hostnames")) { /* Validate arguments are valid */
if ((numval = yesnotoi(val->ptr)) == -1) goto badfmt; for (int i = 3; i < c->argc; i++) {
sentinel.resolve_hostnames = numval; option = c->argv[i]->ptr;
} else if (!strcasecmp(o->ptr, "announce-hostnames")) {
if ((numval = yesnotoi(val->ptr)) == -1) goto badfmt; /* Validate option is valid */
sentinel.announce_hostnames = numval; if (dictFind(options_dict, option) == NULL) {
} else if (!strcasecmp(o->ptr, "announce-ip")) { addReplyErrorFormat(c, "Invalid argument '%s' to SENTINEL CONFIG SET", option);
if (sentinel.announce_ip) sdsfree(sentinel.announce_ip); goto exit;
sentinel.announce_ip = sdsnew(val->ptr); }
} else if (!strcasecmp(o->ptr, "announce-port")) {
if (getLongLongFromObject(val, &numval) == C_ERR || /* Check duplicates */
numval < 0 || numval > 65535) if (dictFind(set_configs, option) != NULL) {
goto badfmt; addReplyErrorFormat(c, "Duplicate argument '%s' to SENTINEL CONFIG SET", option);
sentinel.announce_port = numval; goto exit;
} else if (!strcasecmp(o->ptr, "sentinel-user")) { }
sdsfree(sentinel.sentinel_auth_user);
sentinel.sentinel_auth_user = sdslen(val->ptr) == 0 ? serverAssert(dictAdd(set_configs, sdsnew(option), NULL) == C_OK);
NULL : sdsdup(val->ptr);
drop_conns = 1; /* Validate argument */
} else if (!strcasecmp(o->ptr, "sentinel-pass")) { if (i + 1 == c->argc) {
sdsfree(sentinel.sentinel_auth_pass); addReplyErrorFormat(c, "Missing argument '%s' value", option);
sentinel.sentinel_auth_pass = sdslen(val->ptr) == 0 ? goto exit;
NULL : sdsdup(val->ptr); }
drop_conns = 1; val = c->argv[++i];
} else if (!strcasecmp(o->ptr, "loglevel")) {
if (!strcasecmp(val->ptr, "debug")) if (!strcasecmp(option, "resolve-hostnames")) {
server.verbosity = LL_DEBUG; if ((yesnotoi(val->ptr)) == -1) goto badfmt;
else if (!strcasecmp(val->ptr, "verbose")) } else if (!strcasecmp(option, "announce-hostnames")) {
server.verbosity = LL_VERBOSE; if ((yesnotoi(val->ptr)) == -1) goto badfmt;
else if (!strcasecmp(val->ptr, "notice")) } else if (!strcasecmp(option, "announce-port")) {
server.verbosity = LL_NOTICE; if (getLongLongFromObject(val, &numval) == C_ERR ||
else if (!strcasecmp(val->ptr, "warning")) numval < 0 || numval > 65535) goto badfmt;
server.verbosity = LL_WARNING; } else if (!strcasecmp(option, "loglevel")) {
else if (!(!strcasecmp(val->ptr, "debug") || !strcasecmp(val->ptr, "verbose") ||
goto badfmt; !strcasecmp(val->ptr, "notice") || !strcasecmp(val->ptr, "warning"))) goto badfmt;
} else { }
addReplyErrorFormat(c, "Invalid argument '%s' to SENTINEL CONFIG SET", }
(char *) o->ptr);
return; /* Apply changes */
for (int i = 3; i < c->argc; i++) {
int moreargs = (c->argc-1) - i;
option = c->argv[i]->ptr;
if (!strcasecmp(option, "loglevel") && moreargs > 0) {
val = c->argv[++i];
if (!strcasecmp(val->ptr, "debug"))
server.verbosity = LL_DEBUG;
else if (!strcasecmp(val->ptr, "verbose"))
server.verbosity = LL_VERBOSE;
else if (!strcasecmp(val->ptr, "notice"))
server.verbosity = LL_NOTICE;
else if (!strcasecmp(val->ptr, "warning"))
server.verbosity = LL_WARNING;
} else if (!strcasecmp(option, "resolve-hostnames") && moreargs > 0) {
val = c->argv[++i];
numval = yesnotoi(val->ptr);
sentinel.resolve_hostnames = numval;
} else if (!strcasecmp(option, "announce-hostnames") && moreargs > 0) {
val = c->argv[++i];
numval = yesnotoi(val->ptr);
sentinel.announce_hostnames = numval;
} else if (!strcasecmp(option, "announce-ip") && moreargs > 0) {
val = c->argv[++i];
if (sentinel.announce_ip) sdsfree(sentinel.announce_ip);
sentinel.announce_ip = sdsnew(val->ptr);
} else if (!strcasecmp(option, "announce-port") && moreargs > 0) {
val = c->argv[++i];
getLongLongFromObject(val, &numval);
sentinel.announce_port = numval;
} else if (!strcasecmp(option, "sentinel-user") && moreargs > 0) {
val = c->argv[++i];
sdsfree(sentinel.sentinel_auth_user);
sentinel.sentinel_auth_user = sdslen(val->ptr) == 0 ?
NULL : sdsdup(val->ptr);
drop_conns = 1;
} else if (!strcasecmp(option, "sentinel-pass") && moreargs > 0) {
val = c->argv[++i];
sdsfree(sentinel.sentinel_auth_pass);
sentinel.sentinel_auth_pass = sdslen(val->ptr) == 0 ?
NULL : sdsdup(val->ptr);
drop_conns = 1;
} else {
/* Should never reach here */
serverAssert(0);
}
} }
sentinelFlushConfigAndReply(c); sentinelFlushConfigAndReply(c);
@ -3243,61 +3310,72 @@ void sentinelConfigSetCommand(client *c) {
if (drop_conns) if (drop_conns)
sentinelDropConnections(); sentinelDropConnections();
exit:
dictRelease(set_configs);
return; return;
badfmt: badfmt:
addReplyErrorFormat(c, "Invalid value '%s' to SENTINEL CONFIG SET '%s'", addReplyErrorFormat(c, "Invalid value '%s' to SENTINEL CONFIG SET '%s'",
(char *) val->ptr, (char *) o->ptr); (char *) val->ptr, option);
dictRelease(set_configs);
} }
/* SENTINEL CONFIG GET <option> */ /* SENTINEL CONFIG GET <option> [<option> ...] */
void sentinelConfigGetCommand(client *c) { void sentinelConfigGetCommand(client *c) {
robj *o = c->argv[3]; char *pattern;
const char *pattern = o->ptr;
void *replylen = addReplyDeferredLen(c); void *replylen = addReplyDeferredLen(c);
int matches = 0; int matches = 0;
/* Create a dictionary to store the input configs,to avoid adding duplicate twice */
if (stringmatch(pattern,"resolve-hostnames",1)) { dict *d = dictCreate(&externalStringType);
addReplyBulkCString(c,"resolve-hostnames"); for (int i = 3; i < c->argc; i++) {
addReplyBulkCString(c,sentinel.resolve_hostnames ? "yes" : "no"); pattern = c->argv[i]->ptr;
matches++; /* If the string doesn't contain glob patterns and available in dictionary, don't look further, just continue. */
} if (!strpbrk(pattern, "[*?") && dictFind(d, pattern)) continue;
/* we want to print all the matched patterns and avoid printing duplicates twice */
if (stringmatch(pattern, "announce-hostnames", 1)) { if (stringmatch(pattern,"resolve-hostnames",1) && !dictFind(d, "resolve-hostnames")) {
addReplyBulkCString(c,"announce-hostnames"); addReplyBulkCString(c,"resolve-hostnames");
addReplyBulkCString(c,sentinel.announce_hostnames ? "yes" : "no"); addReplyBulkCString(c,sentinel.resolve_hostnames ? "yes" : "no");
matches++; dictAdd(d, "resolve-hostnames", NULL);
} matches++;
}
if (stringmatch(pattern, "announce-ip", 1)) { if (stringmatch(pattern, "announce-hostnames", 1) && !dictFind(d, "announce-hostnames")) {
addReplyBulkCString(c,"announce-ip"); addReplyBulkCString(c,"announce-hostnames");
addReplyBulkCString(c,sentinel.announce_ip ? sentinel.announce_ip : ""); addReplyBulkCString(c,sentinel.announce_hostnames ? "yes" : "no");
matches++; dictAdd(d, "announce-hostnames", NULL);
} matches++;
}
if (stringmatch(pattern, "announce-port", 1)) { if (stringmatch(pattern, "announce-ip", 1) && !dictFind(d, "announce-ip")) {
addReplyBulkCString(c, "announce-port"); addReplyBulkCString(c,"announce-ip");
addReplyBulkLongLong(c, sentinel.announce_port); addReplyBulkCString(c,sentinel.announce_ip ? sentinel.announce_ip : "");
matches++; dictAdd(d, "announce-ip", NULL);
} matches++;
}
if (stringmatch(pattern, "sentinel-user", 1)) { if (stringmatch(pattern, "announce-port", 1) && !dictFind(d, "announce-port")) {
addReplyBulkCString(c, "sentinel-user"); addReplyBulkCString(c, "announce-port");
addReplyBulkCString(c, sentinel.sentinel_auth_user ? sentinel.sentinel_auth_user : ""); addReplyBulkLongLong(c, sentinel.announce_port);
matches++; dictAdd(d, "announce-port", NULL);
} matches++;
}
if (stringmatch(pattern, "sentinel-pass", 1)) { if (stringmatch(pattern, "sentinel-user", 1) && !dictFind(d, "sentinel-user")) {
addReplyBulkCString(c, "sentinel-pass"); addReplyBulkCString(c, "sentinel-user");
addReplyBulkCString(c, sentinel.sentinel_auth_pass ? sentinel.sentinel_auth_pass : ""); addReplyBulkCString(c, sentinel.sentinel_auth_user ? sentinel.sentinel_auth_user : "");
matches++; dictAdd(d, "sentinel-user", NULL);
} matches++;
}
if (stringmatch(pattern, "loglevel", 1)) { if (stringmatch(pattern, "sentinel-pass", 1) && !dictFind(d, "sentinel-pass")) {
addReplyBulkCString(c, "loglevel"); addReplyBulkCString(c, "sentinel-pass");
addReplyBulkCString(c, getLogLevel()); addReplyBulkCString(c, sentinel.sentinel_auth_pass ? sentinel.sentinel_auth_pass : "");
matches++; dictAdd(d, "sentinel-pass", NULL);
matches++;
}
if (stringmatch(pattern, "loglevel", 1) && !dictFind(d, "loglevel")) {
addReplyBulkCString(c, "loglevel");
addReplyBulkCString(c, getLogLevel());
dictAdd(d, "loglevel", NULL);
matches++;
}
} }
dictRelease(d);
setDeferredMapLen(c, replylen, matches); setDeferredMapLen(c, replylen, matches);
} }
@ -3787,9 +3865,9 @@ void sentinelCommand(client *c) {
" Check if the current Sentinel configuration is able to reach the quorum", " Check if the current Sentinel configuration is able to reach the quorum",
" needed to failover a master and the majority needed to authorize the", " needed to failover a master and the majority needed to authorize the",
" failover.", " failover.",
"CONFIG SET <param> <value>", "CONFIG SET param value [param value ...]",
" Set a global Sentinel configuration parameter.", " Set a global Sentinel configuration parameter.",
"CONFIG GET <param>", "CONFIG GET <param> [param param param ...]",
" Get global Sentinel configuration parameter.", " Get global Sentinel configuration parameter.",
"DEBUG [<param> <value> ...]", "DEBUG [<param> <value> ...]",
" Show a list of configurable time parameters and their values (milliseconds).", " Show a list of configurable time parameters and their values (milliseconds).",
@ -4042,12 +4120,12 @@ NULL
sentinelSetCommand(c); sentinelSetCommand(c);
} else if (!strcasecmp(c->argv[1]->ptr,"config")) { } else if (!strcasecmp(c->argv[1]->ptr,"config")) {
if (c->argc < 4) goto numargserr; if (c->argc < 4) goto numargserr;
if (!strcasecmp(c->argv[2]->ptr,"set") && c->argc == 5) if (!strcasecmp(c->argv[2]->ptr,"set") && c->argc >= 5)
sentinelConfigSetCommand(c); sentinelConfigSetCommand(c);
else if (!strcasecmp(c->argv[2]->ptr,"get") && c->argc == 4) else if (!strcasecmp(c->argv[2]->ptr,"get") && c->argc >= 4)
sentinelConfigGetCommand(c); sentinelConfigGetCommand(c);
else else
addReplyError(c, "Only SENTINEL CONFIG GET <option> / SET <option> <value> are supported."); addReplyError(c, "Only SENTINEL CONFIG GET <param> [<param> <param> ...]/ SET <param> <value> [<param> <value> ...] are supported.");
} else if (!strcasecmp(c->argv[1]->ptr,"info-cache")) { } else if (!strcasecmp(c->argv[1]->ptr,"info-cache")) {
/* SENTINEL INFO-CACHE <name> */ /* SENTINEL INFO-CACHE <name> */
if (c->argc < 2) goto numargserr; if (c->argc < 2) goto numargserr;

View File

@ -0,0 +1,58 @@
source "../tests/includes/init-tests.tcl"
test "SENTINEL CONFIG SET and SENTINEL CONFIG GET handles multiple variables" {
foreach_sentinel_id id {
assert_equal {OK} [S $id SENTINEL CONFIG SET resolve-hostnames yes announce-port 1234]
}
assert_match {*yes*1234*} [S 1 SENTINEL CONFIG GET resolve-hostnames announce-port]
assert_match {announce-port 1234} [S 1 SENTINEL CONFIG GET announce-port]
}
test "SENTINEL CONFIG GET for duplicate and unknown variables" {
assert_equal {OK} [S 1 SENTINEL CONFIG SET resolve-hostnames yes announce-port 1234]
assert_match {resolve-hostnames yes} [S 1 SENTINEL CONFIG GET resolve-hostnames resolve-hostnames does-not-exist]
}
test "SENTINEL CONFIG GET for patterns" {
assert_equal {OK} [S 1 SENTINEL CONFIG SET loglevel notice announce-port 1234 announce-hostnames yes ]
assert_match {loglevel notice} [S 1 SENTINEL CONFIG GET log* *level loglevel]
assert_match {announce-hostnames yes announce-ip*announce-port 1234} [S 1 SENTINEL CONFIG GET announce*]
}
test "SENTINEL CONFIG SET duplicate variables" {
catch {[S 1 SENTINEL CONFIG SET resolve-hostnames yes announce-port 1234 announce-port 100]} e
if {![string match "*Duplicate argument*" $e]} {
fail "Should give wrong arity error"
}
}
test "SENTINEL CONFIG SET, one option does not exist" {
foreach_sentinel_id id {
assert_equal {OK} [S $id SENTINEL CONFIG SET announce-port 111]
catch {[S $id SENTINEL CONFIG SET does-not-exist yes announce-port 1234]} e
if {![string match "*Invalid argument*" $e]} {
fail "Should give Invalid argument error"
}
}
# The announce-port should not be set to 1234 as it was called with a wrong argument
assert_match {*111*} [S 1 SENTINEL CONFIG GET announce-port]
}
test "SENTINEL CONFIG SET, one option with wrong value" {
foreach_sentinel_id id {
assert_equal {OK} [S $id SENTINEL CONFIG SET resolve-hostnames no]
catch {[S $id SENTINEL CONFIG SET announce-port -1234 resolve-hostnames yes]} e
if {![string match "*Invalid value*" $e]} {
fail "Expected to return Invalid value error"
}
}
# The resolve-hostnames should not be set to yes as it was called after an argument with an invalid value
assert_match {*no*} [S 1 SENTINEL CONFIG GET resolve-hostnames]
}
test "SENTINEL CONFIG SET, wrong number of arguments" {
catch {[S 1 SENTINEL CONFIG SET resolve-hostnames yes announce-port 1234 announce-ip]} e
if {![string match "*Missing argument*" $e]} {
fail "Expected to return Missing argument error"
}
}