Unverified Commit 1df8c129 authored by Huang Zhw's avatar Huang Zhw Committed by GitHub
Browse files

Fix some minor bugs in redis-cli (#8982)

* clusterManagerAddSlots check argv_idx error.

* clusterManagerLoadInfoFromNode remove unused param opts.

* redis-cli node->ip may be an sds or a c string. Using %s to format
is always right, %S may be wrong.

* In clusterManagerFixOpenSlot clusterManagerBumpEpoch call is redundant,
because it is already called in clusterManagerSetSlotOwner.

* redis-cli cluster help add more commands in help messages.
parent 39b0f0dd
...@@ -2498,7 +2498,7 @@ static void clusterManagerPrintNotClusterNodeError(clusterManagerNode *node, ...@@ -2498,7 +2498,7 @@ static void clusterManagerPrintNotClusterNodeError(clusterManagerNode *node,
char *err); char *err);
static int clusterManagerNodeLoadInfo(clusterManagerNode *node, int opts, static int clusterManagerNodeLoadInfo(clusterManagerNode *node, int opts,
char **err); char **err);
static int clusterManagerLoadInfoFromNode(clusterManagerNode *node, int opts); static int clusterManagerLoadInfoFromNode(clusterManagerNode *node);
static int clusterManagerNodeIsEmpty(clusterManagerNode *node, char **err); static int clusterManagerNodeIsEmpty(clusterManagerNode *node, char **err);
static int clusterManagerGetAntiAffinityScore(clusterManagerNodeArray *ipnodes, static int clusterManagerGetAntiAffinityScore(clusterManagerNodeArray *ipnodes,
int ip_count, clusterManagerNode ***offending, int *offending_len); int ip_count, clusterManagerNode ***offending, int *offending_len);
...@@ -3424,7 +3424,7 @@ static int clusterManagerAddSlots(clusterManagerNode *node, char**err) ...@@ -3424,7 +3424,7 @@ static int clusterManagerAddSlots(clusterManagerNode *node, char**err)
argv_idx++; argv_idx++;
} }
} }
if (!argv_idx) { if (argv_idx == 2) {
success = 0; success = 0;
goto cleanup; goto cleanup;
} }
...@@ -4260,12 +4260,11 @@ cleanup: ...@@ -4260,12 +4260,11 @@ cleanup:
* point. All nodes will be loaded inside the cluster_manager.nodes list. * point. All nodes will be loaded inside the cluster_manager.nodes list.
* Warning: if something goes wrong, it will free the starting node before * Warning: if something goes wrong, it will free the starting node before
* returning 0. */ * returning 0. */
static int clusterManagerLoadInfoFromNode(clusterManagerNode *node, int opts) { static int clusterManagerLoadInfoFromNode(clusterManagerNode *node) {
if (node->context == NULL && !clusterManagerNodeConnect(node)) { if (node->context == NULL && !clusterManagerNodeConnect(node)) {
freeClusterManagerNode(node); freeClusterManagerNode(node);
return 0; return 0;
} }
opts |= CLUSTER_MANAGER_OPT_GETFRIENDS;
char *e = NULL; char *e = NULL;
if (!clusterManagerNodeIsCluster(node, &e)) { if (!clusterManagerNodeIsCluster(node, &e)) {
clusterManagerPrintNotClusterNodeError(node, e); clusterManagerPrintNotClusterNodeError(node, e);
...@@ -4274,7 +4273,7 @@ static int clusterManagerLoadInfoFromNode(clusterManagerNode *node, int opts) { ...@@ -4274,7 +4273,7 @@ static int clusterManagerLoadInfoFromNode(clusterManagerNode *node, int opts) {
return 0; return 0;
} }
e = NULL; e = NULL;
if (!clusterManagerNodeLoadInfo(node, opts, &e)) { if (!clusterManagerNodeLoadInfo(node, CLUSTER_MANAGER_OPT_GETFRIENDS, &e)) {
if (e) { if (e) {
CLUSTER_MANAGER_PRINT_REPLY_ERROR(node, e); CLUSTER_MANAGER_PRINT_REPLY_ERROR(node, e);
zfree(e); zfree(e);
...@@ -4990,7 +4989,7 @@ static int clusterManagerFixOpenSlot(int slot) { ...@@ -4990,7 +4989,7 @@ static int clusterManagerFixOpenSlot(int slot) {
"in node %s:%d!\n", slot, n->ip, "in node %s:%d!\n", slot, n->ip,
n->port); n->port);
char *sep = (listLength(importing) == 0 ? "" : ","); char *sep = (listLength(importing) == 0 ? "" : ",");
importing_str = sdscatfmt(importing_str, "%s%S:%u", importing_str = sdscatfmt(importing_str, "%s%s:%u",
sep, n->ip, n->port); sep, n->ip, n->port);
listAddNodeTail(importing, n); listAddNodeTail(importing, n);
} }
...@@ -5028,11 +5027,6 @@ static int clusterManagerFixOpenSlot(int slot) { ...@@ -5028,11 +5027,6 @@ static int clusterManagerFixOpenSlot(int slot) {
/* Since CLUSTER ADDSLOTS succeeded, we also update the slot /* Since CLUSTER ADDSLOTS succeeded, we also update the slot
* info into the node struct, in order to keep it synced */ * info into the node struct, in order to keep it synced */
owner->slots[slot] = 1; owner->slots[slot] = 1;
/* Make sure this information will propagate. Not strictly needed
* since there is no past owner, so all the other nodes will accept
* whatever epoch this node will claim the slot with. */
success = clusterManagerBumpEpoch(owner);
if (!success) goto cleanup;
/* Remove the owner from the list of migrating/importing /* Remove the owner from the list of migrating/importing
* nodes. */ * nodes. */
clusterManagerRemoveNodeFromList(migrating, owner); clusterManagerRemoveNodeFromList(migrating, owner);
...@@ -5872,7 +5866,7 @@ assign_replicas: ...@@ -5872,7 +5866,7 @@ assign_replicas:
else freeClusterManagerNode(node); else freeClusterManagerNode(node);
} }
listEmpty(cluster_manager.nodes); listEmpty(cluster_manager.nodes);
if (!clusterManagerLoadInfoFromNode(first_node, 0)) { if (!clusterManagerLoadInfoFromNode(first_node)) {
success = 0; success = 0;
goto cleanup; goto cleanup;
} }
...@@ -5903,7 +5897,7 @@ static int clusterManagerCommandAddNode(int argc, char **argv) { ...@@ -5903,7 +5897,7 @@ static int clusterManagerCommandAddNode(int argc, char **argv) {
ref_ip, ref_port); ref_ip, ref_port);
// Check the existing cluster // Check the existing cluster
clusterManagerNode *refnode = clusterManagerNewNode(ref_ip, ref_port); clusterManagerNode *refnode = clusterManagerNewNode(ref_ip, ref_port);
if (!clusterManagerLoadInfoFromNode(refnode, 0)) return 0; if (!clusterManagerLoadInfoFromNode(refnode)) return 0;
if (!clusterManagerCheckCluster(0)) return 0; if (!clusterManagerCheckCluster(0)) return 0;
/* If --cluster-master-id was specified, try to resolve it now so that we /* If --cluster-master-id was specified, try to resolve it now so that we
...@@ -6000,7 +5994,7 @@ static int clusterManagerCommandDeleteNode(int argc, char **argv) { ...@@ -6000,7 +5994,7 @@ static int clusterManagerCommandDeleteNode(int argc, char **argv) {
clusterManagerNode *node = NULL; clusterManagerNode *node = NULL;
// Load cluster information // Load cluster information
if (!clusterManagerLoadInfoFromNode(ref_node, 0)) return 0; if (!clusterManagerLoadInfoFromNode(ref_node)) return 0;
// Check if the node exists and is not empty // Check if the node exists and is not empty
node = clusterManagerNodeByName(node_id); node = clusterManagerNodeByName(node_id);
...@@ -6059,7 +6053,7 @@ static int clusterManagerCommandInfo(int argc, char **argv) { ...@@ -6059,7 +6053,7 @@ static int clusterManagerCommandInfo(int argc, char **argv) {
char *ip = NULL; char *ip = NULL;
if (!getClusterHostFromCmdArgs(argc, argv, &ip, &port)) goto invalid_args; if (!getClusterHostFromCmdArgs(argc, argv, &ip, &port)) goto invalid_args;
clusterManagerNode *node = clusterManagerNewNode(ip, port); clusterManagerNode *node = clusterManagerNewNode(ip, port);
if (!clusterManagerLoadInfoFromNode(node, 0)) return 0; if (!clusterManagerLoadInfoFromNode(node)) return 0;
clusterManagerShowClusterInfo(); clusterManagerShowClusterInfo();
return 1; return 1;
invalid_args: invalid_args:
...@@ -6072,7 +6066,7 @@ static int clusterManagerCommandCheck(int argc, char **argv) { ...@@ -6072,7 +6066,7 @@ static int clusterManagerCommandCheck(int argc, char **argv) {
char *ip = NULL; char *ip = NULL;
if (!getClusterHostFromCmdArgs(argc, argv, &ip, &port)) goto invalid_args; if (!getClusterHostFromCmdArgs(argc, argv, &ip, &port)) goto invalid_args;
clusterManagerNode *node = clusterManagerNewNode(ip, port); clusterManagerNode *node = clusterManagerNewNode(ip, port);
if (!clusterManagerLoadInfoFromNode(node, 0)) return 0; if (!clusterManagerLoadInfoFromNode(node)) return 0;
clusterManagerShowClusterInfo(); clusterManagerShowClusterInfo();
return clusterManagerCheckCluster(0); return clusterManagerCheckCluster(0);
invalid_args: invalid_args:
...@@ -6090,7 +6084,7 @@ static int clusterManagerCommandReshard(int argc, char **argv) { ...@@ -6090,7 +6084,7 @@ static int clusterManagerCommandReshard(int argc, char **argv) {
char *ip = NULL; char *ip = NULL;
if (!getClusterHostFromCmdArgs(argc, argv, &ip, &port)) goto invalid_args; if (!getClusterHostFromCmdArgs(argc, argv, &ip, &port)) goto invalid_args;
clusterManagerNode *node = clusterManagerNewNode(ip, port); clusterManagerNode *node = clusterManagerNewNode(ip, port);
if (!clusterManagerLoadInfoFromNode(node, 0)) return 0; if (!clusterManagerLoadInfoFromNode(node)) return 0;
clusterManagerCheckCluster(0); clusterManagerCheckCluster(0);
if (cluster_manager.errors && listLength(cluster_manager.errors) > 0) { if (cluster_manager.errors && listLength(cluster_manager.errors) > 0) {
fflush(stdout); fflush(stdout);
...@@ -6279,7 +6273,7 @@ static int clusterManagerCommandRebalance(int argc, char **argv) { ...@@ -6279,7 +6273,7 @@ static int clusterManagerCommandRebalance(int argc, char **argv) {
list *involved = NULL; list *involved = NULL;
if (!getClusterHostFromCmdArgs(argc, argv, &ip, &port)) goto invalid_args; if (!getClusterHostFromCmdArgs(argc, argv, &ip, &port)) goto invalid_args;
clusterManagerNode *node = clusterManagerNewNode(ip, port); clusterManagerNode *node = clusterManagerNewNode(ip, port);
if (!clusterManagerLoadInfoFromNode(node, 0)) return 0; if (!clusterManagerLoadInfoFromNode(node)) return 0;
int result = 1, i; int result = 1, i;
if (config.cluster_manager_command.weight != NULL) { if (config.cluster_manager_command.weight != NULL) {
for (i = 0; i < config.cluster_manager_command.weight_argc; i++) { for (i = 0; i < config.cluster_manager_command.weight_argc; i++) {
...@@ -6474,7 +6468,7 @@ static int clusterManagerCommandSetTimeout(int argc, char **argv) { ...@@ -6474,7 +6468,7 @@ static int clusterManagerCommandSetTimeout(int argc, char **argv) {
} }
// Load cluster information // Load cluster information
clusterManagerNode *node = clusterManagerNewNode(ip, port); clusterManagerNode *node = clusterManagerNewNode(ip, port);
if (!clusterManagerLoadInfoFromNode(node, 0)) return 0; if (!clusterManagerLoadInfoFromNode(node)) return 0;
int ok_count = 0, err_count = 0; int ok_count = 0, err_count = 0;
clusterManagerLogInfo(">>> Reconfiguring node timeout in every " clusterManagerLogInfo(">>> Reconfiguring node timeout in every "
...@@ -6544,7 +6538,7 @@ static int clusterManagerCommandImport(int argc, char **argv) { ...@@ -6544,7 +6538,7 @@ static int clusterManagerCommandImport(int argc, char **argv) {
src_ip, src_port, ip, port); src_ip, src_port, ip, port);
clusterManagerNode *refnode = clusterManagerNewNode(ip, port); clusterManagerNode *refnode = clusterManagerNewNode(ip, port);
if (!clusterManagerLoadInfoFromNode(refnode, 0)) return 0; if (!clusterManagerLoadInfoFromNode(refnode)) return 0;
if (!clusterManagerCheckCluster(0)) return 0; if (!clusterManagerCheckCluster(0)) return 0;
char *reply_err = NULL; char *reply_err = NULL;
redisReply *src_reply = NULL; redisReply *src_reply = NULL;
...@@ -6679,7 +6673,7 @@ static int clusterManagerCommandCall(int argc, char **argv) { ...@@ -6679,7 +6673,7 @@ static int clusterManagerCommandCall(int argc, char **argv) {
char *ip = NULL; char *ip = NULL;
if (!getClusterHostFromCmdArgs(1, argv, &ip, &port)) goto invalid_args; if (!getClusterHostFromCmdArgs(1, argv, &ip, &port)) goto invalid_args;
clusterManagerNode *refnode = clusterManagerNewNode(ip, port); clusterManagerNode *refnode = clusterManagerNewNode(ip, port);
if (!clusterManagerLoadInfoFromNode(refnode, 0)) return 0; if (!clusterManagerLoadInfoFromNode(refnode)) return 0;
argc--; argc--;
argv++; argv++;
size_t *argvlen = zmalloc(argc*sizeof(size_t)); size_t *argvlen = zmalloc(argc*sizeof(size_t));
...@@ -6724,7 +6718,7 @@ static int clusterManagerCommandBackup(int argc, char **argv) { ...@@ -6724,7 +6718,7 @@ static int clusterManagerCommandBackup(int argc, char **argv) {
char *ip = NULL; char *ip = NULL;
if (!getClusterHostFromCmdArgs(1, argv, &ip, &port)) goto invalid_args; if (!getClusterHostFromCmdArgs(1, argv, &ip, &port)) goto invalid_args;
clusterManagerNode *refnode = clusterManagerNewNode(ip, port); clusterManagerNode *refnode = clusterManagerNewNode(ip, port);
if (!clusterManagerLoadInfoFromNode(refnode, 0)) return 0; if (!clusterManagerLoadInfoFromNode(refnode)) return 0;
int no_issues = clusterManagerCheckCluster(0); int no_issues = clusterManagerCheckCluster(0);
int cluster_errors_count = (no_issues ? 0 : int cluster_errors_count = (no_issues ? 0 :
listLength(cluster_manager.errors)); listLength(cluster_manager.errors));
...@@ -6816,7 +6810,8 @@ static int clusterManagerCommandHelp(int argc, char **argv) { ...@@ -6816,7 +6810,8 @@ static int clusterManagerCommandHelp(int argc, char **argv) {
} }
} }
} }
fprintf(stderr, "\nFor check, fix, reshard, del-node, set-timeout you " fprintf(stderr, "\nFor check, fix, reshard, del-node, set-timeout, "
"info, rebalance, call, import, backup you "
"can specify the host and port of any working node in " "can specify the host and port of any working node in "
"the cluster.\n"); "the cluster.\n");
......
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