You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "proost (via GitHub)" <gi...@apache.org> on 2024/04/12 16:21:12 UTC
[PR] feat: support cluster replicas command [kvrocks]
proost opened a new pull request, #2244:
URL: https://github.com/apache/kvrocks/pull/2244
close: [1942](https://github.com/apache/kvrocks/issues/1942)
Add `CLUSTER REPLICAS` command
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support of the CLUSTER REPLICAS command [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk merged PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support cluster replicas command [kvrocks]
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on code in PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244#discussion_r1563815350
##########
src/cluster/cluster.cc:
##########
@@ -469,6 +469,52 @@ Status Cluster::GetClusterNodes(std::string *nodes_str) {
return Status::OK();
}
+Status Cluster::GetReplicas(const std::string &node_id, std::string *replicas_str) {
+ if (version_ < 0) {
+ return {Status::ClusterDown, errClusterNoInitialized};
+ }
+
+ auto item = nodes_.find(node_id);
+ if (item == nodes_.end()) {
+ return {Status::InvalidArgument, errInvalidNodeID};
+ }
+
+ auto node = item->second;
+ if (node->role != kClusterMaster) {
+ return {Status::InvalidArgument, errNoMasterNode};
+ }
+
+ replicas_str->clear();
+
+ auto now = util::GetTimeStampMS();
+ std::string replicas_desc;
+ for (const auto &replica_id : node->replicas) {
+ auto n = nodes_.find(replica_id);
+ if (n == nodes_.end()) {
+ continue;
+ }
+
+ auto replica = n->second;
+
+ std::string node_str;
+ // ID, host, port
+ node_str.append(replica_id + " ");
+ node_str.append(fmt::format("{}:{}@{} ", replica->host, replica->port, replica->port + kClusterPortIncr));
Review Comment:
[e9726fb](https://github.com/apache/kvrocks/pull/2244/commits/e9726fb4cee176feba13b633818f7a4d1652ddeb)
##########
src/cluster/cluster.cc:
##########
@@ -469,6 +469,52 @@ Status Cluster::GetClusterNodes(std::string *nodes_str) {
return Status::OK();
}
+Status Cluster::GetReplicas(const std::string &node_id, std::string *replicas_str) {
+ if (version_ < 0) {
+ return {Status::ClusterDown, errClusterNoInitialized};
+ }
+
+ auto item = nodes_.find(node_id);
+ if (item == nodes_.end()) {
+ return {Status::InvalidArgument, errInvalidNodeID};
+ }
+
+ auto node = item->second;
+ if (node->role != kClusterMaster) {
+ return {Status::InvalidArgument, errNoMasterNode};
+ }
+
+ replicas_str->clear();
+
+ auto now = util::GetTimeStampMS();
+ std::string replicas_desc;
+ for (const auto &replica_id : node->replicas) {
+ auto n = nodes_.find(replica_id);
+ if (n == nodes_.end()) {
+ continue;
+ }
+
+ auto replica = n->second;
+
+ std::string node_str;
+ // ID, host, port
+ node_str.append(replica_id + " ");
+ node_str.append(fmt::format("{}:{}@{} ", replica->host, replica->port, replica->port + kClusterPortIncr));
+
+ // Flags
+ node_str.append("slave " + replica->master_id + " ");
Review Comment:
[e9726fb](https://github.com/apache/kvrocks/pull/2244/commits/e9726fb4cee176feba13b633818f7a4d1652ddeb)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support of the CLUSTER REPLICAS command [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244#issuecomment-2053592076
@proost Thanks for your contribution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support cluster replicas command [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244#issuecomment-2052768839
Generally LGTM, only one comment is inline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support cluster replicas command [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244#discussion_r1563732465
##########
src/cluster/cluster.cc:
##########
@@ -469,6 +469,52 @@ Status Cluster::GetClusterNodes(std::string *nodes_str) {
return Status::OK();
}
+Status Cluster::GetReplicas(const std::string &node_id, std::string *replicas_str) {
+ if (version_ < 0) {
+ return {Status::ClusterDown, errClusterNoInitialized};
+ }
+
+ auto item = nodes_.find(node_id);
+ if (item == nodes_.end()) {
+ return {Status::InvalidArgument, errInvalidNodeID};
+ }
+
+ auto node = item->second;
+ if (node->role != kClusterMaster) {
+ return {Status::InvalidArgument, errNoMasterNode};
+ }
+
+ replicas_str->clear();
+
+ auto now = util::GetTimeStampMS();
+ std::string replicas_desc;
+ for (const auto &replica_id : node->replicas) {
+ auto n = nodes_.find(replica_id);
+ if (n == nodes_.end()) {
+ continue;
+ }
+
+ auto replica = n->second;
+
+ std::string node_str;
+ // ID, host, port
+ node_str.append(replica_id + " ");
+ node_str.append(fmt::format("{}:{}@{} ", replica->host, replica->port, replica->port + kClusterPortIncr));
Review Comment:
```suggestion
node_str.append(fmt::format("{} {}:{}@{} ", replica_id, replica->host, replica->port, replica->port + kClusterPortIncr));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support cluster replicas command [kvrocks]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244#issuecomment-2052971939
## [](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2244) **Quality Gate failed**
Failed conditions
 [48.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2244&metric=new_coverage&view=list) (required ≥ 50%)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2244)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support cluster replicas command [kvrocks]
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on code in PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244#discussion_r1563815077
##########
src/cluster/cluster.cc:
##########
@@ -469,6 +469,52 @@ Status Cluster::GetClusterNodes(std::string *nodes_str) {
return Status::OK();
}
+Status Cluster::GetReplicas(const std::string &node_id, std::string *replicas_str) {
+ if (version_ < 0) {
+ return {Status::ClusterDown, errClusterNoInitialized};
+ }
+
+ auto item = nodes_.find(node_id);
+ if (item == nodes_.end()) {
+ return {Status::InvalidArgument, errInvalidNodeID};
+ }
+
+ auto node = item->second;
+ if (node->role != kClusterMaster) {
+ return {Status::InvalidArgument, errNoMasterNode};
+ }
+
+ replicas_str->clear();
+
+ auto now = util::GetTimeStampMS();
+ std::string replicas_desc;
+ for (const auto &replica_id : node->replicas) {
+ auto n = nodes_.find(replica_id);
+ if (n == nodes_.end()) {
+ continue;
+ }
+
+ auto replica = n->second;
+
+ std::string node_str;
+ // ID, host, port
+ node_str.append(replica_id + " ");
+ node_str.append(fmt::format("{}:{}@{} ", replica->host, replica->port, replica->port + kClusterPortIncr));
Review Comment:
[c9088c2](https://github.com/apache/kvrocks/pull/2244/commits/c9088c2dd2e7ea5ab206fbb2a707da4b2e18d167)
##########
src/cluster/cluster.cc:
##########
@@ -469,6 +469,52 @@ Status Cluster::GetClusterNodes(std::string *nodes_str) {
return Status::OK();
}
+Status Cluster::GetReplicas(const std::string &node_id, std::string *replicas_str) {
+ if (version_ < 0) {
+ return {Status::ClusterDown, errClusterNoInitialized};
+ }
+
+ auto item = nodes_.find(node_id);
+ if (item == nodes_.end()) {
+ return {Status::InvalidArgument, errInvalidNodeID};
+ }
+
+ auto node = item->second;
+ if (node->role != kClusterMaster) {
+ return {Status::InvalidArgument, errNoMasterNode};
+ }
+
+ replicas_str->clear();
+
+ auto now = util::GetTimeStampMS();
+ std::string replicas_desc;
+ for (const auto &replica_id : node->replicas) {
+ auto n = nodes_.find(replica_id);
+ if (n == nodes_.end()) {
+ continue;
+ }
+
+ auto replica = n->second;
+
+ std::string node_str;
+ // ID, host, port
+ node_str.append(replica_id + " ");
+ node_str.append(fmt::format("{}:{}@{} ", replica->host, replica->port, replica->port + kClusterPortIncr));
Review Comment:
[c9088c2](https://github.com/apache/kvrocks/pull/2244/commits/c9088c2dd2e7ea5ab206fbb2a707da4b2e18d167)
##########
src/cluster/cluster.cc:
##########
@@ -469,6 +469,52 @@ Status Cluster::GetClusterNodes(std::string *nodes_str) {
return Status::OK();
}
+Status Cluster::GetReplicas(const std::string &node_id, std::string *replicas_str) {
Review Comment:
[c9088c2](https://github.com/apache/kvrocks/pull/2244/commits/c9088c2dd2e7ea5ab206fbb2a707da4b2e18d167)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support cluster replicas command [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244#discussion_r1563456569
##########
src/cluster/cluster.cc:
##########
@@ -469,6 +469,52 @@ Status Cluster::GetClusterNodes(std::string *nodes_str) {
return Status::OK();
}
+Status Cluster::GetReplicas(const std::string &node_id, std::string *replicas_str) {
Review Comment:
Can use the StatusOr here:
```C++
StatusOr<std::string> Cluster::GetReplicas(const std::string &node_id) {
...
return replicas_desc;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support of the CLUSTER REPLICAS command [kvrocks]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244#issuecomment-2053593015
## [](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2244) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=2244&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2244&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=2244&resolved=false&inNewCodePeriod=true)
 [51.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2244&metric=new_coverage&view=list)
 [0.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2244&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2244)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support cluster replicas command [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #2244:
URL: https://github.com/apache/kvrocks/pull/2244#discussion_r1563732887
##########
src/cluster/cluster.cc:
##########
@@ -469,6 +469,52 @@ Status Cluster::GetClusterNodes(std::string *nodes_str) {
return Status::OK();
}
+Status Cluster::GetReplicas(const std::string &node_id, std::string *replicas_str) {
+ if (version_ < 0) {
+ return {Status::ClusterDown, errClusterNoInitialized};
+ }
+
+ auto item = nodes_.find(node_id);
+ if (item == nodes_.end()) {
+ return {Status::InvalidArgument, errInvalidNodeID};
+ }
+
+ auto node = item->second;
+ if (node->role != kClusterMaster) {
+ return {Status::InvalidArgument, errNoMasterNode};
+ }
+
+ replicas_str->clear();
+
+ auto now = util::GetTimeStampMS();
+ std::string replicas_desc;
+ for (const auto &replica_id : node->replicas) {
+ auto n = nodes_.find(replica_id);
+ if (n == nodes_.end()) {
+ continue;
+ }
+
+ auto replica = n->second;
+
+ std::string node_str;
+ // ID, host, port
+ node_str.append(replica_id + " ");
+ node_str.append(fmt::format("{}:{}@{} ", replica->host, replica->port, replica->port + kClusterPortIncr));
+
+ // Flags
+ node_str.append("slave " + replica->master_id + " ");
Review Comment:
```suggestion
node_str.append(fmt::format("slave {} ", replica->master_id));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org