From 81423f19535ed5336eff4a11930981360807b6e0 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 11 Dec 2024 17:23:06 -0800 Subject: [PATCH 1/2] HDDS-11908. Snapshot diff DAG traversal should not skip node based on prefix presence Change-Id: Ib96280e9d22f9a895899d56034cd8d020ec9081e --- .../rocksdiff/RocksDBCheckpointDiffer.java | 41 ------------------- .../ozone/rocksdiff/RocksDiffUtils.java | 33 +++++++++++++++ .../TestRocksDBCheckpointDiffer.java | 4 +- 3 files changed, 35 insertions(+), 43 deletions(-) diff --git a/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java b/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java index 08a013fc7c70..f241f93b93f9 100644 --- a/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java +++ b/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java @@ -33,7 +33,6 @@ import com.google.protobuf.InvalidProtocolBufferException; import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.StringUtils; import org.apache.hadoop.hdds.conf.ConfigurationSource; @@ -1006,15 +1005,6 @@ synchronized void internalGetSSTDiffList( } for (CompactionNode nextNode : successors) { - if (shouldSkipNode(nextNode, columnFamilyToPrefixMap)) { - LOG.debug("Skipping next node: '{}' with startKey: '{}' and " + - "endKey: '{}' because it doesn't have keys related to " + - "columnFamilyToPrefixMap: '{}'.", - nextNode.getFileName(), nextNode.getStartKey(), - nextNode.getEndKey(), columnFamilyToPrefixMap); - continue; - } - if (sameFiles.contains(nextNode.getFileName()) || differentFiles.contains(nextNode.getFileName())) { LOG.debug("Skipping known processed SST: {}", @@ -1536,35 +1526,4 @@ private CompactionFileInfo toFileInfo(String sstFile, return fileInfoBuilder.build(); } - @VisibleForTesting - boolean shouldSkipNode(CompactionNode node, - Map columnFamilyToPrefixMap) { - // This is for backward compatibility. Before the compaction log table - // migration, startKey, endKey and columnFamily information is not persisted - // in compaction log files. - // Also for the scenario when there is an exception in reading SST files - // for the file node. - if (node.getStartKey() == null || node.getEndKey() == null || - node.getColumnFamily() == null) { - LOG.debug("Compaction node with fileName: {} doesn't have startKey, " + - "endKey and columnFamily details.", node.getFileName()); - return false; - } - - if (MapUtils.isEmpty(columnFamilyToPrefixMap)) { - LOG.debug("Provided columnFamilyToPrefixMap is null or empty."); - return false; - } - - if (!columnFamilyToPrefixMap.containsKey(node.getColumnFamily())) { - LOG.debug("SstFile node: {} is for columnFamily: {} while filter map " + - "contains columnFamilies: {}.", node.getFileName(), - node.getColumnFamily(), columnFamilyToPrefixMap.keySet()); - return true; - } - - String keyPrefix = columnFamilyToPrefixMap.get(node.getColumnFamily()); - return !RocksDiffUtils.isKeyWithPrefixPresent(keyPrefix, node.getStartKey(), - node.getEndKey()); - } } diff --git a/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java b/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java index e116868410f1..576b932891b5 100644 --- a/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java +++ b/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java @@ -17,6 +17,8 @@ */ package org.apache.ozone.rocksdiff; +import com.google.common.annotations.VisibleForTesting; +import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hdds.utils.db.managed.ManagedOptions; import org.apache.hadoop.hdds.utils.db.managed.ManagedReadOptions; @@ -113,4 +115,35 @@ public static boolean doesSstFileContainKeyRange(String filepath, } + @VisibleForTesting + static boolean shouldSkipNode(CompactionNode node, + Map columnFamilyToPrefixMap) { + // This is for backward compatibility. Before the compaction log table + // migration, startKey, endKey and columnFamily information is not persisted + // in compaction log files. + // Also for the scenario when there is an exception in reading SST files + // for the file node. + if (node.getStartKey() == null || node.getEndKey() == null || + node.getColumnFamily() == null) { + LOG.debug("Compaction node with fileName: {} doesn't have startKey, " + + "endKey and columnFamily details.", node.getFileName()); + return false; + } + + if (MapUtils.isEmpty(columnFamilyToPrefixMap)) { + LOG.debug("Provided columnFamilyToPrefixMap is null or empty."); + return false; + } + + if (!columnFamilyToPrefixMap.containsKey(node.getColumnFamily())) { + LOG.debug("SstFile node: {} is for columnFamily: {} while filter map " + + "contains columnFamilies: {}.", node.getFileName(), + node.getColumnFamily(), columnFamilyToPrefixMap.keySet()); + return true; + } + + String keyPrefix = columnFamilyToPrefixMap.get(node.getColumnFamily()); + return !isKeyWithPrefixPresent(keyPrefix, node.getStartKey(), + node.getEndKey()); + } } diff --git a/hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java b/hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java index 0164e3a23bd5..f3868535a887 100644 --- a/hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java +++ b/hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java @@ -1794,7 +1794,7 @@ public void testShouldSkipNode(Map columnFamilyToPrefixMap, .getCompactionNodeMap().values().stream() .sorted(Comparator.comparing(CompactionNode::getFileName)) .map(node -> - rocksDBCheckpointDiffer.shouldSkipNode(node, + RocksDiffUtils.shouldSkipNode(node, columnFamilyToPrefixMap)) .collect(Collectors.toList()); @@ -1831,7 +1831,7 @@ public void testShouldSkipNodeEdgeCase( rocksDBCheckpointDiffer.loadAllCompactionLogs(); - assertEquals(expectedResponse, rocksDBCheckpointDiffer.shouldSkipNode(node, + assertEquals(expectedResponse, RocksDiffUtils.shouldSkipNode(node, columnFamilyToPrefixMap)); } From c4b5a3cdc2bd818c23fdce4903a4649103deeecc Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Thu, 12 Dec 2024 19:55:03 -0800 Subject: [PATCH 2/2] HDDS-11908. Fix findbugs Change-Id: Ia66e086a58f9b5b03c20300df35b0eed5025639c --- .../org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java b/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java index f241f93b93f9..6ef89c7b5916 100644 --- a/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java +++ b/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java @@ -938,10 +938,6 @@ synchronized void internalGetSSTDiffList( Preconditions.checkArgument(sameFiles.isEmpty(), "Set must be empty"); Preconditions.checkArgument(differentFiles.isEmpty(), "Set must be empty"); - // Use source snapshot's table prefix. At this point Source and target's - // table prefix should be same. - Map columnFamilyToPrefixMap = src.getTablePrefixes(); - for (String fileName : srcSnapFiles) { if (destSnapFiles.contains(fileName)) { LOG.debug("Source '{}' and destination '{}' share the same SST '{}'",