Better error handling for updateClientOutputBufferLimit. (#9308)
This one follow #9313 and goes deeper (validation of config file parsing) Move the check/update logic to a new updateClientOutputBufferLimit function. So that it can be used in CONFIG SET and config file parsing.
This commit is contained in:
Binbin
GitHub
parent
97dcf95cc8
commit
aefbc23451
123
src/config.c
123
src/config.c
@ -384,6 +384,62 @@ static int updateOOMScoreAdjValues(sds *args, const char **err, int apply) {
|
||||
return C_OK;
|
||||
}
|
||||
|
||||
/* Parse an array of `arg_len` sds strings, validate and populate
|
||||
* server.client_obuf_limits if valid.
|
||||
* Used in CONFIG SET and configuration file parsing. */
|
||||
static int updateClientOutputBufferLimit(sds *args, int arg_len, const char **err) {
|
||||
int j;
|
||||
int class;
|
||||
unsigned long long hard, soft;
|
||||
int hard_err, soft_err;
|
||||
int soft_seconds;
|
||||
char *soft_seconds_eptr;
|
||||
clientBufferLimitsConfig values[CLIENT_TYPE_OBUF_COUNT];
|
||||
int classes[CLIENT_TYPE_OBUF_COUNT] = {0};
|
||||
|
||||
/* We need a multiple of 4: <class> <hard> <soft> <soft_seconds> */
|
||||
if (arg_len % 4) {
|
||||
if (err) *err = "Wrong number of arguments in "
|
||||
"buffer limit configuration.";
|
||||
return C_ERR;
|
||||
}
|
||||
|
||||
/* Sanity check of single arguments, so that we either refuse the
|
||||
* whole configuration string or accept it all, even if a single
|
||||
* error in a single client class is present. */
|
||||
for (j = 0; j < arg_len; j += 4) {
|
||||
class = getClientTypeByName(args[j]);
|
||||
if (class == -1 || class == CLIENT_TYPE_MASTER) {
|
||||
if (err) *err = "Invalid client class specified in "
|
||||
"buffer limit configuration.";
|
||||
return C_ERR;
|
||||
}
|
||||
|
||||
hard = memtoull(args[j+1], &hard_err);
|
||||
soft = memtoull(args[j+2], &soft_err);
|
||||
soft_seconds = strtoll(args[j+3], &soft_seconds_eptr, 10);
|
||||
if (hard_err || soft_err ||
|
||||
soft_seconds < 0 || *soft_seconds_eptr != '\0')
|
||||
{
|
||||
if (err) *err = "Error in hard, soft or soft_seconds setting in "
|
||||
"buffer limit configuration.";
|
||||
return C_ERR;
|
||||
}
|
||||
|
||||
values[class].hard_limit_bytes = hard;
|
||||
values[class].soft_limit_bytes = soft;
|
||||
values[class].soft_limit_seconds = soft_seconds;
|
||||
classes[class] = 1;
|
||||
}
|
||||
|
||||
/* Finally set the new config. */
|
||||
for (j = 0; j < CLIENT_TYPE_OBUF_COUNT; j++) {
|
||||
if (classes[j]) server.client_obuf_limits[j] = values[j];
|
||||
}
|
||||
|
||||
return C_OK;
|
||||
}
|
||||
|
||||
void initConfigValues() {
|
||||
for (standardConfig *config = configs; config->name != NULL; config++) {
|
||||
config->interface.init(config->data);
|
||||
@ -550,25 +606,7 @@ void loadServerConfigFromString(char *config) {
|
||||
} else if (!strcasecmp(argv[0],"client-output-buffer-limit") &&
|
||||
argc == 5)
|
||||
{
|
||||
int class = getClientTypeByName(argv[1]);
|
||||
unsigned long long hard, soft;
|
||||
int soft_seconds;
|
||||
|
||||
if (class == -1 || class == CLIENT_TYPE_MASTER) {
|
||||
err = "Unrecognized client limit class: the user specified "
|
||||
"an invalid one, or 'master' which has no buffer limits.";
|
||||
goto loaderr;
|
||||
}
|
||||
hard = memtoull(argv[2],NULL);
|
||||
soft = memtoull(argv[3],NULL);
|
||||
soft_seconds = atoi(argv[4]);
|
||||
if (soft_seconds < 0) {
|
||||
err = "Negative number of seconds in soft limit is invalid";
|
||||
goto loaderr;
|
||||
}
|
||||
server.client_obuf_limits[class].hard_limit_bytes = hard;
|
||||
server.client_obuf_limits[class].soft_limit_bytes = soft;
|
||||
server.client_obuf_limits[class].soft_limit_seconds = soft_seconds;
|
||||
if (updateClientOutputBufferLimit(&argv[1], 4, &err) == C_ERR) goto loaderr;
|
||||
} else if (!strcasecmp(argv[0],"oom-score-adj-values") && argc == 1 + CONFIG_OOM_COUNT) {
|
||||
if (updateOOMScoreAdjValues(&argv[1], &err, 0) == C_ERR) goto loaderr;
|
||||
} else if (!strcasecmp(argv[0],"notify-keyspace-events") && argc == 2) {
|
||||
@ -760,7 +798,6 @@ void loadServerConfig(char *filename, char config_from_stdin, char *options) {
|
||||
void configSetCommand(client *c) {
|
||||
robj *o;
|
||||
long long ll;
|
||||
int err;
|
||||
const char *errstr = NULL;
|
||||
serverAssertWithInfo(c,c->argv[2],sdsEncodedObject(c->argv[2]));
|
||||
serverAssertWithInfo(c,c->argv[3],sdsEncodedObject(c->argv[3]));
|
||||
@ -842,54 +879,14 @@ void configSetCommand(client *c) {
|
||||
return;
|
||||
}
|
||||
} config_set_special_field("client-output-buffer-limit") {
|
||||
int vlen, j;
|
||||
int vlen;
|
||||
sds *v = sdssplitlen(o->ptr,sdslen(o->ptr)," ",1,&vlen);
|
||||
|
||||
/* We need a multiple of 4: <class> <hard> <soft> <soft_seconds> */
|
||||
if (vlen % 4) {
|
||||
sdsfreesplitres(v,vlen);
|
||||
if (updateClientOutputBufferLimit(v, vlen, &errstr) == C_ERR) {
|
||||
sdsfreesplitres(v, vlen);
|
||||
goto badfmt;
|
||||
}
|
||||
|
||||
/* Sanity check of single arguments, so that we either refuse the
|
||||
* whole configuration string or accept it all, even if a single
|
||||
* error in a single client class is present. */
|
||||
for (j = 0; j < vlen; j++) {
|
||||
if ((j % 4) == 0) {
|
||||
int class = getClientTypeByName(v[j]);
|
||||
if (class == -1 || class == CLIENT_TYPE_MASTER)
|
||||
break;
|
||||
} else if ((j % 4) == 3) {
|
||||
char *endptr;
|
||||
long l = strtoll(v[j],&endptr,10);
|
||||
if (l < 0 || *endptr != '\0')
|
||||
break;
|
||||
} else {
|
||||
memtoull(v[j], &err);
|
||||
if (err)
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (j < vlen) {
|
||||
sdsfreesplitres(v,vlen);
|
||||
goto badfmt;
|
||||
}
|
||||
|
||||
/* Finally set the new config */
|
||||
for (j = 0; j < vlen; j += 4) {
|
||||
int class;
|
||||
unsigned long long hard, soft;
|
||||
int soft_seconds;
|
||||
|
||||
class = getClientTypeByName(v[j]);
|
||||
hard = memtoull(v[j+1],NULL);
|
||||
soft = memtoull(v[j+2],NULL);
|
||||
soft_seconds = strtoll(v[j+3],NULL,10);
|
||||
|
||||
server.client_obuf_limits[class].hard_limit_bytes = hard;
|
||||
server.client_obuf_limits[class].soft_limit_bytes = soft;
|
||||
server.client_obuf_limits[class].soft_limit_seconds = soft_seconds;
|
||||
}
|
||||
sdsfreesplitres(v,vlen);
|
||||
} config_set_special_field("oom-score-adj-values") {
|
||||
int vlen;
|
||||
|
||||
@ -1,4 +1,32 @@
|
||||
start_server {tags {"obuf-limits external:skip"}} {
|
||||
test {CONFIG SET client-output-buffer-limit} {
|
||||
set oldval [lindex [r config get client-output-buffer-limit] 1]
|
||||
|
||||
catch {r config set client-output-buffer-limit "wrong number"} e
|
||||
assert_match {*Wrong*arguments*} $e
|
||||
|
||||
catch {r config set client-output-buffer-limit "invalid_class 10mb 10mb 60"} e
|
||||
assert_match {*Invalid*client*class*} $e
|
||||
catch {r config set client-output-buffer-limit "master 10mb 10mb 60"} e
|
||||
assert_match {*Invalid*client*class*} $e
|
||||
|
||||
catch {r config set client-output-buffer-limit "normal 10mbs 10mb 60"} e
|
||||
assert_match {*Error*hard*} $e
|
||||
|
||||
catch {r config set client-output-buffer-limit "replica 10mb 10mbs 60"} e
|
||||
assert_match {*Error*soft*} $e
|
||||
|
||||
catch {r config set client-output-buffer-limit "pubsub 10mb 10mb 60s"} e
|
||||
assert_match {*Error*soft_seconds*} $e
|
||||
|
||||
r config set client-output-buffer-limit "normal 1mb 2mb 60 replica 3mb 4mb 70 pubsub 5mb 6mb 80"
|
||||
set res [lindex [r config get client-output-buffer-limit] 1]
|
||||
assert_equal $res "normal 1048576 2097152 60 slave 3145728 4194304 70 pubsub 5242880 6291456 80"
|
||||
|
||||
# Set back to the original value.
|
||||
r config set client-output-buffer-limit $oldval
|
||||
}
|
||||
|
||||
test {Client output buffer hard limit is enforced} {
|
||||
r config set client-output-buffer-limit {pubsub 100000 0 0}
|
||||
set rd1 [redis_deferring_client]
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user