You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by ta...@apache.org on 2022/12/13 05:43:45 UTC

[iotdb] branch cp-jira5185 created (now bdd01dbf7f)

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

tanxinyu pushed a change to branch cp-jira5185
in repository https://gitbox.apache.org/repos/asf/iotdb.git


      at bdd01dbf7f fix code smell

This branch includes the following new commits:

     new 3a8b0a8f29 add judgement for ratis
     new 3564afe4d3 clear old snapshot after triggerSnapshot
     new bdd01dbf7f fix code smell

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[iotdb] 01/03: add judgement for ratis

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tanxinyu pushed a commit to branch cp-jira5185
in repository https://gitbox.apache.org/repos/asf/iotdb.git

commit 3a8b0a8f29b15c09be339784b9761d929193fe42
Author: OneSizeFitQuorum <ta...@apache.org>
AuthorDate: Tue Dec 13 10:36:55 2022 +0800

    add judgement for ratis
    
    Signed-off-by: OneSizeFitQuorum <ta...@apache.org>
---
 .../java/org/apache/iotdb/db/service/IoTDBShutdownHook.java | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/server/src/main/java/org/apache/iotdb/db/service/IoTDBShutdownHook.java b/server/src/main/java/org/apache/iotdb/db/service/IoTDBShutdownHook.java
index ef83211603..cbf93ebeb8 100644
--- a/server/src/main/java/org/apache/iotdb/db/service/IoTDBShutdownHook.java
+++ b/server/src/main/java/org/apache/iotdb/db/service/IoTDBShutdownHook.java
@@ -19,6 +19,7 @@
 package org.apache.iotdb.db.service;
 
 import org.apache.iotdb.commons.conf.CommonDescriptor;
+import org.apache.iotdb.consensus.ConsensusFactory;
 import org.apache.iotdb.db.conf.IoTDBDescriptor;
 import org.apache.iotdb.db.conf.directories.DirectoryChecker;
 import org.apache.iotdb.db.consensus.DataRegionConsensusImpl;
@@ -53,9 +54,15 @@ public class IoTDBShutdownHook extends Thread {
     }
     WALManager.getInstance().deleteOutdatedWALFiles();
 
