From 274e6d9f49499b4d69ab16be6df9e1e2126425b8 Mon Sep 17 00:00:00 2001
From: dennis zhuang <killme2008@gmail.com>
Date: Wed, 30 Mar 2022 13:52:00 +0800
Subject: [PATCH] (fix) key not deleted in truncatePrefix (#806)

---
 .../jraft/storage/impl/RocksDBLogStorage.java | 38 +++++++++++--------
 .../storage/impl/BaseLogStorageTest.java      |  6 ++-
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/impl/RocksDBLogStorage.java b/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/impl/RocksDBLogStorage.java
index e16761086..4d22c4d9d 100644
--- a/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/impl/RocksDBLogStorage.java
+++ b/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/impl/RocksDBLogStorage.java
@@ -56,6 +56,7 @@
 import com.alipay.sofa.jraft.util.BytesUtil;
 import com.alipay.sofa.jraft.util.DebugStatistics;
 import com.alipay.sofa.jraft.util.Describer;
+import com.alipay.sofa.jraft.util.OnlyForTest;
 import com.alipay.sofa.jraft.util.Requires;
 import com.alipay.sofa.jraft.util.StorageOptionsFactory;
 import com.alipay.sofa.jraft.util.Utils;
@@ -435,18 +436,7 @@ public LogEntry getEntry(final long index) {
             if (this.hasLoadFirstLogIndex && index < this.firstLogIndex) {
                 return null;
             }
-            final byte[] keyBytes = getKeyBytes(index);
-            final byte[] bs = onDataGet(index, getValueFromRocksDB(keyBytes));
-            if (bs != null) {
-                final LogEntry entry = this.logEntryDecoder.decode(bs);
-                if (entry != null) {
-                    return entry;
-                } else {
-                    LOG.error("Bad log entry format for index={}, the log data is: {}.", index, BytesUtil.toHex(bs));
-                    // invalid data remove? TODO
-                    return null;
-                }
-            }
+            return getEntryFromDB(index);
         } catch (final RocksDBException | IOException e) {
             LOG.error("Fail to get log entry at index {} in data path: {}.", index, this.path, e);
         } finally {
@@ -455,6 +445,23 @@ public LogEntry getEntry(final long index) {
         return null;
     }
 
+    @OnlyForTest
+    LogEntry getEntryFromDB(final long index) throws IOException, RocksDBException {
+      final byte[] keyBytes = getKeyBytes(index);
+      final byte[] bs = onDataGet(index, getValueFromRocksDB(keyBytes));
+      if (bs != null) {
+          final LogEntry entry = this.logEntryDecoder.decode(bs);
+          if (entry != null) {
+              return entry;
+          } else {
+              LOG.error("Bad log entry format for index={}, the log data is: {}.", index, BytesUtil.toHex(bs));
+              // invalid data remove? TODO
+              return null;
+          }
+      }
+      return null;
+    }
+
     protected byte[] getValueFromRocksDB(final byte[] keyBytes) throws RocksDBException {
         checkState();
         return this.db.get(this.defaultHandle, keyBytes);
@@ -589,11 +596,12 @@ private void truncatePrefixInBackground(final long startIndex, final long firstI
                 // Note https://github.com/facebook/rocksdb/wiki/Delete-A-Range-Of-Keys
                 final byte[] startKey = getKeyBytes(startIndex);
                 final byte[] endKey = getKeyBytes(firstIndexKept);
+                // deleteRange to delete all keys in range.
+                db.deleteRange(this.defaultHandle, startKey, endKey);
+                db.deleteRange(this.confHandle, startKey, endKey);
+                // deleteFilesInRanges to speedup reclaiming disk space on write-heavy load.
                 db.deleteFilesInRanges(this.defaultHandle, Arrays.asList(startKey, endKey), false);
                 db.deleteFilesInRanges(this.confHandle, Arrays.asList(startKey, endKey), false);
-                // After deleteFilesInrange, some keys in the range may still exist in the database, so we have to compactionRange.
-                db.compactRange(this.defaultHandle, startKey, endKey);
-                db.compactRange(this.confHandle, startKey, endKey);
             } catch (final RocksDBException | IOException e) {
                 LOG.error("Fail to truncatePrefix in data path: {}, firstIndexKept={}.", this.path, firstIndexKept, e);
             } finally {
diff --git a/jraft-core/src/test/java/com/alipay/sofa/jraft/storage/impl/BaseLogStorageTest.java b/jraft-core/src/test/java/com/alipay/sofa/jraft/storage/impl/BaseLogStorageTest.java
index 2d4532b38..b13cca754 100644
--- a/jraft-core/src/test/java/com/alipay/sofa/jraft/storage/impl/BaseLogStorageTest.java
+++ b/jraft-core/src/test/java/com/alipay/sofa/jraft/storage/impl/BaseLogStorageTest.java
@@ -164,16 +164,20 @@ public void testReset() {
     }
 
     @Test
-    public void testTruncatePrefix() {
+    public void testTruncatePrefix() throws Exception {
         final List<LogEntry> entries = TestUtils.mockEntries();
 
         assertEquals(10, this.logStorage.appendEntries(entries));
         this.logStorage.truncatePrefix(5);
+        Thread.sleep(1000);
         assertEquals(5, this.logStorage.getFirstLogIndex());
         assertEquals(9, this.logStorage.getLastLogIndex());
         for (int i = 0; i < 10; i++) {
             if (i < 5) {
                 assertNull(this.logStorage.getEntry(i));
+                if (this.logStorage instanceof RocksDBLogStorage) {
+                    assertNull(((RocksDBLogStorage) this.logStorage).getEntryFromDB(i));
+                }
             } else {
                 Assert.assertEquals(entries.get(i), this.logStorage.getEntry(i));
             }