mapleFU commented on code in PR #1219:
URL: 
https://github.com/apache/incubator-kvrocks/pull/1219#discussion_r1064633971


##########
src/cluster/cluster.cc:
##########
@@ -511,37 +552,122 @@ std::string Cluster::GenNodesDescription() {
     }
   }
 
-  std::string nodes_desc;
   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
+  }
+}
 
+std::string Cluster::GenNodesInfo() {
+  UpdateSlotsInfo();
+
+  std::string nodes_info;
+  for (const auto &item : nodes_) {
+    const std::shared_ptr<ClusterNode> n = item.second;
     std::string node_str;
-    // ID, host, port
+    // ID
     node_str.append(n->id_ + " ");
-    node_str.append(fmt::format("{}:{}@{} ", n->host_, n->port_, n->port_ + 
kClusterPortIncr));
+    // Host + Port
+    node_str.append(fmt::format("{} {} ", n->host_, n->port_));
 
-    // Flags
-    if (n->id_ == myid_) node_str.append("myself,");
+    // Role
     if (n->role_ == kClusterMaster) {
       node_str.append("master - ");
     } else {
       node_str.append("slave " + n->master_id_ + " ");
     }
 
-    // Ping sent, pong received, config epoch, link status
-    auto now = Util::GetTimeStampMS();
-    node_str.append(fmt::format("{} {} {} connected", now - 1, now, version_));
-
     // Slots
-    if (n->slots_info_.size() > 0) n->slots_info_.pop_back();  // Trim space
     if (n->role_ == kClusterMaster && n->slots_info_.size() > 0) {
       node_str.append(" " + n->slots_info_);
     }
-    n->slots_info_.clear();  // Reset
+    nodes_info.append(node_str + "\n");
+  }
+  return nodes_info;
+}
 
-    nodes_desc.append(node_str + "\n");
+Status Cluster::DumpClusterNodes(const std::string &file) {
+  // Parse and validate the cluster nodes string before dumping into file
+  std::string tmp_path = file + ".tmp";
+  remove(tmp_path.data());
+  std::ofstream output_file(tmp_path, std::ios::out);
+  output_file << "# version\n";
+  output_file << fmt::format("{}\n", version_);
+  output_file << "# id\n";
+  output_file << fmt::format("{}\n", myid_);
+  output_file << "# nodes\n";
+  output_file << GenNodesInfo();
+  output_file.close();
+  if (rename(tmp_path.data(), file.data()) < 0) {
+    return {Status::NotOK, fmt::format("rename file encounter error: {}", 
strerror(errno))};
   }
-  return nodes_desc;
+  return Status::OK();
+}
+
+LoadState parseLoadState(const std::string &in) {
+  if (in == "version") {
+    return LoadState::Version;
+  } else if (in == "id") {
+    return LoadState::ID;
+  } else if (in == "nodes") {
+    return LoadState::NodesInfo;
+  } else {
+    return LoadState::Unknown;
+  }
+}
+
+Status Cluster::LoadClusterNodes(const std::string &file_path) {
+  if (rocksdb::Env::Default()->FileExists(file_path).IsNotFound()) {
+    LOG(INFO) << fmt::format("The cluster nodes file {} is not found. Use 
CLUSTERX subcommands to specify it.",
+                             file_path);
+    return Status::OK();
+  }
+
+  std::ifstream file;
+  file.open(file_path);
+  if (!file.is_open()) {
+    return {Status::NotOK, fmt::format("error opening the file '{}': {}", 
file_path, strerror(errno))};
+  }
+
+  int64_t version = -1;
+  std::string id, nodesInfo;
+  LoadState state = LoadState::Unknown;
+  std::string line;
+  while (!file.eof()) {
+    std::getline(file, line);
+    line = Util::Trim(line, " \t");
+    if (line.empty()) continue;  // skip empty line
+
+    if (line[0] == '#') {
+      std::string comment = Util::ToLower(Util::Trim(line, "# \t"));
+      state = parseLoadState(comment);
+      continue;
+    }
+
+    switch (state) {
+      case LoadState::Version: {
+        auto parse_result = ParseInt<int64_t>(line, 10);
+        if (!parse_result) {
+          return {Status::NotOK, errInvalidClusterVersion};
+        }
+        version = *parse_result;
+        break;
+      }
+      case LoadState::ID:
+        id = line;
+        if (id.length() != kClusterNodeIdLen) {
+          return {Status::NotOK, errInvalidNodeID};
+        }
+        break;
+      case LoadState::NodesInfo:
+        // std::getline swallowed the newline in the node info, so need to add 
it back here
+        nodesInfo.append(line + "\n");
+        break;
+      default:
+        return {Status::NotOK, "got unknown parse state"};

Review Comment:
   > It should make sense that the older version can't parse new state.
   
   It's ok but, if a user want to rollback(maybe because some other reason), 
and the protocol is updated. He will failed to start, unless he delete the file.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to