[kvrocks] branch unstable updated: Fix data race when updating the slots_info string (#1615)

[twice]

This is an automated email from the ASF dual-hosted git repository.

twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new f3d3105b Fix data race when updating the slots_info string (#1615)
f3d3105b is described below

commit f3d3105b5ad1a6b276cfc305a57e7cc5233e4fed
Author: hulk <hulk.webs...@gmail.com>
AuthorDate: Sat Jul 29 11:37:38 2023 +0800

    Fix data race when updating the slots_info string (#1615)
    
    Co-authored-by: Twice <tw...@apache.org>
---
 src/cluster/cluster.cc | 37 ++++++++++++++++++++-----------------
 src/cluster/cluster.h  |  3 +--
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/cluster/cluster.cc b/src/cluster/cluster.cc
index e1b4dbe6..b1596928 100644
--- a/src/cluster/cluster.cc
+++ b/src/cluster/cluster.cc
@@ -472,7 +472,7 @@ Status Cluster::GetClusterNodes(std::string *nodes_str) {
 }
 
 std::string Cluster::genNodesDescription() {
-  updateSlotsInfo();
+  auto slots_infos = getClusterNodeSlots();
 
   auto now = util::GetTimeStampMS();
   std::string nodes_desc;
@@ -495,8 +495,11 @@ std::string Cluster::genNodesDescription() {
     // Ping sent, pong received, config epoch, link status
     node_str.append(fmt::format("{} {} {} connected", now - 1, now, version_));
 
-    if (n->role == kClusterMaster && n->slots_info.size() > 0) {
-      node_str.append(" " + n->slots_info);
+    if (n->role == kClusterMaster) {
+      auto iter = slots_infos.find(n->id);
+      if (iter != slots_infos.end() && iter->second.size() > 0) {
+        node_str.append(" " + iter->second);
+      }
     }
 
     nodes_desc.append(node_str + "\n");
@@ -504,13 +507,10 @@ std::string Cluster::genNodesDescription() {
   return nodes_desc;
 }
 
-void Cluster::updateSlotsInfo() {
+std::map<std::string, std::string> Cluster::getClusterNodeSlots() const {
   int start = -1;
-  // reset the previous slots info
-  for (const auto &item : nodes_) {
-    const std::shared_ptr<ClusterNode> &n = item.second;
-    n->slots_info.clear();
-  }
+  // node id => slots info string
+  std::map<std::string, std::string> slots_infos;
 
   std::shared_ptr<ClusterNode> n = nullptr;
   for (int i = 0; i <= kClusterSlots; i++) {
@@ -524,9 +524,9 @@ void Cluster::updateSlotsInfo() {
     // Generate slots info when occur different node with start or end of slot
     if (i == kClusterSlots || n != slots_nodes_[i]) {
       if (start == i - 1) {
-        n->slots_info += fmt::format("{} ", start);
+        slots_infos[n->id] += fmt::format("{} ", start);
       } else {
-        n->slots_info += fmt::format("{}-{} ", start, i - 1);
+        slots_infos[n->id] += fmt::format("{}-{} ", start, i - 1);
       }
       if (i == kClusterSlots) break;
       n = slots_nodes_[i];
@@ -534,14 +534,14 @@ void Cluster::updateSlotsInfo() {
     }
   }
 
-  for (const auto &item : nodes_) {
-    const std::shared_ptr<ClusterNode> n = item.second;
-    if (n->slots_info.size() > 0) n->slots_info.pop_back();  // Remove last 
space
+  for (auto &[_, info] : slots_infos) {
+    if (info.size() > 0) info.pop_back();  // Remove last space
   }
+  return slots_infos;
 }
 
 std::string Cluster::genNodesInfo() {
-  updateSlotsInfo();
+  auto slots_infos = getClusterNodeSlots();
 
   std::string nodes_info;
   for (const auto &item : nodes_) {
@@ -561,8 +561,11 @@ std::string Cluster::genNodesInfo() {
     }
 
     // Slots
-    if (n->role == kClusterMaster && n->slots_info.size() > 0) {
-      node_str.append(" " + n->slots_info);
+    if (n->role == kClusterMaster) {
+      auto iter = slots_infos.find(n->id);
+      if (iter != slots_infos.end() && iter->second.size() > 0) {
+        node_str.append(" " + iter->second);
+      }
     }
     nodes_info.append(node_str + "\n");
   }
diff --git a/src/cluster/cluster.h b/src/cluster/cluster.h
index b2923efa..8cad0c4f 100644
--- a/src/cluster/cluster.h
+++ b/src/cluster/cluster.h
@@ -45,7 +45,6 @@ class ClusterNode {
   int port;
   int role;
   std::string master_id;
-  std::string slots_info;
   std::bitset<kClusterSlots> slots;
   std::vector<std::string> replicas;
   int importing_slot = -1;
@@ -96,7 +95,7 @@ class Cluster {
  private:
   std::string genNodesDescription();
   std::string genNodesInfo();
-  void updateSlotsInfo();
+  std::map<std::string, std::string> getClusterNodeSlots() const;
   SlotInfo genSlotNodeInfo(int start, int end, const 
std::shared_ptr<ClusterNode> &n);
   static Status parseClusterNodes(const std::string &nodes_str, ClusterNodes 
*nodes,
                                   std::unordered_map<int, std::string> 
*slots_nodes);