Re: [PR] [Region Migration] Add md5 check for IoTConsensus snapshot transmission & (for ratis) Prefer not choose new peer as new leader [iotdb]

["HxpSerein (via GitHub)" <gi...@apache.org>]

HxpSerein commented on code in PR #13111:
URL: https://github.com/apache/iotdb/pull/13111#discussion_r1713773158


##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensusServerImpl.java:
##########
@@ -326,20 +327,30 @@ public void transmitSnapshot(Peer targetPeer) throws ConsensusGroupModifyPeerExc
         syncClientManager.borrowClient(targetPeer.getEndpoint())) {
       for (File file : snapshotPaths) {
         SnapshotFragmentReader reader =
-            new SnapshotFragmentReader(newSnapshotDirName, file.toPath());
+            new SnapshotFragmentReader(newSnapshotDirName, file.toPath(), needDataVerification);
         try {
-          while (reader.hasNext()) {
+          reader.hasNext();
+          while (true) {
             // TODO: zero copy ?
             TSendSnapshotFragmentReq req = reader.next().toTSendSnapshotFragmentReq();
             req.setConsensusGroupId(targetPeer.getGroupId().convertToTConsensusGroupId());
             ioTConsensusRateLimiter.acquireTransitDataSizeWithRateLimiter(req.getChunkLength());
             TSendSnapshotFragmentRes res = client.sendSnapshotFragment(req);
+            if (res.getStatus().getCode()
+                == TSStatusCode.SNAPSHOT_CHUNK_MD5_CHECK_NOT_PASS.getStatusCode()) {
+              // maybe network problem, retry
+              logger.info("Snapshot chunk md5 check not pass, automatically retry...");
+              continue;

Review Comment:
   Should we consider increasing the maximum number of retries?



##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/iot/service/IoTConsensusRPCServiceProcessor.java:
##########
@@ -251,6 +252,18 @@ public TSendSnapshotFragmentRes sendSnapshotFragment(TSendSnapshotFragmentReq re
       return new TSendSnapshotFragmentRes(status);
     }
     TSStatus responseStatus;
+    // check file chunk md5
+    if (req.isSetFileChunkMD5()) {
+      String receiverMd5 = SnapshotFragment.calculateMd5(req.fileChunk);
+      if (!receiverMd5.isEmpty()) {

Review Comment:
   Is it possible for receiverMd5 to be empty while req is not?



-- 
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: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org