Unverified Commit ef6f9023 authored by filipe oliveira's avatar filipe oliveira Committed by GitHub
Browse files

redis-benchmark: Error/Warning handling updates. (#8869)

- Immediately exit on errors that are not related to topology updates.
- Deprecates the `-e` option ( retro compatible ) and warns that we now
  exit immediately on errors that are not related to topology updates.
- Fixed wrongfully failing on config fetch error (warning only). This only affects RE.

Bottom line:
- MOVED and ASK errors will not show any warning (unlike the throttled error with `-e` before).
- CLUSTERDOWN still prints an error unconditionally and sleeps for 1 second.
- other errors are fatal.
parent 2b22fffc
...@@ -99,7 +99,6 @@ static struct config { ...@@ -99,7 +99,6 @@ static struct config {
int randomkeys_keyspacelen; int randomkeys_keyspacelen;
int keepalive; int keepalive;
int pipeline; int pipeline;
int showerrors;
long long start; long long start;
long long totlatency; long long totlatency;
const char *title; const char *title;
...@@ -300,7 +299,9 @@ static redisContext *getRedisContext(const char *ip, int port, ...@@ -300,7 +299,9 @@ static redisContext *getRedisContext(const char *ip, int port,
fprintf(stderr, "Node %s:%d replied with error:\n%s\n", ip, port, reply->str); fprintf(stderr, "Node %s:%d replied with error:\n%s\n", ip, port, reply->str);
else else
fprintf(stderr, "Node %s replied with error:\n%s\n", hostsocket, reply->str); fprintf(stderr, "Node %s replied with error:\n%s\n", hostsocket, reply->str);
goto cleanup; freeReplyObject(reply);
redisFree(ctx);
exit(1);
} }
freeReplyObject(reply); freeReplyObject(reply);
return ctx; return ctx;
...@@ -359,9 +360,15 @@ fail: ...@@ -359,9 +360,15 @@ fail:
fprintf(stderr, "ERROR: failed to fetch CONFIG from "); fprintf(stderr, "ERROR: failed to fetch CONFIG from ");
if (hostsocket == NULL) fprintf(stderr, "%s:%d\n", ip, port); if (hostsocket == NULL) fprintf(stderr, "%s:%d\n", ip, port);
else fprintf(stderr, "%s\n", hostsocket); else fprintf(stderr, "%s\n", hostsocket);
int abort_test = 0;
if (!strncmp(reply->str,"NOAUTH",5) ||
!strncmp(reply->str,"WRONGPASS",9) ||
!strncmp(reply->str,"NOPERM",5))
abort_test = 1;
freeReplyObject(reply); freeReplyObject(reply);
redisFree(c); redisFree(c);
freeRedisConfig(cfg); freeRedisConfig(cfg);
if (abort_test) exit(1);
return NULL; return NULL;
} }
static void freeRedisConfig(redisConfig *cfg) { static void freeRedisConfig(redisConfig *cfg) {
...@@ -506,27 +513,13 @@ static void readHandler(aeEventLoop *el, int fd, void *privdata, int mask) { ...@@ -506,27 +513,13 @@ static void readHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
exit(1); exit(1);
} }
redisReply *r = reply; redisReply *r = reply;
int is_err = (r->type == REDIS_REPLY_ERROR); if (r->type == REDIS_REPLY_ERROR) {
if (is_err && config.showerrors) {
/* TODO: static lasterr_time not thread-safe */
static time_t lasterr_time = 0;
time_t now = time(NULL);
if (lasterr_time != now) {
lasterr_time = now;
if (c->cluster_node) {
printf("Error from server %s:%d: %s\n",
c->cluster_node->ip,
c->cluster_node->port,
r->str);
} else printf("Error from server: %s\n", r->str);
}
}
/* Try to update slots configuration if reply error is /* Try to update slots configuration if reply error is
* MOVED/ASK/CLUSTERDOWN and the key(s) used by the command * MOVED/ASK/CLUSTERDOWN and the key(s) used by the command
* contain(s) the slot hash tag. */ * contain(s) the slot hash tag.
if (is_err && c->cluster_node && c->staglen) { * If the error is not topology-update related then we
* immediately exit to avoid false results. */
if (c->cluster_node && c->staglen) {
int fetch_slots = 0, do_wait = 0; int fetch_slots = 0, do_wait = 0;
if (!strncmp(r->str,"MOVED",5) || !strncmp(r->str,"ASK",3)) if (!strncmp(r->str,"MOVED",5) || !strncmp(r->str,"ASK",3))
fetch_slots = 1; fetch_slots = 1;
...@@ -536,7 +529,7 @@ static void readHandler(aeEventLoop *el, int fd, void *privdata, int mask) { ...@@ -536,7 +529,7 @@ static void readHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
* before requesting the new configuration. */ * before requesting the new configuration. */
fetch_slots = 1; fetch_slots = 1;
do_wait = 1; do_wait = 1;
printf("Error from server %s:%d: %s\n", printf("Error from server %s:%d: %s.\n",
c->cluster_node->ip, c->cluster_node->ip,
c->cluster_node->port, c->cluster_node->port,
r->str); r->str);
...@@ -544,6 +537,15 @@ static void readHandler(aeEventLoop *el, int fd, void *privdata, int mask) { ...@@ -544,6 +537,15 @@ static void readHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
if (do_wait) sleep(1); if (do_wait) sleep(1);
if (fetch_slots && !fetchClusterSlotsConfiguration(c)) if (fetch_slots && !fetchClusterSlotsConfiguration(c))
exit(1); exit(1);
} else {
if (c->cluster_node) {
printf("Error from server %s:%d: %s\n",
c->cluster_node->ip,
c->cluster_node->port,
r->str);
} else printf("Error from server: %s\n", r->str);
exit(1);
}
} }
freeReplyObject(reply); freeReplyObject(reply);
...@@ -1286,8 +1288,7 @@ static int fetchClusterSlotsConfiguration(client c) { ...@@ -1286,8 +1288,7 @@ static int fetchClusterSlotsConfiguration(client c) {
atomicGetIncr(config.is_fetching_slots, is_fetching_slots, 1); atomicGetIncr(config.is_fetching_slots, is_fetching_slots, 1);
if (is_fetching_slots) return -1; //TODO: use other codes || errno ? if (is_fetching_slots) return -1; //TODO: use other codes || errno ?
atomicSet(config.is_fetching_slots, 1); atomicSet(config.is_fetching_slots, 1);
if (config.showerrors) printf("WARNING: Cluster slots configuration changed, fetching new one...\n");
printf("Cluster slots configuration changed, fetching new one...\n");
const char *errmsg = "Failed to update cluster slots configuration"; const char *errmsg = "Failed to update cluster slots configuration";
static dictType dtype = { static dictType dtype = {
dictSdsHash, /* hash function */ dictSdsHash, /* hash function */
...@@ -1463,7 +1464,8 @@ int parseOptions(int argc, const char **argv) { ...@@ -1463,7 +1464,8 @@ int parseOptions(int argc, const char **argv) {
} else if (!strcmp(argv[i],"-I")) { } else if (!strcmp(argv[i],"-I")) {
config.idlemode = 1; config.idlemode = 1;
} else if (!strcmp(argv[i],"-e")) { } else if (!strcmp(argv[i],"-e")) {
config.showerrors = 1; printf("WARNING: -e option has been deprecated. "
"We now immediatly exit on error to avoid false results.\n");
} else if (!strcmp(argv[i],"-t")) { } else if (!strcmp(argv[i],"-t")) {
if (lastarg) goto invalid; if (lastarg) goto invalid;
/* We get the list of tests to run as a string in the form /* We get the list of tests to run as a string in the form
...@@ -1566,8 +1568,6 @@ usage: ...@@ -1566,8 +1568,6 @@ usage:
" is executed. Default tests use this to hit random keys in the\n" " is executed. Default tests use this to hit random keys in the\n"
" specified range.\n" " specified range.\n"
" -P <numreq> Pipeline <numreq> requests. Default 1 (no pipeline).\n" " -P <numreq> Pipeline <numreq> requests. Default 1 (no pipeline).\n"
" -e If server replies with errors, show them on stdout.\n"
" (no more than 1 error per second is displayed)\n"
" -q Quiet. Just show query/sec values\n" " -q Quiet. Just show query/sec values\n"
" --precision Number of decimal places to display in latency output (default 0)\n" " --precision Number of decimal places to display in latency output (default 0)\n"
" --csv Output in CSV format\n" " --csv Output in CSV format\n"
...@@ -1692,7 +1692,6 @@ int main(int argc, const char **argv) { ...@@ -1692,7 +1692,6 @@ int main(int argc, const char **argv) {
config.keepalive = 1; config.keepalive = 1;
config.datasize = 3; config.datasize = 3;
config.pipeline = 1; config.pipeline = 1;
config.showerrors = 0;
config.randomkeys = 0; config.randomkeys = 0;
config.randomkeys_keyspacelen = 0; config.randomkeys_keyspacelen = 0;
config.quiet = 0; config.quiet = 0;
...@@ -1777,7 +1776,6 @@ int main(int argc, const char **argv) { ...@@ -1777,7 +1776,6 @@ int main(int argc, const char **argv) {
getRedisConfig(config.hostip, config.hostport, config.hostsocket); getRedisConfig(config.hostip, config.hostport, config.hostsocket);
if (config.redis_config == NULL) { if (config.redis_config == NULL) {
fprintf(stderr, "WARN: could not fetch server CONFIG\n"); fprintf(stderr, "WARN: could not fetch server CONFIG\n");
exit(1);
} }
} }
if (config.num_threads > 0) { if (config.num_threads > 0) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment