fix updatePart (vesoft-inc#795)

* fix updatePart * fix format check Co-authored-by: Sophie <[email protected]> Co-authored-by: Doodle <[email protected]> Co-authored-by: Sophie <[email protected]>
liwenhui-soul · Apr 12, 2022 · f19e716 · f19e716
1 parent 4e12d32
commit f19e716
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 10 deletions.
diff --git a/src/kvstore/KVEngine.h b/src/kvstore/KVEngine.h
@@ -232,8 +232,9 @@ class KVEngine {
    *
    * @param partId
    * @param raftPeer
+   * @return nebula::cpp2::ErrorCode
    */
-  virtual void updatePart(PartitionID partId, const Peer& raftPeer) = 0;
+  virtual nebula::cpp2::ErrorCode updatePart(PartitionID partId, const Peer& raftPeer) = 0;
 
   /**
    * @brief Remove a partition from kv engine

diff --git a/src/kvstore/Part.cpp b/src/kvstore/Part.cpp
@@ -448,7 +448,10 @@ bool Part::preProcessLog(LogID logId, TermID termId, ClusterID clusterId, const
           VLOG(1) << idStr_ << "preprocess add learner " << learner;
           addLearner(learner, false);
           // persist the part learner info in case of storaged restarting
-          engine_->updatePart(partId_, Peer(learner, Peer::Status::kLearner));
+          auto ret = engine_->updatePart(partId_, Peer(learner, Peer::Status::kLearner));
+          if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) {
+            return false;
+          }
         } else {
           VLOG(1) << idStr_ << "Skip stale add learner " << learner << ", the part is opened at "
                   << startTimeMs_ << ", but the log timestamp is " << ts;
@@ -474,7 +477,10 @@ bool Part::preProcessLog(LogID logId, TermID termId, ClusterID clusterId, const
         if (ts > startTimeMs_) {
           VLOG(1) << idStr_ << "preprocess add peer " << peer;
           addPeer(peer);
-          engine_->updatePart(partId_, Peer(peer, Peer::Status::kPromotedPeer));
+          auto ret = engine_->updatePart(partId_, Peer(peer, Peer::Status::kPromotedPeer));
+          if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) {
+            return false;
+          }
         } else {
           VLOG(1) << idStr_ << "Skip stale add peer " << peer << ", the part is opened at "
                   << startTimeMs_ << ", but the log timestamp is " << ts;
@@ -488,7 +494,10 @@ bool Part::preProcessLog(LogID logId, TermID termId, ClusterID clusterId, const
           VLOG(1) << idStr_ << "preprocess remove peer " << peer;
           preProcessRemovePeer(peer);
           // remove peer in the persist info
-          engine_->updatePart(partId_, Peer(peer, Peer::Status::kDeleted));
+          auto ret = engine_->updatePart(partId_, Peer(peer, Peer::Status::kDeleted));
+          if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) {
+            return false;
+          }
         } else {
           VLOG(1) << idStr_ << "Skip stale remove peer " << peer << ", the part is opened at "
                   << startTimeMs_ << ", but the log timestamp is " << ts;

diff --git a/src/kvstore/RocksEngine.cpp b/src/kvstore/RocksEngine.cpp
@@ -339,18 +339,25 @@ void RocksEngine::addPart(PartitionID partId, const Peers& raftPeers) {
   }
 }
 
-void RocksEngine::updatePart(PartitionID partId, const Peer& raftPeer) {
+nebula::cpp2::ErrorCode RocksEngine::updatePart(PartitionID partId, const Peer& raftPeer) {
   std::string val;
   auto ret = get(balanceKey(partId), &val);
-  if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) {
+
+  Peers peers;
+  if (ret == nebula::cpp2::ErrorCode::SUCCEEDED) {
+    peers = Peers::fromString(val);
+  } else if (ret == nebula::cpp2::ErrorCode::E_KEY_NOT_FOUND) {
+    // do nothing
+  } else {
     LOG(INFO) << "Update part failed when get, partId=" << partId;
-    return;
+    return ret;
   }
 
-  auto peers = Peers::fromString(val);
   peers.addOrUpdate(raftPeer);
 
   if (peers.allNormalPeers()) {
+    // When all replica become normal peers, delete this temp key. (when learner is promoted to
+    // normal replica)
     ret = remove(balanceKey(partId));
   } else {
     ret = put(balanceKey(partId), peers.toString());
@@ -359,6 +366,7 @@ void RocksEngine::updatePart(PartitionID partId, const Peer& raftPeer) {
   if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) {
     LOG(INFO) << "Update part failed when put back, partId=" << partId;
   }
+  return ret;
 }
 
 void RocksEngine::removePart(PartitionID partId) {

diff --git a/src/kvstore/RocksEngine.h b/src/kvstore/RocksEngine.h
@@ -405,9 +405,9 @@ class RocksEngine : public KVEngine {
    *
    * @param partId
    * @param raftPeer
-   *
+   * @return nebula::cpp2::ErrorCode
    */
-  void updatePart(PartitionID partId, const Peer& raftPeer) override;
+  nebula::cpp2::ErrorCode updatePart(PartitionID partId, const Peer& raftPeer) override;
 
   /**
    * @brief Remove the part key from rocksdb