fix: 18571: Current path range should be respected when path to hash and path to KV indices are restored (#18592)

Fixes: #18571
Reviewed-by: Anthony Petrov <[email protected]>, Ivan Malygin <[email protected]>
Signed-off-by: Artem Ananev <[email protected]>

Author: artemananiev

platform-sdk/swirlds-merkledb/src/main/java/com/swirlds/merkledb/MerkleDbDataSource.java

@@ -287,15 +287,19 @@ public MerkleDbDataSource(
         hasDiskStoreForHashes = tableConfig.getHashesRamToDiskThreshold() < Long.MAX_VALUE;
         final DataFileCompactor hashStoreDiskFileCompactor;
         if (hasDiskStoreForHashes) {
-            final boolean hashIndexEmpty = pathToDiskLocationInternalNodes.size() == 0;
+            final boolean needRestorePathToDiskLocationInternalNodes = pathToDiskLocationInternalNodes.size() == 0;
             final LoadedDataCallback hashRecordLoadedCallback;
-            if (hashIndexEmpty) {
+            if (needRestorePathToDiskLocationInternalNodes) {
                 if (validLeafPathRange.getMaxValidKey() >= 0) {
                     pathToDiskLocationInternalNodes.updateValidRange(0, validLeafPathRange.getMaxValidKey());
                 }
                 hashRecordLoadedCallback = (dataLocation, hashData) -> {
                     final VirtualHashRecord hashRecord = VirtualHashRecord.parseFrom(hashData);
-                    pathToDiskLocationInternalNodes.put(hashRecord.path(), dataLocation);
+                    final long path = hashRecord.path();
+                    // Old data files may contain entries with paths outside the current virtual node range
+                    if (path <= validLeafPathRange.getMaxValidKey()) {
+                        pathToDiskLocationInternalNodes.put(path, dataLocation);
+                    }
                 };
             } else {
                 hashRecordLoadedCallback = null;
@@ -344,15 +348,20 @@ public MerkleDbDataSource(
         keyToPath.printStats();
 
         final LoadedDataCallback leafRecordLoadedCallback;
-        final boolean needRestorePathToDiskLocationLeafNodes = pathToDiskLocationLeafNodes.size() == 0;
+        final boolean needRestorePathToDiskLocationLeafNodes =
+                (pathToDiskLocationLeafNodes.size() == 0) && (validLeafPathRange.getMinValidKey() > 0);
         if (needRestorePathToDiskLocationLeafNodes) {
             if (validLeafPathRange.getMaxValidKey() >= 0) {
                 pathToDiskLocationLeafNodes.updateValidRange(
                         validLeafPathRange.getMinValidKey(), validLeafPathRange.getMaxValidKey());
             }
             leafRecordLoadedCallback = (dataLocation, leafData) -> {
                 final VirtualLeafBytes leafBytes = VirtualLeafBytes.parseFrom(leafData);
-                pathToDiskLocationLeafNodes.put(leafBytes.path(), dataLocation);
+                final long path = leafBytes.path();
+                // Old data files may contain entries with paths outside the current leaf range
+                if (validLeafPathRange.withinRange(path)) {
+                    pathToDiskLocationLeafNodes.put(path, dataLocation);
+                }
             };
         } else {
             leafRecordLoadedCallback = null;

platform-sdk/swirlds-merkledb/src/timingSensitive/java/com/swirlds/merkledb/MerkleDbDataSourceTest.java

@@ -479,45 +479,65 @@ void snapshotRestoreIndex(final TestType testType) throws IOException {
         final Path originalDbPath = testDirectory.resolve("merkledb-snapshotRestoreIndex-" + testType);
         final KeySerializer keySerializer = testType.dataType().getKeySerializer();
         final ValueSerializer valueSerializer = testType.dataType().getValueSerializer();
-        createAndApplyDataSource(originalDbPath, tableName, testType, count, 0, dataSource -> {
-            final int tableId = dataSource.getTableId();
-            // create some leaves
-            dataSource.saveRecords(
-                    count - 1,
-                    count * 2 - 2,
-                    IntStream.range(0, count * 2 - 1).mapToObj(i -> createVirtualInternalRecord(i, i + 1)),
-                    IntStream.range(count - 1, count * 2 - 1)
-                            .mapToObj(i -> testType.dataType().createVirtualLeafRecord(i))
-                            .map(r -> r.toBytes(keySerializer, valueSerializer)),
-                    Stream.empty());
-            // create a snapshot
-            final Path snapshotDbPath =
-                    testDirectory.resolve("merkledb-snapshotRestoreIndex-" + testType + "_SNAPSHOT");
-            dataSource.getDatabase().snapshot(snapshotDbPath, dataSource);
-            // close data source
-            dataSource.close();
-
-            final MerkleDb snapshotDb = MerkleDb.getInstance(snapshotDbPath, CONFIGURATION);
-            final MerkleDbPaths snapshotPaths = new MerkleDbPaths(snapshotDb.getTableDir(tableName, tableId));
-            // Delete all indices
-            Files.delete(snapshotPaths.pathToDiskLocationLeafNodesFile);
-            Files.delete(snapshotPaths.pathToDiskLocationInternalNodesFile);
-            // There is no way to use MerkleDbPaths to get bucket index file path
-            Files.deleteIfExists(snapshotPaths.keyToPathDirectory.resolve(tableName + "_bucket_index.ll"));
-
-            final MerkleDbDataSource snapshotDataSource = snapshotDb.getDataSource(tableName, false);
-            reinitializeDirectMemoryUsage();
-            IntStream.range(0, count * 2 - 1).forEach(i -> assertHash(snapshotDataSource, i, i + 1));
-            IntStream.range(count - 1, count * 2 - 1)
-                    .forEach(i ->
-                            assertLeaf(testType, keySerializer, valueSerializer, snapshotDataSource, i, i, i + 1, i));
-            // close data source
-            snapshotDataSource.close();
-
-            // check db count
-            assertEventuallyEquals(
-                    0L, MerkleDbDataSource::getCountOfOpenDatabases, Duration.ofSeconds(1), "Expected no open dbs");
-        });
+        final int[] deltas = {-10, 0, 10};
+        for (int delta : deltas) {
+            createAndApplyDataSource(originalDbPath, tableName, testType, count + Math.abs(delta), 0, dataSource -> {
+                final int tableId = dataSource.getTableId();
+                // create some records
+                dataSource.saveRecords(
+                        count - 1,
+                        count * 2 - 2,
+                        IntStream.range(0, count * 2 - 1).mapToObj(i -> createVirtualInternalRecord(i, i + 1)),
+                        IntStream.range(count - 1, count * 2 - 1)
+                                .mapToObj(i -> testType.dataType().createVirtualLeafRecord(i))
+                                .map(r -> r.toBytes(keySerializer, valueSerializer)),
+                        Stream.empty());
+                if (delta != 0) {
+                    // create some more, current leaf path range shifted by delta
+                    dataSource.saveRecords(
+                            count - 1 + delta,
+                            count * 2 - 2 + 2 * delta,
+                            IntStream.range(0, count * 2 - 1 + 2 * delta)
+                                    .mapToObj(i -> createVirtualInternalRecord(i, i + 1)),
+                            IntStream.range(count - 1 + delta, count * 2 - 1 + 2 * delta)
+                                    .mapToObj(i -> testType.dataType().createVirtualLeafRecord(i))
+                                    .map(r -> r.toBytes(keySerializer, valueSerializer)),
+                            Stream.empty());
+                }
+                // create a snapshot
+                final Path snapshotDbPath =
+                        testDirectory.resolve("merkledb-snapshotRestoreIndex-" + testType + "_SNAPSHOT");
+                dataSource.getDatabase().snapshot(snapshotDbPath, dataSource);
+                // close data source
+                dataSource.close();
+
+                final MerkleDb snapshotDb = MerkleDb.getInstance(snapshotDbPath, CONFIGURATION);
+                final MerkleDbPaths snapshotPaths = new MerkleDbPaths(snapshotDb.getTableDir(tableName, tableId));
+                // Delete all indices
+                Files.delete(snapshotPaths.pathToDiskLocationLeafNodesFile);
+                Files.delete(snapshotPaths.pathToDiskLocationInternalNodesFile);
+                // There is no way to use MerkleDbPaths to get bucket index file path
+                Files.deleteIfExists(snapshotPaths.keyToPathDirectory.resolve(tableName + "_bucket_index.ll"));
+
+                final MerkleDbDataSource snapshotDataSource = snapshotDb.getDataSource(tableName, false);
+                reinitializeDirectMemoryUsage();
+                // Check hashes
+                IntStream.range(0, count * 2 - 1 + 2 * delta).forEach(i -> assertHash(snapshotDataSource, i, i + 1));
+                assertNullHash(snapshotDataSource, count * 2 + 2 * delta);
+                // Check leaves
+                IntStream.range(0, count - 2 + delta).forEach(i -> assertNullLeaf(snapshotDataSource, i));
+                IntStream.range(count - 1 + delta, count * 2 - 1 + 2 * delta)
+                        .forEach(i -> assertLeaf(
+                                testType, keySerializer, valueSerializer, snapshotDataSource, i, i, i + 1, i));
+                assertNullLeaf(snapshotDataSource, count * 2 + 2 * delta);
+                // close data source
+                snapshotDataSource.close();
+
+                // check db count
+                assertEventuallyEquals(
+                        0L, MerkleDbDataSource::getCountOfOpenDatabases, Duration.ofSeconds(1), "Expected no open dbs");
+            });
+        }
     }
 
     @Test
@@ -789,7 +809,16 @@ public static void assertHash(final MerkleDbDataSource dataSource, final long pa
         try {
             assertEqualsAndPrint(hash(i), dataSource.loadHash(path));
         } catch (final Exception e) {
-            e.printStackTrace();
+            e.printStackTrace(System.err);
+            fail("Exception should not have been thrown here!");
+        }
+    }
+
+    public static void assertNullHash(final MerkleDbDataSource dataSource, final long path) {
+        try {
+            assertNull(dataSource.loadHash(path));
+        } catch (final Exception e) {
+            e.printStackTrace(System.err);
             fail("Exception should not have been thrown here!");
         }
     }
@@ -830,6 +859,15 @@ public static void assertLeaf(
         }
     }
 
+    public static void assertNullLeaf(final MerkleDbDataSource dataSource, final long path) {
+        try {
+            assertNull(dataSource.loadLeafRecord(path));
+        } catch (final Exception e) {
+            e.printStackTrace(System.err);
+            fail("Exception should not have been thrown here!");
+        }
+    }
+
     public static <T> void assertEqualsAndPrint(final T recordA, final T recordB) {
         assertEquals(
                 recordA == null ? null : recordA.toString(),