[ratis] branch master updated: RATIS-1332. Always remove raft-meta.tmp to clean uncommitted changes (#442)

[ru...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 5fba786  RATIS-1332. Always remove raft-meta.tmp to clean uncommitted changes (#442)
5fba786 is described below

commit 5fba786bdbd20adfab957f7ab8f35cb8c5ad6e26
Author: Symious <yi...@foxmail.com>
AuthorDate: Mon Mar 15 09:46:59 2021 +0800

    RATIS-1332. Always remove raft-meta.tmp to clean uncommitted changes (#442)
    
    Co-authored-by: Symious <yi...@gmail.com>
---
 .../org/apache/ratis/server/storage/RaftStorageImpl.java     |  8 +++++---
 .../org/apache/ratis/server/storage/TestRaftStorage.java     | 12 +++---------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java
index 25a0c4a..93772da 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageImpl.java
@@ -93,6 +93,11 @@ public class RaftStorageImpl implements RaftStorage {
 
   private StorageState analyzeAndRecoverStorage(boolean toLock) throws IOException {
     StorageState storageState = storageDir.analyzeStorage(toLock);
+    // Existence of raft-meta.tmp means the change of votedFor/term has not
+    // been committed. Thus we should delete the tmp file.
+    if (storageState != StorageState.NON_EXISTENT) {
+      cleanMetaTmpFile();
+    }
     if (storageState == StorageState.NORMAL) {
       final File f = storageDir.getMetaFile();
       if (!f.exists()) {
@@ -101,9 +106,6 @@ public class RaftStorageImpl implements RaftStorage {
       metaFile = new RaftStorageMetadataFileImpl(f);
       final RaftStorageMetadata metadata = metaFile.getMetadata();
       LOG.info("Read {} from {}", metadata, f);
-      // Existence of raft-meta.tmp means the change of votedFor/term has not
-      // been committed. Thus we should delete the tmp file.
-      cleanMetaTmpFile();
       return StorageState.NORMAL;
     } else if (storageState == StorageState.NOT_FORMATTED &&
         storageDir.isCurrentEmpty()) {
diff --git a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java
index 0dc85d8..678e432 100644
--- a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java
+++ b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftStorage.java
@@ -162,16 +162,10 @@ public class TestRaftStorage extends BaseTest {
 
     Assert.assertEquals(StorageState.NOT_FORMATTED, sd.analyzeStorage(false));
 
-    try {
-      newRaftStorage(storageDir);
-      Assert.fail("should throw IOException since storage dir is not formatted");
-    } catch (IOException e) {
-      Assert.assertTrue(
-          e.getMessage().contains(StorageState.NOT_FORMATTED.name()));
-    }
+    // RaftStorage initialization should succeed as the raft-meta.tmp is
+    // always cleaned.
+    newRaftStorage(storageDir).close();
 
-    // let the storage dir contain both raft-meta and raft-meta.tmp
-    formatRaftStorage(storageDir).close();
     Assert.assertTrue(sd.getMetaFile().exists());
     Assert.assertTrue(sd.getMetaTmpFile().createNewFile());
     Assert.assertTrue(sd.getMetaTmpFile().exists());