From 4f2da00e94aed661942af3ea44782fc61f478e85 Mon Sep 17 00:00:00 2001 From: filipe oliveira Date: Wed, 28 Apr 2021 07:51:07 +0100 Subject: 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. (cherry picked from commit ef6f902372d4646b1894ec5dbd5f857dea5688d6) --- src/redis-benchmark.c | 82 +++++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 068c094e1..51dba9511 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -99,7 +99,6 @@ static struct config { int randomkeys_keyspacelen; int keepalive; int pipeline; - int showerrors; long long start; long long totlatency; const char *title; @@ -307,7 +306,9 @@ static redisContext *getRedisContext(const char *ip, int port, fprintf(stderr, "Node %s:%d replied with error:\n%s\n", ip, port, reply->str); else fprintf(stderr, "Node %s replied with error:\n%s\n", hostsocket, reply->str); - goto cleanup; + freeReplyObject(reply); + redisFree(ctx); + exit(1); } freeReplyObject(reply); return ctx; @@ -366,9 +367,15 @@ fail: fprintf(stderr, "ERROR: failed to fetch CONFIG from "); if (hostsocket == NULL) fprintf(stderr, "%s:%d\n", ip, port); 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); redisFree(c); freeRedisConfig(cfg); + if (abort_test) exit(1); return NULL; } static void freeRedisConfig(redisConfig *cfg) { @@ -513,44 +520,39 @@ static void readHandler(aeEventLoop *el, int fd, void *privdata, int mask) { exit(1); } redisReply *r = reply; - int is_err = (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", + if (r->type == REDIS_REPLY_ERROR) { + /* Try to update slots configuration if reply error is + * MOVED/ASK/CLUSTERDOWN and the key(s) used by the command + * contain(s) the slot hash tag. + * 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; + if (!strncmp(r->str,"MOVED",5) || !strncmp(r->str,"ASK",3)) + fetch_slots = 1; + else if (!strncmp(r->str,"CLUSTERDOWN",11)) { + /* Usually the cluster is able to recover itself after + * a CLUSTERDOWN error, so try to sleep one second + * before requesting the new configuration. */ + fetch_slots = 1; + do_wait = 1; + printf("Error from server %s:%d: %s.\n", c->cluster_node->ip, c->cluster_node->port, r->str); + } + if (do_wait) sleep(1); + if (fetch_slots && !fetchClusterSlotsConfiguration(c)) + 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); - } - } - - /* Try to update slots configuration if reply error is - * MOVED/ASK/CLUSTERDOWN and the key(s) used by the command - * contain(s) the slot hash tag. */ - if (is_err && c->cluster_node && c->staglen) { - int fetch_slots = 0, do_wait = 0; - if (!strncmp(r->str,"MOVED",5) || !strncmp(r->str,"ASK",3)) - fetch_slots = 1; - else if (!strncmp(r->str,"CLUSTERDOWN",11)) { - /* Usually the cluster is able to recover itself after - * a CLUSTERDOWN error, so try to sleep one second - * before requesting the new configuration. */ - fetch_slots = 1; - do_wait = 1; - printf("Error from server %s:%d: %s\n", - c->cluster_node->ip, - c->cluster_node->port, - r->str); - } - if (do_wait) sleep(1); - if (fetch_slots && !fetchClusterSlotsConfiguration(c)) exit(1); + } } freeReplyObject(reply); @@ -1293,8 +1295,7 @@ static int fetchClusterSlotsConfiguration(client c) { atomicGetIncr(config.is_fetching_slots, is_fetching_slots, 1); if (is_fetching_slots) return -1; //TODO: use other codes || errno ? atomicSet(config.is_fetching_slots, 1); - if (config.showerrors) - printf("Cluster slots configuration changed, fetching new one...\n"); + printf("WARNING: Cluster slots configuration changed, fetching new one...\n"); const char *errmsg = "Failed to update cluster slots configuration"; static dictType dtype = { dictSdsHash, /* hash function */ @@ -1470,7 +1471,8 @@ int parseOptions(int argc, const char **argv) { } else if (!strcmp(argv[i],"-I")) { config.idlemode = 1; } 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")) { if (lastarg) goto invalid; /* We get the list of tests to run as a string in the form @@ -1573,8 +1575,6 @@ usage: " is executed. Default tests use this to hit random keys in the\n" " specified range.\n" " -P Pipeline 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" " --precision Number of decimal places to display in latency output (default 0)\n" " --csv Output in CSV format\n" @@ -1699,7 +1699,6 @@ int main(int argc, const char **argv) { config.keepalive = 1; config.datasize = 3; config.pipeline = 1; - config.showerrors = 0; config.randomkeys = 0; config.randomkeys_keyspacelen = 0; config.quiet = 0; @@ -1784,7 +1783,6 @@ int main(int argc, const char **argv) { getRedisConfig(config.hostip, config.hostport, config.hostsocket); if (config.redis_config == NULL) { fprintf(stderr, "WARN: could not fetch server CONFIG\n"); - exit(1); } } if (config.num_threads > 0) { -- cgit v1.2.1