git-hulk commented on code in PR #1219:
URL: 
https://github.com/apache/incubator-kvrocks/pull/1219#discussion_r1064538611


##########
src/server/server.cc:
##########
@@ -137,12 +137,17 @@ Status Server::Start() {
   }
 
   if (config_->cluster_enabled) {
+    auto s = cluster_->LoadClusterNodes(config_->NodesFilePath());

Review Comment:
   Hi @mapleFU, thanks for your warm review. 
   
   Currently, the cluster topology is expected to maintain out of the Kvrocks 
node(Like kvrocks-controller) and the external component will re-sync the 
cluster topology if it's changed. 
   
   So for question 1/2, it's not a problem since the external component will 
re-sync if it's changed or lost.
   
   For question 3, we can regard this topology as a local cache, the external 
controller component will re-sync the cluster topology and won't depend on this 
persistent topology.
   
   Briefly, this PR is only to cache the cluster topo in the local disk, so it 
will bring the side effect is the user may get stale data if the client cached 
the topology and the controller failed to update the topology in time. But I 
think it's better than before. 
   
   
   
   



-- 
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