From 28e2d854048ca714247f4f5a238aa9d82ce30395 Mon Sep 17 00:00:00 2001 From: Abhishek Pal Date: Fri, 21 Nov 2025 02:10:06 +0530 Subject: [PATCH 1/6] RATIS-244. Skip snapshot file if corresponding MD5 file is missing --- .../ratis/statemachine/impl/SimpleStateMachineStorage.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java index 0ca6734a07..496cbc8a8b 100644 --- a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java +++ b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java @@ -89,7 +89,8 @@ static List getSingleFileSnapshotInfos(Path dir) throws final Path filename = path.getFileName(); if (filename != null) { final Matcher matcher = SNAPSHOT_REGEX.matcher(filename.toString()); - if (matcher.matches()) { + // If the file doesn't have an MD5 hash it would doesn't need to be matched as it might be corrupted + if (MD5FileUtil.getDigestFileForFile(filename.toFile()).exists() && matcher.matches()) { final long term = Long.parseLong(matcher.group(1)); final long index = Long.parseLong(matcher.group(2)); final FileInfo fileInfo = new FileInfo(path, null); //No FileDigest here. From 1484e9478710016c76e716917056f64c21cd3c1e Mon Sep 17 00:00:00 2001 From: Abhishek Pal Date: Sat, 22 Nov 2025 00:49:22 +0530 Subject: [PATCH 2/6] Address review comments, add new tests for cleanupOldSnapshots and findLatestSnapshot --- .../impl/SimpleStateMachineStorage.java | 40 +++- .../impl/SingleFileSnapshotInfo.java | 16 +- .../ratis/server/storage/TestRaftStorage.java | 178 ++++++++++++++++++ 3 files changed, 222 insertions(+), 12 deletions(-) diff --git a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java index 496cbc8a8b..4a0f5e9124 100644 --- a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java +++ b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java @@ -82,19 +82,23 @@ public void format() throws IOException { // TODO } - static List getSingleFileSnapshotInfos(Path dir) throws IOException { + static List getSingleFileSnapshotInfos(Path dir, boolean requireMd5) throws IOException { final List infos = new ArrayList<>(); try (DirectoryStream stream = Files.newDirectoryStream(dir)) { for (Path path : stream) { final Path filename = path.getFileName(); if (filename != null) { final Matcher matcher = SNAPSHOT_REGEX.matcher(filename.toString()); - // If the file doesn't have an MD5 hash it would doesn't need to be matched as it might be corrupted - if (MD5FileUtil.getDigestFileForFile(filename.toFile()).exists() && matcher.matches()) { + if (matcher.matches()) { + final boolean hasMd5 = MD5FileUtil.getDigestFileForFile(path.toFile()).exists(); + if (requireMd5 && !hasMd5) { + continue; + } + final long term = Long.parseLong(matcher.group(1)); final long index = Long.parseLong(matcher.group(2)); final FileInfo fileInfo = new FileInfo(path, null); //No FileDigest here. - infos.add(new SingleFileSnapshotInfo(fileInfo, term, index)); + infos.add(new SingleFileSnapshotInfo(fileInfo, term, index, hasMd5)); } } } @@ -115,11 +119,27 @@ public void cleanupOldSnapshots(SnapshotRetentionPolicy snapshotRetentionPolicy) return; } - final List allSnapshotFiles = getSingleFileSnapshotInfos(stateMachineDir.toPath()); + // Fetch all the snapshot files irrespective of whether they have an MD5 file or not + final List allSnapshotFiles = getSingleFileSnapshotInfos(stateMachineDir.toPath(), false); + allSnapshotFiles.sort(Comparator.comparing(SingleFileSnapshotInfo::getIndex).reversed()); + int numSnapshotsWithMd5 = 0; + int deleteIdx = -1; + + for (int i = 0; i < allSnapshotFiles.size(); i++) { + final SingleFileSnapshotInfo snapshot = allSnapshotFiles.get(i); + if (snapshot.hasMd5()) { + if (++numSnapshotsWithMd5 == numSnapshotsRetained) { + // We have found the last snapshot with an MD5 file that needs to be retained + deleteIdx = i + 1; + break; + } + } else { + LOG.warn("Snapshot file {} has missing MD5 file.", snapshot); + } + } - if (allSnapshotFiles.size() > numSnapshotsRetained) { - allSnapshotFiles.sort(Comparator.comparing(SingleFileSnapshotInfo::getIndex).reversed()); - allSnapshotFiles.subList(numSnapshotsRetained, allSnapshotFiles.size()) + if (deleteIdx > 0) { + allSnapshotFiles.subList(deleteIdx, allSnapshotFiles.size()) .stream() .map(SingleFileSnapshotInfo::getFile) .map(FileInfo::getPath) @@ -183,7 +203,7 @@ protected File getCorruptSnapshotFile(long term, long endIndex) { } static SingleFileSnapshotInfo findLatestSnapshot(Path dir) throws IOException { - final Iterator i = getSingleFileSnapshotInfos(dir).iterator(); + final Iterator i = getSingleFileSnapshotInfos(dir, true).iterator(); if (!i.hasNext()) { return null; } @@ -200,7 +220,7 @@ static SingleFileSnapshotInfo findLatestSnapshot(Path dir) throws IOException { final Path path = latest.getFile().getPath(); final MD5Hash md5 = MD5FileUtil.readStoredMd5ForFile(path.toFile()); final FileInfo info = new FileInfo(path, md5); - return new SingleFileSnapshotInfo(info, latest.getTerm(), latest.getIndex()); + return new SingleFileSnapshotInfo(info, latest.getTerm(), latest.getIndex(), true); } public SingleFileSnapshotInfo updateLatestSnapshot(SingleFileSnapshotInfo info) { diff --git a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SingleFileSnapshotInfo.java b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SingleFileSnapshotInfo.java index 14d501a4af..922fa2ff52 100644 --- a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SingleFileSnapshotInfo.java +++ b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SingleFileSnapshotInfo.java @@ -28,12 +28,24 @@ * The objects of this class are immutable. */ public class SingleFileSnapshotInfo extends FileListSnapshotInfo { + private final Boolean hasMd5; // Whether the snapshot file has a corresponding MD5 file + public SingleFileSnapshotInfo(FileInfo fileInfo, TermIndex termIndex) { + this(fileInfo, termIndex, null); + } + + public SingleFileSnapshotInfo(FileInfo fileInfo, TermIndex termIndex, Boolean hasMd5) { super(Collections.singletonList(fileInfo), termIndex); + this.hasMd5 = hasMd5; + } + + public SingleFileSnapshotInfo(FileInfo fileInfo, long term, long endIndex, boolean hasMd5) { + this(fileInfo, TermIndex.valueOf(term, endIndex), hasMd5); } - public SingleFileSnapshotInfo(FileInfo fileInfo, long term, long endIndex) { - this(fileInfo, TermIndex.valueOf(term, endIndex)); + /** @return the md5 file exists for the snapshot file */ + public boolean hasMd5() { + return hasMd5 != null && hasMd5; } /** @return the file associated with the snapshot. */ 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 12cd771315..69e11a8c53 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 @@ -18,6 +18,7 @@ package org.apache.ratis.server.storage; import static java.util.stream.Collectors.toList; +import static org.apache.ratis.statemachine.impl.SimpleStateMachineStorage.SNAPSHOT_MD5_REGEX; import static org.apache.ratis.statemachine.impl.SimpleStateMachineStorage.SNAPSHOT_REGEX; import static org.apache.ratis.util.MD5FileUtil.MD5_SUFFIX; @@ -29,6 +30,7 @@ import org.apache.ratis.server.storage.RaftStorageDirectoryImpl.StorageState; import org.apache.ratis.statemachine.impl.SimpleStateMachineStorage; import org.apache.ratis.statemachine.SnapshotRetentionPolicy; +import org.apache.ratis.statemachine.impl.SingleFileSnapshotInfo; import org.apache.ratis.util.FileUtils; import org.apache.ratis.util.Preconditions; import org.apache.ratis.util.SizeInBytes; @@ -44,6 +46,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicReference; @@ -296,6 +299,162 @@ public int getNumSnapshotsRetained() { assertFileCount(stateMachineDir, 5); } + @Test + public void testSnapshotCleanupWithMissingMd5File() throws IOException { + + SnapshotRetentionPolicy snapshotRetentionPolicy = new SnapshotRetentionPolicy() { + @Override + public int getNumSnapshotsRetained() { + return 2; + } + }; + + + SimpleStateMachineStorage simpleStateMachineStorage = new SimpleStateMachineStorage(); + final RaftStorage storage = newRaftStorage(storageDir); + simpleStateMachineStorage.init(storage); + + Set termIndexSet = new HashSet<>(); + + // Create one snapshot file without MD5 file + if (termIndexSet.add(TermIndex.valueOf(1, 100))) { + createSnapshot(simpleStateMachineStorage, 1, 100, false); + } + + //Create 4 snapshot files in storage dir. + while (termIndexSet.size() < 5) { + final long term = ThreadLocalRandom.current().nextLong(2, 10L); + final long index = ThreadLocalRandom.current().nextLong(100, 1000L); + if (termIndexSet.add(TermIndex.valueOf(term, index))) { + createSnapshot(simpleStateMachineStorage, term, index, true); + } + } + + File stateMachineDir = storage.getStorageDir().getStateMachineDir(); + assertFileCount(stateMachineDir, 5); + + simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); + + // We should have 3 files remaining, 1 snapshot file without MD5 hash, and 2 snapshots with MD5 hash + assertFileCount(stateMachineDir, 3); + } + + @Test + public void testCleanupOldSnapshotsDeletesOlderSnapshotsWithMd5() throws Exception { + SnapshotRetentionPolicy snapshotRetentionPolicy = new SnapshotRetentionPolicy() { + @Override + public int getNumSnapshotsRetained() { + return 2; + } + }; + + SimpleStateMachineStorage simpleStateMachineStorage = new SimpleStateMachineStorage(); + final RaftStorage storage = newRaftStorage(storageDir); + simpleStateMachineStorage.init(storage); + try { + createSnapshot(simpleStateMachineStorage, 1, 100, true); + createSnapshot(simpleStateMachineStorage, 1, 200, true); + createSnapshot(simpleStateMachineStorage, 1, 300, true); + createSnapshot(simpleStateMachineStorage, 1, 400, true); + + File stateMachineDir = storage.getStorageDir().getStateMachineDir(); + simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); + + List snapshotNames = listMatchingFileNames(stateMachineDir, SNAPSHOT_REGEX); + Assertions.assertEquals(2, snapshotNames.size()); + Assertions.assertTrue(snapshotNames.contains(SimpleStateMachineStorage.getSnapshotFileName(1, 400))); + Assertions.assertTrue(snapshotNames.contains(SimpleStateMachineStorage.getSnapshotFileName(1, 300))); + Assertions.assertFalse(snapshotNames.contains(SimpleStateMachineStorage.getSnapshotFileName(1, 200))); + Assertions.assertFalse(snapshotNames.contains(SimpleStateMachineStorage.getSnapshotFileName(1, 100))); + + List md5Names = listMatchingFileNames(stateMachineDir, SNAPSHOT_MD5_REGEX); + Assertions.assertEquals(2, md5Names.size()); + Assertions.assertTrue(md5Names.contains(SimpleStateMachineStorage.getSnapshotFileName(1, 400) + MD5_SUFFIX)); + Assertions.assertTrue(md5Names.contains(SimpleStateMachineStorage.getSnapshotFileName(1, 300) + MD5_SUFFIX)); + Assertions.assertFalse(md5Names.contains(SimpleStateMachineStorage.getSnapshotFileName(1, 200) + MD5_SUFFIX)); + Assertions.assertFalse(md5Names.contains(SimpleStateMachineStorage.getSnapshotFileName(1, 100) + MD5_SUFFIX)); + } finally { + storage.close(); + } + } + + @Test + public void testCleanupOldSnapshotsWithoutAnyMd5() throws Exception { + SnapshotRetentionPolicy snapshotRetentionPolicy = new SnapshotRetentionPolicy() { + @Override + public int getNumSnapshotsRetained() { + return 2; + } + }; + + SimpleStateMachineStorage simpleStateMachineStorage = new SimpleStateMachineStorage(); + final RaftStorage storage = newRaftStorage(storageDir); + simpleStateMachineStorage.init(storage); + try { + createSnapshot(simpleStateMachineStorage, 1, 100, false); + createSnapshot(simpleStateMachineStorage, 1, 200, false); + createSnapshot(simpleStateMachineStorage, 1, 300, false); + + File stateMachineDir = storage.getStorageDir().getStateMachineDir(); + simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); + + List snapshotNames = listMatchingFileNames(stateMachineDir, SNAPSHOT_REGEX); + Assertions.assertEquals(3, snapshotNames.size()); + Assertions.assertTrue(listMatchingFileNames(stateMachineDir, SNAPSHOT_MD5_REGEX).isEmpty()); + } finally { + storage.close(); + } + } + + @Test + public void testGetLatestSnapshotReturnsNewest() throws Exception { + SimpleStateMachineStorage simpleStateMachineStorage = new SimpleStateMachineStorage(); + final RaftStorage storage = newRaftStorage(storageDir); + simpleStateMachineStorage.init(storage); + try { + Assertions.assertNull(simpleStateMachineStorage.getLatestSnapshot()); + + createSnapshot(simpleStateMachineStorage, 1, 100, true); + simpleStateMachineStorage.loadLatestSnapshot(); + SingleFileSnapshotInfo first = simpleStateMachineStorage.getLatestSnapshot(); + Assertions.assertNotNull(first); + Assertions.assertEquals(1, first.getTerm()); + Assertions.assertEquals(100, first.getIndex()); + Assertions.assertNotNull(first.getFile().getFileDigest()); + + createSnapshot(simpleStateMachineStorage, 1, 200, true); + simpleStateMachineStorage.loadLatestSnapshot(); + SingleFileSnapshotInfo second = simpleStateMachineStorage.getLatestSnapshot(); + Assertions.assertNotNull(second); + Assertions.assertEquals(1, second.getTerm()); + Assertions.assertEquals(200, second.getIndex()); + Assertions.assertNotNull(second.getFile().getFileDigest()); + } finally { + storage.close(); + } + } + + @Test + public void testGetLatestSnapshotIgnoresSnapshotsWithoutMd5() throws Exception { + SimpleStateMachineStorage simpleStateMachineStorage = new SimpleStateMachineStorage(); + final RaftStorage storage = newRaftStorage(storageDir); + simpleStateMachineStorage.init(storage); + try { + createSnapshot(simpleStateMachineStorage, 1, 100, true); + simpleStateMachineStorage.loadLatestSnapshot(); + + createSnapshot(simpleStateMachineStorage, 1, 200, false); + simpleStateMachineStorage.loadLatestSnapshot(); + + SingleFileSnapshotInfo latest = simpleStateMachineStorage.getLatestSnapshot(); + Assertions.assertNotNull(latest); + Assertions.assertEquals(100, latest.getIndex()); + Assertions.assertEquals(1, latest.getTerm()); + } finally { + storage.close(); + } + } + private static File[] assertFileCount(File dir, int expected) { File[] files = dir.listFiles(); Assertions.assertNotNull(files); @@ -303,6 +462,25 @@ private static File[] assertFileCount(File dir, int expected) { return files; } + private File createSnapshot(SimpleStateMachineStorage storage, + long term, long endIndex, + boolean withMd5) throws IOException { + File snapshotFile = storage.getSnapshotFile(term, endIndex); + Assertions.assertTrue(snapshotFile.createNewFile()); + + if (withMd5) { + File md5File = new File(snapshotFile.getParentFile(), snapshotFile.getName() + MD5_SUFFIX); + Assertions.assertTrue(md5File.createNewFile()); + } + return snapshotFile; + } + + private static List listMatchingFileNames(File dir, java.util.regex.Pattern pattern) { + return Arrays.stream(Objects.requireNonNull(dir.list())) + .filter(name -> pattern.matcher(name).matches()) + .collect(toList()); + } + @Test public void testNotEnoughSpace() throws IOException { File mockStorageDir = Mockito.spy(storageDir); From d7a1bfa2f2ed0f8366f4cc1aa481952e52aace4c Mon Sep 17 00:00:00 2001 From: Abhishek Pal Date: Sat, 22 Nov 2025 16:51:20 +0530 Subject: [PATCH 3/6] Always delete md5 files without snapshot, fix some tests --- .../impl/SimpleStateMachineStorage.java | 29 +++++++++--------- .../ratis/server/storage/TestRaftStorage.java | 30 +++++++++++++------ 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java index 4a0f5e9124..2a1b2257b2 100644 --- a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java +++ b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java @@ -147,20 +147,21 @@ public void cleanupOldSnapshots(SnapshotRetentionPolicy snapshotRetentionPolicy) LOG.info("Deleting old snapshot at {}", snapshotPath.toAbsolutePath()); FileUtils.deletePathQuietly(snapshotPath); }); - // clean up the md5 files if the corresponding snapshot file does not exist - try (DirectoryStream stream = Files.newDirectoryStream(stateMachineDir.toPath(), - SNAPSHOT_MD5_FILTER)) { - for (Path md5path : stream) { - Path md5FileNamePath = md5path.getFileName(); - if (md5FileNamePath == null) { - continue; - } - final String md5FileName = md5FileNamePath.toString(); - final File snapshotFile = new File(stateMachineDir, - md5FileName.substring(0, md5FileName.length() - MD5_SUFFIX.length())); - if (!snapshotFile.exists()) { - FileUtils.deletePathQuietly(md5path); - } + } + + // clean up the md5 files if the corresponding snapshot file does not exist + try (DirectoryStream stream = Files.newDirectoryStream(stateMachineDir.toPath(), + SNAPSHOT_MD5_FILTER)) { + for (Path md5path : stream) { + Path md5FileNamePath = md5path.getFileName(); + if (md5FileNamePath == null) { + continue; + } + final String md5FileName = md5FileNamePath.toString(); + final File snapshotFile = new File(stateMachineDir, + md5FileName.substring(0, md5FileName.length() - MD5_SUFFIX.length())); + if (!snapshotFile.exists()) { + FileUtils.deletePathQuietly(md5path); } } } 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 69e11a8c53..6aa3399239 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 @@ -231,7 +231,7 @@ public void testSnapshotCleanup() throws IOException { SnapshotRetentionPolicy snapshotRetentionPolicy = new SnapshotRetentionPolicy() { @Override public int getNumSnapshotsRetained() { - return 3; + return 2; } }; @@ -242,15 +242,24 @@ public int getNumSnapshotsRetained() { Set termIndexSet = new HashSet<>(); - //Create 5 snapshot files in storage dir. + //Create 3 snapshot files in storage dir. + while (termIndexSet.size() < 3) { + final long term = ThreadLocalRandom.current().nextLong(1, 10L); + final long index = ThreadLocalRandom.current().nextLong(100, 1000L); + if (termIndexSet.add(TermIndex.valueOf(term, index))) { + createSnapshot(simpleStateMachineStorage, term, index, true); + } + } + + // Create 2 more snapshot files in storage dir without MD5 files while (termIndexSet.size() < 5) { final long term = ThreadLocalRandom.current().nextLong(1, 10L); final long index = ThreadLocalRandom.current().nextLong(100, 1000L); if (termIndexSet.add(TermIndex.valueOf(term, index))) { - File file = simpleStateMachineStorage.getSnapshotFile(term, index); - Assertions.assertTrue(file.createNewFile()); + createSnapshot(simpleStateMachineStorage, term, index, true); } } + // create MD5 files that will not be deleted in older version while (termIndexSet.size() < 7) { final long term = 1; @@ -267,7 +276,9 @@ public int getNumSnapshotsRetained() { simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); - File[] remainingFiles = assertFileCount(stateMachineDir, 3); + // Since the MD5 files are not matching the snapshot files they are not cleaned up. + // So we still have 7 files - 5 snapshots and 2 MD5 files. + File[] remainingFiles = assertFileCount(stateMachineDir, 4); List remainingIndices = termIndexSet.stream() .map(TermIndex::getIndex) @@ -284,7 +295,7 @@ public int getNumSnapshotsRetained() { // Attempt to clean up again should not delete any more files. simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); - assertFileCount(stateMachineDir, 3); + assertFileCount(stateMachineDir, 4); //Test with Retention disabled. //Create 2 snapshot files in storage dir. @@ -321,7 +332,7 @@ public int getNumSnapshotsRetained() { createSnapshot(simpleStateMachineStorage, 1, 100, false); } - //Create 4 snapshot files in storage dir. + //Create 4 snapshot files in storage dir while (termIndexSet.size() < 5) { final long term = ThreadLocalRandom.current().nextLong(2, 10L); final long index = ThreadLocalRandom.current().nextLong(100, 1000L); @@ -330,13 +341,14 @@ public int getNumSnapshotsRetained() { } } + // 1 snapshot file without MD5 hash, 4 snapshots + 4 md5 hash files = 9 files File stateMachineDir = storage.getStorageDir().getStateMachineDir(); - assertFileCount(stateMachineDir, 5); + assertFileCount(stateMachineDir, 9); simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); // We should have 3 files remaining, 1 snapshot file without MD5 hash, and 2 snapshots with MD5 hash - assertFileCount(stateMachineDir, 3); + assertFileCount(stateMachineDir, 5); } @Test From 77e3643bd8ce7ea450dcf2c006602d7841d6f59c Mon Sep 17 00:00:00 2001 From: Abhishek Pal Date: Sat, 22 Nov 2025 17:21:34 +0530 Subject: [PATCH 4/6] Add a new test scenario --- .../ratis/server/storage/TestRaftStorage.java | 62 +++++++++++++++---- 1 file changed, 51 insertions(+), 11 deletions(-) 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 6aa3399239..4d6f79207f 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 @@ -244,7 +244,7 @@ public int getNumSnapshotsRetained() { //Create 3 snapshot files in storage dir. while (termIndexSet.size() < 3) { - final long term = ThreadLocalRandom.current().nextLong(1, 10L); + final long term = ThreadLocalRandom.current().nextLong(1, 5L); final long index = ThreadLocalRandom.current().nextLong(100, 1000L); if (termIndexSet.add(TermIndex.valueOf(term, index))) { createSnapshot(simpleStateMachineStorage, term, index, true); @@ -253,7 +253,7 @@ public int getNumSnapshotsRetained() { // Create 2 more snapshot files in storage dir without MD5 files while (termIndexSet.size() < 5) { - final long term = ThreadLocalRandom.current().nextLong(1, 10L); + final long term = ThreadLocalRandom.current().nextLong(5, 10L); final long index = ThreadLocalRandom.current().nextLong(100, 1000L); if (termIndexSet.add(TermIndex.valueOf(term, index))) { createSnapshot(simpleStateMachineStorage, term, index, true); @@ -272,13 +272,13 @@ public int getNumSnapshotsRetained() { } File stateMachineDir = storage.getStorageDir().getStateMachineDir(); - assertFileCount(stateMachineDir, 7); + assertFileCount(stateMachineDir, 10); simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); // Since the MD5 files are not matching the snapshot files they are not cleaned up. - // So we still have 7 files - 5 snapshots and 2 MD5 files. - File[] remainingFiles = assertFileCount(stateMachineDir, 4); + // So we still have 6 files - 4 snapshots and 2 MD5 files. + File[] remainingFiles = assertFileCount(stateMachineDir, 6); List remainingIndices = termIndexSet.stream() .map(TermIndex::getIndex) @@ -295,19 +295,18 @@ public int getNumSnapshotsRetained() { // Attempt to clean up again should not delete any more files. simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); - assertFileCount(stateMachineDir, 4); + assertFileCount(stateMachineDir, 6); //Test with Retention disabled. //Create 2 snapshot files in storage dir. for (int i = 0; i < 2; i++) { - final long term = ThreadLocalRandom.current().nextLong(1, 10L); + final long term = ThreadLocalRandom.current().nextLong(11, 20L); final long index = ThreadLocalRandom.current().nextLong(1000L); - File file = simpleStateMachineStorage.getSnapshotFile(term, index); - Assertions.assertTrue(file.createNewFile()); + createSnapshot(simpleStateMachineStorage, term, index, false); } simpleStateMachineStorage.cleanupOldSnapshots(new SnapshotRetentionPolicy() { }); - assertFileCount(stateMachineDir, 5); + assertFileCount(stateMachineDir, 8); } @Test @@ -347,7 +346,48 @@ public int getNumSnapshotsRetained() { simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); - // We should have 3 files remaining, 1 snapshot file without MD5 hash, and 2 snapshots with MD5 hash + // We should have 4 files remaining, and 2 snapshots with MD5 hash + assertFileCount(stateMachineDir, 4); + } + + @Test + public void testSnapshotCleanupWithLatestSnapshotMissingMd5File() throws IOException { + + SnapshotRetentionPolicy snapshotRetentionPolicy = new SnapshotRetentionPolicy() { + @Override + public int getNumSnapshotsRetained() { + return 2; + } + }; + + + SimpleStateMachineStorage simpleStateMachineStorage = new SimpleStateMachineStorage(); + final RaftStorage storage = newRaftStorage(storageDir); + simpleStateMachineStorage.init(storage); + + Set termIndexSet = new HashSet<>(); + + //Create 4 snapshot files in storage dir + while (termIndexSet.size() < 4) { + final long term = ThreadLocalRandom.current().nextLong(2, 10L); + final long index = ThreadLocalRandom.current().nextLong(100, 1000L); + if (termIndexSet.add(TermIndex.valueOf(term, index))) { + createSnapshot(simpleStateMachineStorage, term, index, true); + } + } + + // Create a snapshot file with a missing MD5 file and having the highest term index + if (termIndexSet.add(TermIndex.valueOf(99, 100))) { + createSnapshot(simpleStateMachineStorage, 99, 100, false); + } + + // 1 snapshot file without MD5 hash, 4 snapshots + 4 md5 hash files = 9 files + File stateMachineDir = storage.getStorageDir().getStateMachineDir(); + assertFileCount(stateMachineDir, 9); + + simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); + + // We should have 5 files remaining, and 2 snapshots with MD5 hash and 1 snapshot file without MD5 hash assertFileCount(stateMachineDir, 5); } From 0e87d3b0d5a2d3f8f805020d6bbae5011f00fced Mon Sep 17 00:00:00 2001 From: Abhishek Pal Date: Thu, 27 Nov 2025 02:02:45 +0530 Subject: [PATCH 5/6] Fix some tests --- .../ratis/server/storage/TestRaftStorage.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 4d6f79207f..f4eabfacf7 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 @@ -244,8 +244,8 @@ public int getNumSnapshotsRetained() { //Create 3 snapshot files in storage dir. while (termIndexSet.size() < 3) { - final long term = ThreadLocalRandom.current().nextLong(1, 5L); - final long index = ThreadLocalRandom.current().nextLong(100, 1000L); + final long term = ThreadLocalRandom.current().nextLong(1, 10L); + final long index = ThreadLocalRandom.current().nextLong(100, 500L); if (termIndexSet.add(TermIndex.valueOf(term, index))) { createSnapshot(simpleStateMachineStorage, term, index, true); } @@ -253,10 +253,10 @@ public int getNumSnapshotsRetained() { // Create 2 more snapshot files in storage dir without MD5 files while (termIndexSet.size() < 5) { - final long term = ThreadLocalRandom.current().nextLong(5, 10L); - final long index = ThreadLocalRandom.current().nextLong(100, 1000L); + final long term = ThreadLocalRandom.current().nextLong(11, 20L); + final long index = ThreadLocalRandom.current().nextLong(501, 1000L); if (termIndexSet.add(TermIndex.valueOf(term, index))) { - createSnapshot(simpleStateMachineStorage, term, index, true); + createSnapshot(simpleStateMachineStorage, term, index, false); } } @@ -276,14 +276,14 @@ public int getNumSnapshotsRetained() { simpleStateMachineStorage.cleanupOldSnapshots(snapshotRetentionPolicy); - // Since the MD5 files are not matching the snapshot files they are not cleaned up. + // Since the MD5 files are not matching the snapshot files they are cleaned up. // So we still have 6 files - 4 snapshots and 2 MD5 files. File[] remainingFiles = assertFileCount(stateMachineDir, 6); List remainingIndices = termIndexSet.stream() .map(TermIndex::getIndex) .sorted(Collections.reverseOrder()) - .limit(3) + .limit(4) .collect(toList()); for (File file : remainingFiles) { System.out.println(file.getName()); @@ -300,7 +300,7 @@ public int getNumSnapshotsRetained() { //Test with Retention disabled. //Create 2 snapshot files in storage dir. for (int i = 0; i < 2; i++) { - final long term = ThreadLocalRandom.current().nextLong(11, 20L); + final long term = ThreadLocalRandom.current().nextLong(21, 30L); final long index = ThreadLocalRandom.current().nextLong(1000L); createSnapshot(simpleStateMachineStorage, term, index, false); } From a4a1ea9e66a28b7d5c6fc2ffcb24dd4faeeb5301 Mon Sep 17 00:00:00 2001 From: Abhishek Pal Date: Mon, 1 Dec 2025 14:29:35 +0530 Subject: [PATCH 6/6] Fix server unit tests --- .../apache/ratis/server/storage/TestRaftStorage.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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 f4eabfacf7..a86f6ff1a1 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 @@ -32,6 +32,7 @@ import org.apache.ratis.statemachine.SnapshotRetentionPolicy; import org.apache.ratis.statemachine.impl.SingleFileSnapshotInfo; import org.apache.ratis.util.FileUtils; +import org.apache.ratis.util.MD5FileUtil; import org.apache.ratis.util.Preconditions; import org.apache.ratis.util.SizeInBytes; import org.junit.jupiter.api.AfterEach; @@ -369,7 +370,7 @@ public int getNumSnapshotsRetained() { //Create 4 snapshot files in storage dir while (termIndexSet.size() < 4) { - final long term = ThreadLocalRandom.current().nextLong(2, 10L); + final long term = ThreadLocalRandom.current().nextLong(1, 10L); final long index = ThreadLocalRandom.current().nextLong(100, 1000L); if (termIndexSet.add(TermIndex.valueOf(term, index))) { createSnapshot(simpleStateMachineStorage, term, index, true); @@ -377,8 +378,8 @@ public int getNumSnapshotsRetained() { } // Create a snapshot file with a missing MD5 file and having the highest term index - if (termIndexSet.add(TermIndex.valueOf(99, 100))) { - createSnapshot(simpleStateMachineStorage, 99, 100, false); + if (termIndexSet.add(TermIndex.valueOf(99, 1001))) { + createSnapshot(simpleStateMachineStorage, 99, 1001, false); } // 1 snapshot file without MD5 hash, 4 snapshots + 4 md5 hash files = 9 files @@ -521,9 +522,9 @@ private File createSnapshot(SimpleStateMachineStorage storage, Assertions.assertTrue(snapshotFile.createNewFile()); if (withMd5) { - File md5File = new File(snapshotFile.getParentFile(), snapshotFile.getName() + MD5_SUFFIX); - Assertions.assertTrue(md5File.createNewFile()); + MD5FileUtil.computeAndSaveMd5ForFile(snapshotFile); } + return snapshotFile; }