-    if (IoTDBDescriptor.getInstance().getConfig().isClusterMode()) {
-      // This setting ensures that compaction work is not discarded
-      // even if there are frequent restarts
+    // We did this work because the RatisConsensus recovery mechanism is different from other
+    // consensus algorithms, which will replace the underlying storage engine based on its own
+    // latest snapshot, while other consensus algorithms will not. This judgement ensures that
+    // compaction work is not discarded even if there are frequent restarts
+    if (IoTDBDescriptor.getInstance().getConfig().isClusterMode()
+        && IoTDBDescriptor.getInstance()
+            .getConfig()
+            .getDataRegionConsensusProtocolClass()
+            .equals(ConsensusFactory.RATIS_CONSENSUS)) {
       DataRegionConsensusImpl.getInstance()
           .getAllConsensusGroupIds()
           .parallelStream()


[iotdb] 03/03: fix code smell

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tanxinyu pushed a commit to branch cp-jira5185
in repository https://gitbox.apache.org/repos/asf/iotdb.git

commit bdd01dbf7f40a98015dc85ddff6e56be9429449f
Author: OneSizeFitQuorum <ta...@apache.org>
AuthorDate: Tue Dec 13 13:39:22 2022 +0800

    fix code smell
    
    Signed-off-by: OneSizeFitQuorum <ta...@apache.org>
---
 .../src/test/java/org/apache/iotdb/consensus/iot/StabilityTest.java   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/consensus/src/test/java/org/apache/iotdb/consensus/iot/StabilityTest.java b/consensus/src/test/java/org/apache/iotdb/consensus/iot/StabilityTest.java
index 215940f7ed..474515fe3a 100644
--- a/consensus/src/test/java/org/apache/iotdb/consensus/iot/StabilityTest.java
+++ b/consensus/src/test/java/org/apache/iotdb/consensus/iot/StabilityTest.java
@@ -114,7 +114,7 @@ public class StabilityTest {
     File[] versionFiles1 =
         dataDir.listFiles((dir, name) -> name.startsWith(IoTConsensusServerImpl.SNAPSHOT_DIR_NAME));
     Assert.assertNotNull(versionFiles1);
-    Assert.assertEquals(versionFiles1.length, 1);
+    Assert.assertEquals(1, versionFiles1.length);
 
     consensusImpl.triggerSnapshot(dataRegionId);
     consensusImpl.triggerSnapshot(dataRegionId);
@@ -122,7 +122,7 @@ public class StabilityTest {
     File[] versionFiles2 =
         dataDir.listFiles((dir, name) -> name.startsWith(IoTConsensusServerImpl.SNAPSHOT_DIR_NAME));
     Assert.assertNotNull(versionFiles2);
-    Assert.assertEquals(versionFiles2.length, 1);
+    Assert.assertEquals(1, versionFiles2.length);
 
     Assert.assertNotEquals(versionFiles1[0].getName(), versionFiles2[0].getName());
   }


[iotdb] 02/03: clear old snapshot after triggerSnapshot

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tanxinyu pushed a commit to branch cp-jira5185
in repository https://gitbox.apache.org/repos/asf/iotdb.git

commit 3564afe4d314981d10d34435a991c37cb0142e6e
Author: OneSizeFitQuorum <ta...@apache.org>
AuthorDate: Tue Dec 13 11:56:25 2022 +0800

    clear old snapshot after triggerSnapshot
    
    Signed-off-by: OneSizeFitQuorum <ta...@apache.org>
---
 .../apache/iotdb/consensus/iot/IoTConsensus.java   |  6 +--
 .../consensus/iot/IoTConsensusServerImpl.java      | 24 +++++++++-
 .../{IoTConsensusTest.java => ReplicateTest.java}  |  4 +-
 .../iot/{RecoveryTest.java => StabilityTest.java}  | 52 +++++++++++++++++-----
 .../iotdb/consensus/iot/util/TestStateMachine.java |  2 +-
 5 files changed, 71 insertions(+), 17 deletions(-)

diff --git a/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensus.java b/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensus.java
index 9986b38bd1..5135d384a2 100644
--- a/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensus.java
+++ b/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensus.java
@@ -202,7 +202,7 @@ public class IoTConsensus implements IConsensus {
         groupId,
         k -> {
           exist.set(false);
-          String path = buildPeerDir(groupId);
+          String path = buildPeerDir(storageDir, groupId);
           File file = new File(path);
           if (!file.mkdirs()) {
             logger.warn("Unable to create consensus dir for group {} at {}", groupId, path);
@@ -235,7 +235,7 @@ public class IoTConsensus implements IConsensus {
         (k, v) -> {
           exist.set(true);
           v.stop();
-          FileUtils.deleteDirectory(new File(buildPeerDir(groupId)));
+          FileUtils.deleteDirectory(new File(buildPeerDir(storageDir, groupId)));
           return null;
         });
 
@@ -390,7 +390,7 @@ public class IoTConsensus implements IConsensus {
     return stateMachineMap.get(groupId);
   }
 
-  private String buildPeerDir(ConsensusGroupId groupId) {
+  public static String buildPeerDir(File storageDir, ConsensusGroupId groupId) {
     return storageDir + File.separator + groupId.getType().getValue() + "_" + groupId.getId();
   }
 }
diff --git a/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensusServerImpl.java b/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensusServerImpl.java
index b95fa128bf..cfd5907b68 100644
--- a/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensusServerImpl.java
+++ b/consensus/src/main/java/org/apache/iotdb/consensus/iot/IoTConsensusServerImpl.java
@@ -73,7 +73,9 @@ import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardOpenOption;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
@@ -85,7 +87,7 @@ public class IoTConsensusServerImpl {
 
   private static final String CONFIGURATION_FILE_NAME = "configuration.dat";
   private static final String CONFIGURATION_TMP_FILE_NAME = "configuration.dat.tmp";
-  private static final String SNAPSHOT_DIR_NAME = "snapshot";
+  public static final String SNAPSHOT_DIR_NAME = "snapshot";
 
   private final Logger logger = LoggerFactory.getLogger(IoTConsensusServerImpl.class);
 
@@ -299,6 +301,7 @@ public class IoTConsensusServerImpl {
       if (!stateMachine.takeSnapshot(snapshotDir)) {
         throw new ConsensusGroupModifyPeerException("unknown error when taking snapshot");
       }
+      clearOldSnapshot();
     } catch (IOException e) {
       throw new ConsensusGroupModifyPeerException("error when taking snapshot", e);
     }
@@ -363,6 +366,25 @@ public class IoTConsensusServerImpl {
     return originalFilePath.substring(originalFilePath.indexOf(snapshotId));
   }
 
+  private void clearOldSnapshot() {
+    File directory = new File(storageDir);
+    File[] versionFiles = directory.listFiles((dir, name) -> name.startsWith(SNAPSHOT_DIR_NAME));
+    if (versionFiles == null || versionFiles.length == 0) {
+      logger.error(
+          "Can not find any snapshot dir after build a new snapshot for group {}",
+          thisNode.getGroupId());
+      return;
+    }
+    Arrays.sort(versionFiles, Comparator.comparing(File::getName));
+    for (int i = 0; i < versionFiles.length - 1; i++) {
+      try {
+        FileUtils.deleteDirectory(versionFiles[i]);
+      } catch (IOException e) {
+        logger.error("Delete old snapshot dir {} failed", versionFiles[i].getAbsolutePath(), e);
+      }
+    }
+  }
+
   public void loadSnapshot(String snapshotId) {
     // TODO: (xingtanzjr) throw exception if the snapshot load failed
     stateMachine.loadSnapshot(new File(storageDir, snapshotId));
diff --git a/consensus/src/test/java/org/apache/iotdb/consensus/iot/IoTConsensusTest.java b/consensus/src/test/java/org/apache/iotdb/consensus/iot/ReplicateTest.java
similarity index 98%
rename from consensus/src/test/java/org/apache/iotdb/consensus/iot/IoTConsensusTest.java
rename to consensus/src/test/java/org/apache/iotdb/consensus/iot/ReplicateTest.java
index 6f21adc5b6..e1675e7994 100644
--- a/consensus/src/test/java/org/apache/iotdb/consensus/iot/IoTConsensusTest.java
+++ b/consensus/src/test/java/org/apache/iotdb/consensus/iot/ReplicateTest.java
@@ -43,9 +43,9 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
-public class IoTConsensusTest {
+public class ReplicateTest {
   private static final long CHECK_POINT_GAP = 500;
-  private final Logger logger = LoggerFactory.getLogger(IoTConsensusTest.class);
+  private final Logger logger = LoggerFactory.getLogger(ReplicateTest.class);
 
   private final ConsensusGroupId gid = new DataRegionId(1);
 
diff --git a/consensus/src/test/java/org/apache/iotdb/consensus/iot/RecoveryTest.java b/consensus/src/test/java/org/apache/iotdb/consensus/iot/StabilityTest.java
similarity index 61%
rename from consensus/src/test/java/org/apache/iotdb/consensus/iot/RecoveryTest.java
rename to consensus/src/test/java/org/apache/iotdb/consensus/iot/StabilityTest.java
index d391faa17f..215940f7ed 100644
--- a/consensus/src/test/java/org/apache/iotdb/consensus/iot/RecoveryTest.java
+++ b/consensus/src/test/java/org/apache/iotdb/consensus/iot/StabilityTest.java
@@ -21,7 +21,7 @@ package org.apache.iotdb.consensus.iot;
 
 import org.apache.iotdb.common.rpc.thrift.TEndPoint;
 import org.apache.iotdb.commons.consensus.ConsensusGroupId;
-import org.apache.iotdb.commons.consensus.SchemaRegionId;
+import org.apache.iotdb.commons.consensus.DataRegionId;
 import org.apache.iotdb.consensus.ConsensusFactory;
 import org.apache.iotdb.consensus.IConsensus;
 import org.apache.iotdb.consensus.common.Peer;
@@ -39,9 +39,12 @@ import java.io.File;
 import java.io.IOException;
 import java.util.Collections;
 
-public class RecoveryTest {
+public class StabilityTest {
+
+  private final ConsensusGroupId dataRegionId = new DataRegionId(1);
+
+  private final File storageDir = new File("target" + java.io.File.separator + "stability");
 
-  private final ConsensusGroupId schemaRegionId = new SchemaRegionId(1);
   private IConsensus consensusImpl;
 
   public void constructConsensus() throws IOException {
@@ -51,7 +54,7 @@ public class RecoveryTest {
                 ConsensusConfig.newBuilder()
                     .setThisNodeId(1)
                     .setThisNode(new TEndPoint("0.0.0.0", 9000))
-                    .setStorageDir("target" + java.io.File.separator + "recovery")
+                    .setStorageDir(storageDir.getAbsolutePath())
                     .build(),
                 gid -> new TestStateMachine())
             .orElseThrow(
@@ -71,16 +74,16 @@ public class RecoveryTest {
   @After
   public void tearDown() throws IOException {
     consensusImpl.stop();
-    FileUtils.deleteFully(new File("./target/recovery"));
+    FileUtils.deleteFully(storageDir);
   }
 
   @Test
   public void recoveryTest() throws Exception {
     consensusImpl.createPeer(
-        schemaRegionId,
-        Collections.singletonList(new Peer(schemaRegionId, 1, new TEndPoint("0.0.0.0", 9000))));
+        dataRegionId,
+        Collections.singletonList(new Peer(dataRegionId, 1, new TEndPoint("0.0.0.0", 9000))));
 
-    consensusImpl.deletePeer(schemaRegionId);
+    consensusImpl.deletePeer(dataRegionId);
 
     consensusImpl.stop();
     consensusImpl = null;
@@ -89,9 +92,38 @@ public class RecoveryTest {
 
     ConsensusGenericResponse response =
         consensusImpl.createPeer(
-            schemaRegionId,
-            Collections.singletonList(new Peer(schemaRegionId, 1, new TEndPoint("0.0.0.0", 9000))));
+            dataRegionId,
+            Collections.singletonList(new Peer(dataRegionId, 1, new TEndPoint("0.0.0.0", 9000))));
 
     Assert.assertTrue(response.isSuccess());
   }
+
+  @Test
+  public void cleanOldSnapshotAfterTriggerSnapshotTest() {
+    ConsensusGenericResponse response =
+        consensusImpl.createPeer(
+            dataRegionId,
+            Collections.singletonList(new Peer(dataRegionId, 1, new TEndPoint("0.0.0.0", 9000))));
+
+    Assert.assertTrue(response.isSuccess());
+
+    consensusImpl.triggerSnapshot(dataRegionId);
+
+    File dataDir = new File(IoTConsensus.buildPeerDir(storageDir, dataRegionId));
+
+    File[] versionFiles1 =
+        dataDir.listFiles((dir, name) -> name.startsWith(IoTConsensusServerImpl.SNAPSHOT_DIR_NAME));
+    Assert.assertNotNull(versionFiles1);
+    Assert.assertEquals(versionFiles1.length, 1);
+
+    consensusImpl.triggerSnapshot(dataRegionId);
+    consensusImpl.triggerSnapshot(dataRegionId);
+
+    File[] versionFiles2 =
+        dataDir.listFiles((dir, name) -> name.startsWith(IoTConsensusServerImpl.SNAPSHOT_DIR_NAME));
+    Assert.assertNotNull(versionFiles2);
+    Assert.assertEquals(versionFiles2.length, 1);
+
+    Assert.assertNotEquals(versionFiles1[0].getName(), versionFiles2[0].getName());
+  }
 }
diff --git a/consensus/src/test/java/org/apache/iotdb/consensus/iot/util/TestStateMachine.java b/consensus/src/test/java/org/apache/iotdb/consensus/iot/util/TestStateMachine.java
index 672200daa3..91e614bb72 100644
--- a/consensus/src/test/java/org/apache/iotdb/consensus/iot/util/TestStateMachine.java
+++ b/consensus/src/test/java/org/apache/iotdb/consensus/iot/util/TestStateMachine.java
@@ -113,7 +113,7 @@ public class TestStateMachine implements IStateMachine, IStateMachine.EventApi {
 
   @Override
   public boolean takeSnapshot(File snapshotDir) {
-    return false;
+    return true;
   }
 
   @Override