swamirishi commented on code in PR #4273:
URL: https://github.com/apache/ozone/pull/4273#discussion_r1109027241
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java:
##########
@@ -50,5 +62,48 @@ public static String constructBucketKey(String keyName) {
return builder.toString();
}
+ public static void filterRelevantSstFiles(Set<String> inputFiles,
+ Map<String, String> tableToPrefixMap) {
+ for (Iterator<String> fileIterator =
+ inputFiles.iterator(); fileIterator.hasNext();) {
+ String filepath = fileIterator.next();
+ if (!RocksDiffUtils.doesSstFileContainKeyRange(filepath,
+ tableToPrefixMap)) {
+ fileIterator.remove();
+ }
+ }
+ }
+
+ public static boolean doesSstFileContainKeyRange(String filepath,
+ Map<String, String> tableToPrefixMap) {
+ try (SstFileReader sstFileReader = new SstFileReader(new Options())) {
+ sstFileReader.open(filepath);
+ TableProperties properties = sstFileReader.getTableProperties();
+ String tableName = new String(properties.getColumnFamilyName(), UTF_8);
+ if (tableToPrefixMap.containsKey(tableName)) {
+ String prefix = tableToPrefixMap.get(tableName);
+ SstFileReaderIterator iterator =
+ sstFileReader.newIterator(new ReadOptions());
+ iterator.seekToFirst();
+ String firstKey = RocksDiffUtils.constructBucketKey(
+ new String(iterator.key(), UTF_8));
Review Comment:
Wouldn't it be better if we try to seek to the particular key? Instead of
just checking the range of the sst file.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java:
##########
@@ -50,5 +62,48 @@ public static String constructBucketKey(String keyName) {
return builder.toString();
}
+ public static void filterRelevantSstFiles(Set<String> inputFiles,
+ Map<String, String> tableToPrefixMap) {
+ for (Iterator<String> fileIterator =
+ inputFiles.iterator(); fileIterator.hasNext();) {
+ String filepath = fileIterator.next();
+ if (!RocksDiffUtils.doesSstFileContainKeyRange(filepath,
+ tableToPrefixMap)) {
+ fileIterator.remove();
+ }
+ }
+ }
+
+ public static boolean doesSstFileContainKeyRange(String filepath,
+ Map<String, String> tableToPrefixMap) {
+ try (SstFileReader sstFileReader = new SstFileReader(new Options())) {
+ sstFileReader.open(filepath);
+ TableProperties properties = sstFileReader.getTableProperties();
+ String tableName = new String(properties.getColumnFamilyName(), UTF_8);
+ if (tableToPrefixMap.containsKey(tableName)) {
+ String prefix = tableToPrefixMap.get(tableName);
+ SstFileReaderIterator iterator =
+ sstFileReader.newIterator(new ReadOptions());
+ iterator.seekToFirst();
+ String firstKey = RocksDiffUtils.constructBucketKey(
+ new String(iterator.key(), UTF_8));
+ iterator.seekToLast();
+ String lastKey = RocksDiffUtils.constructBucketKey(
+ new String(iterator.key(), UTF_8));
+ boolean keyWithPrefixPresent =
+ RocksDiffUtils.isKeyWithPrefixPresent(prefix, firstKey, lastKey);
+ if (!keyWithPrefixPresent) {
+ return false;
+ }
+ } else {
+ // entry from other tables
+ return false;
+ }
+ } catch (RocksDBException e) {
+ e.printStackTrace();
Review Comment:
Can we log this instead?
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java:
##########
@@ -50,5 +62,48 @@ public static String constructBucketKey(String keyName) {
return builder.toString();
}
+ public static void filterRelevantSstFiles(Set<String> inputFiles,
+ Map<String, String> tableToPrefixMap) {
+ for (Iterator<String> fileIterator =
+ inputFiles.iterator(); fileIterator.hasNext();) {
+ String filepath = fileIterator.next();
+ if (!RocksDiffUtils.doesSstFileContainKeyRange(filepath,
+ tableToPrefixMap)) {
+ fileIterator.remove();
+ }
+ }
+ }
+
+ public static boolean doesSstFileContainKeyRange(String filepath,
+ Map<String, String> tableToPrefixMap) {
+ try (SstFileReader sstFileReader = new SstFileReader(new Options())) {
+ sstFileReader.open(filepath);
+ TableProperties properties = sstFileReader.getTableProperties();
+ String tableName = new String(properties.getColumnFamilyName(), UTF_8);
+ if (tableToPrefixMap.containsKey(tableName)) {
Review Comment:
I guess this check is not required since we are already getting only those
sst files corresponding to the column families we are looking up. But yeah it
is a good to have check.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java:
##########
@@ -50,5 +62,48 @@ public static String constructBucketKey(String keyName) {
return builder.toString();
}
+ public static void filterRelevantSstFiles(Set<String> inputFiles,
+ Map<String, String> tableToPrefixMap) {
+ for (Iterator<String> fileIterator =
+ inputFiles.iterator(); fileIterator.hasNext();) {
+ String filepath = fileIterator.next();
+ if (!RocksDiffUtils.doesSstFileContainKeyRange(filepath,
+ tableToPrefixMap)) {
+ fileIterator.remove();
+ }
+ }
+ }
+
+ public static boolean doesSstFileContainKeyRange(String filepath,
+ Map<String, String> tableToPrefixMap) {
+ try (SstFileReader sstFileReader = new SstFileReader(new Options())) {
+ sstFileReader.open(filepath);
+ TableProperties properties = sstFileReader.getTableProperties();
+ String tableName = new String(properties.getColumnFamilyName(), UTF_8);
+ if (tableToPrefixMap.containsKey(tableName)) {
+ String prefix = tableToPrefixMap.get(tableName);
+ SstFileReaderIterator iterator =
+ sstFileReader.newIterator(new ReadOptions());
+ iterator.seekToFirst();
+ String firstKey = RocksDiffUtils.constructBucketKey(
+ new String(iterator.key(), UTF_8));
+ iterator.seekToLast();
+ String lastKey = RocksDiffUtils.constructBucketKey(
+ new String(iterator.key(), UTF_8));
+ boolean keyWithPrefixPresent =
+ RocksDiffUtils.isKeyWithPrefixPresent(prefix, firstKey, lastKey);
+ if (!keyWithPrefixPresent) {
Review Comment:
why can't we just return RocksDiffUtils.isKeyWithPrefixPresent(prefix,
firstKey, lastKey);
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -387,4 +412,27 @@ private boolean areKeysEqual(WithObjectID oldKey,
WithObjectID newKey) {
}
return false;
}
+
+ /**
+ * check if the given key is in the bucket specified by tablePrefix map.
+ */
+ private boolean isKeyInBucket(String key, Map<String, String> tablePrefixes,
+ String tableName) {
+ String volumeBucketDbPrefix;
+ // In case of FSO - either File/Directory table
+ // the key Prefix would be volumeId/bucketId and
+ // in case of non-fso - volumeName/bucketName
+ if (tableName.equals(
+ OmMetadataManagerImpl.DIRECTORY_TABLE) || tableName.equals(
+ OmMetadataManagerImpl.FILE_TABLE)) {
+ volumeBucketDbPrefix =
+ tablePrefixes.get(OmMetadataManagerImpl.DIRECTORY_TABLE);
+ } else {
+ volumeBucketDbPrefix =
tablePrefixes.get(OmMetadataManagerImpl.KEY_TABLE);
+ }
+ if (key.startsWith(volumeBucketDbPrefix)) {
Review Comment:
return key.startsWith(volumeBucketDbPrefix);
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java:
##########
@@ -50,5 +62,48 @@ public static String constructBucketKey(String keyName) {
return builder.toString();
}
+ public static void filterRelevantSstFiles(Set<String> inputFiles,
+ Map<String, String> tableToPrefixMap) {
+ for (Iterator<String> fileIterator =
+ inputFiles.iterator(); fileIterator.hasNext();) {
+ String filepath = fileIterator.next();
+ if (!RocksDiffUtils.doesSstFileContainKeyRange(filepath,
+ tableToPrefixMap)) {
+ fileIterator.remove();
+ }
+ }
+ }
+
+ public static boolean doesSstFileContainKeyRange(String filepath,
+ Map<String, String> tableToPrefixMap) {
+ try (SstFileReader sstFileReader = new SstFileReader(new Options())) {
+ sstFileReader.open(filepath);
+ TableProperties properties = sstFileReader.getTableProperties();
+ String tableName = new String(properties.getColumnFamilyName(), UTF_8);
+ if (tableToPrefixMap.containsKey(tableName)) {
+ String prefix = tableToPrefixMap.get(tableName);
+ SstFileReaderIterator iterator =
+ sstFileReader.newIterator(new ReadOptions());
+ iterator.seekToFirst();
+ String firstKey = RocksDiffUtils.constructBucketKey(
+ new String(iterator.key(), UTF_8));
Review Comment:
As in it could be the case that SST file doesn't have entry for the bucket
itself. Correct me if I am wrong on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
