You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by xi...@apache.org on 2022/11/09 13:57:28 UTC

[iotdb] branch master updated: [IOTDB-4811]Fix snapshot file name error (#7900)

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

xingtanzjr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/master by this push:
     new b22ed4b4ad [IOTDB-4811]Fix snapshot file name error (#7900)
b22ed4b4ad is described below

commit b22ed4b4addaf9eed6809ecea47410a4736bc0c5
Author: suchenglong <40...@qq.com>
AuthorDate: Wed Nov 9 21:57:21 2022 +0800

    [IOTDB-4811]Fix snapshot file name error (#7900)
    
    * add new interface of take snapshot
    
    * fix unit test
    
    * fix take snpshot bug with new interface
    
    * It is no longer necessary to implement the takeSnapshot(File snapshotDir, String snapshotId) method in the test class
    
    Co-authored-by: Jinrui.Zhang <xi...@gmail.com>
---
 .../java/org/apache/iotdb/consensus/IStateMachine.java | 12 ++++++++++++
 .../consensus/ratis/ApplicationStateMachineProxy.java  |  3 ++-
 .../consensus/statemachine/DataRegionStateMachine.java | 16 ++++++++++++++++
 .../apache/iotdb/db/engine/snapshot/SnapshotTaker.java | 18 +++++++++++++-----
 4 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/consensus/src/main/java/org/apache/iotdb/consensus/IStateMachine.java b/consensus/src/main/java/org/apache/iotdb/consensus/IStateMachine.java
index 4641898a0e..72d8b50e55 100644
--- a/consensus/src/main/java/org/apache/iotdb/consensus/IStateMachine.java
+++ b/consensus/src/main/java/org/apache/iotdb/consensus/IStateMachine.java
@@ -69,6 +69,18 @@ public interface IStateMachine {
    */
   boolean takeSnapshot(File snapshotDir);
 
+  /**
+   * Take a snapshot of current statemachine. Snapshot.log will be stored under snapshotDir, The
+   * data of the snapshot will be stored under `data folder/snapshot/snapshotId`.
+   *
+   * @param snapshotDir required storage dir
+   * @param snapshotId the id of the snapshot
+   * @return true if snapshot is successfully taken
+   */
+  default boolean takeSnapshot(File snapshotDir, String snapshotId) {
+    return takeSnapshot(snapshotDir);
+  }
+
   /**
    * Load the latest snapshot from given dir
    *
diff --git a/consensus/src/main/java/org/apache/iotdb/consensus/ratis/ApplicationStateMachineProxy.java b/consensus/src/main/java/org/apache/iotdb/consensus/ratis/ApplicationStateMachineProxy.java
index 1b6f494113..76d958466c 100644
--- a/consensus/src/main/java/org/apache/iotdb/consensus/ratis/ApplicationStateMachineProxy.java
+++ b/consensus/src/main/java/org/apache/iotdb/consensus/ratis/ApplicationStateMachineProxy.java
@@ -208,7 +208,8 @@ public class ApplicationStateMachineProxy extends BaseStateMachine {
       return RaftLog.INVALID_LOG_INDEX;
     }
 
-    boolean applicationTakeSnapshotSuccess = applicationStateMachine.takeSnapshot(snapshotTmpDir);
+    boolean applicationTakeSnapshotSuccess =
+        applicationStateMachine.takeSnapshot(snapshotTmpDir, metadata);
     if (!applicationTakeSnapshotSuccess) {
       deleteIncompleteSnapshot(snapshotTmpDir);
       return RaftLog.INVALID_LOG_INDEX;
diff --git a/server/src/main/java/org/apache/iotdb/db/consensus/statemachine/DataRegionStateMachine.java b/server/src/main/java/org/apache/iotdb/db/consensus/statemachine/DataRegionStateMachine.java
index 0d90993355..6d2516964d 100644
--- a/server/src/main/java/org/apache/iotdb/db/consensus/statemachine/DataRegionStateMachine.java
+++ b/server/src/main/java/org/apache/iotdb/db/consensus/statemachine/DataRegionStateMachine.java
@@ -111,6 +111,22 @@ public class DataRegionStateMachine extends BaseStateMachine {
     }
   }
 
+  @Override
+  public boolean takeSnapshot(File snapshotDir, String snapshotId) {
+    try {
+      return new SnapshotTaker(region)
+          .takeFullSnapshot(snapshotDir.getAbsolutePath(), snapshotId, true);
+    } catch (Exception e) {
+      logger.error(
+          "Exception occurs when taking snapshot for {}-{} in {}",
+          region.getStorageGroupName(),
+          region.getDataRegionId(),
+          snapshotDir,
+          e);
+      return false;
+    }
+  }
+
   @Override
   public void loadSnapshot(File latestSnapshotRootDir) {
     DataRegion newRegion =
diff --git a/server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java b/server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java
index 1012660aca..175b8c5344 100644
--- a/server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java
+++ b/server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java
@@ -57,6 +57,14 @@ public class SnapshotTaker {
   public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnapshot)
       throws DirectoryNotLegalException, IOException {
     File snapshotDir = new File(snapshotDirPath);
+    String snapshotId = snapshotDir.getName();
+    return takeFullSnapshot(snapshotDirPath, snapshotId, flushBeforeSnapshot);
+  }
+
+  public boolean takeFullSnapshot(
+      String snapshotDirPath, String snapshotId, boolean flushBeforeSnapshot)
+      throws DirectoryNotLegalException, IOException {
+    File snapshotDir = new File(snapshotDirPath);
     if (snapshotDir.exists()
         && snapshotDir.listFiles() != null
         && Objects.requireNonNull(snapshotDir.listFiles()).length > 0) {
@@ -73,7 +81,7 @@ public class SnapshotTaker {
     try {
       snapshotLogger = new SnapshotLogger(snapshotLog);
       boolean success;
-      snapshotLogger.logSnapshotId(snapshotDir.getName());
+      snapshotLogger.logSnapshotId(snapshotId);
 
       try {
         readLockTheFile();
@@ -85,8 +93,8 @@ public class SnapshotTaker {
             dataRegion.writeUnlock();
           }
         }
-        success = createSnapshot(seqFiles, snapshotDir.getName());
-        success = createSnapshot(unseqFiles, snapshotDir.getName()) && success;
+        success = createSnapshot(seqFiles, snapshotId);
+        success = createSnapshot(unseqFiles, snapshotId) && success;
       } finally {
         readUnlockTheFile();
       }
@@ -96,14 +104,14 @@ public class SnapshotTaker {
             "Failed to take snapshot for {}-{}, clean up",
             dataRegion.getStorageGroupName(),
             dataRegion.getDataRegionId());
-        cleanUpWhenFail(snapshotDir.getName());
+        cleanUpWhenFail(snapshotId);
       } else {
         snapshotLogger.logEnd();
         LOGGER.info(
             "Successfully take snapshot for {}-{}, snapshot directory is {}",
             dataRegion.getStorageGroupName(),
             dataRegion.getDataRegionId(),
-            snapshotDirPath);
+            snapshotDir.getParentFile().getAbsolutePath() + File.separator + snapshotId);
       }
 
       return success;