You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "hemantk-12 (via GitHub)" <gi...@apache.org> on 2023/03/17 00:10:35 UTC
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4376: HDDS-8137. Snapdiff to use tombstone entries in SST file
hemantk-12 commented on code in PR #4376:
URL: https://github.com/apache/ozone/pull/4376#discussion_r1139407239
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java:
##########
@@ -597,6 +597,21 @@ public final class OzoneConfigKeys {
OZONE_OM_SNAPSHOT_COMPACTION_DAG_PRUNE_DAEMON_RUN_INTERVAL =
"ozone.om.snapshot.compaction.dag.prune.daemon.run.interval";
+ public static final String
+ OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE =
Review Comment:
nit: just to be in alignment with other configs.
```suggestion
OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE =
"ozone.om.snapshot.sst.dumptool.pool.size";
```
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSSTDumpIterator.java:
##########
@@ -116,13 +116,20 @@ public boolean hasNext() {
return nextKey != null;
}
+ /**
+ * Transform function to transform key to a certain value.
+ * @param value
+ * @return
Review Comment:
Either add the return type or remove it.
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSSTDumpIterator.java:
##########
@@ -116,13 +116,20 @@ public boolean hasNext() {
return nextKey != null;
}
+ /**
+ * Transform function to transform key to a certain value.
+ * @param value
+ * @return
+ */
+ protected abstract T getTransformedValue(KeyValue value);
+
/**
* Returns the next record from SSTDumpTool.
* @return next Key
* Throws Runtime Exception incase of failure.
Review Comment:
Should this need to be changed to `NoSuchElementException`?
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/ManagedSstFileReader.java:
##########
@@ -89,37 +179,33 @@ public boolean hasNext() {
}
@Override
- public String next() {
+ public T next() {
if (hasNext()) {
- final String value = new String(currentFileIterator.key(), UTF_8);
- currentFileIterator.next();
- return value;
+ return currentFileIterator.next();
+// final String value = new String(currentFileIterator.key(), UTF_8);
Review Comment:
nit: Remove this if not needed.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -217,37 +270,91 @@ public SnapshotDiffReport getSnapshotDiffReport(final String volume,
Map<String, String> tablePrefixes =
getTablePrefixes(toSnapshot.getMetadataManager(), volume, bucket);
-
- final Set<String> deltaFilesForKeyOrFileTable =
- getDeltaFiles(fromSnapshot, toSnapshot,
- Collections.singletonList(fsKeyTable.getName()), fsInfo, tsInfo,
+ List<String> tablesToLookUp =
+ Collections.singletonList(fsKeyTable.getName());
+ final Set<String> deltaFilesForKeyOrFileTable = getDeltaFiles(
+ fromSnapshot, toSnapshot, tablesToLookUp, fsInfo, tsInfo,
useFullDiff, tablePrefixes);
- addToObjectIdMap(fsKeyTable,
- tsKeyTable,
- deltaFilesForKeyOrFileTable,
- objectIdToKeyNameMapForFromSnapshot,
- objectIdToKeyNameMapForToSnapshot,
- objectIDsToCheckMap,
- tablePrefixes);
+ // Workaround to handle deletes if native rockstools for reading
+ // tombstone is not loaded.
+ // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read to
+ // read tombstone
+ if (!isNativeRocksToolsLoaded) {
+ deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
+ fromSnapshot, tablesToLookUp));
+ }
+ try {
+ addToObjectIdMap(fsKeyTable,
+ tsKeyTable,
+ Pair.of(isNativeRocksToolsLoaded, deltaFilesForKeyOrFileTable),
+ objectIdToKeyNameMapForFromSnapshot,
+ objectIdToKeyNameMapForToSnapshot,
+ objectIDsToCheckMap,
+ tablePrefixes);
+ } catch (NativeLibraryNotLoadedException e) {
+ // Workaround to handle deletes if use of native rockstools for reading
+ // tombstone fails.
+ // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read to
+ // read tombstone
+ deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
+ fromSnapshot, tablesToLookUp));
+ try {
+ addToObjectIdMap(fsKeyTable,
+ tsKeyTable,
+ Pair.of(false, deltaFilesForKeyOrFileTable),
+ objectIdToKeyNameMapForFromSnapshot,
+ objectIdToKeyNameMapForToSnapshot,
+ objectIDsToCheckMap,
+ tablePrefixes);
+ } catch (NativeLibraryNotLoadedException ex) {
+ //This code should never be never executed.
Review Comment:
Comment is not redundant. You could use `IllegalStateException` instead of `RuntimeException` to denote the this situation should not happen.
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java:
##########
@@ -800,4 +802,12 @@ public static Map<String, String> processForLogging(OzoneConfiguration conf) {
}
return sortedOzoneProps;
}
+
+ /**
+ * Interface for Implementing CloseableIterators.
+ * @param <T> Generic Parameter for Iterating values of type 'T'
+ */
+ public interface CloseableIterator<T> extends Iterator<T>, Closeable {
Review Comment:
I think it is unnecessary interface. You can directly implement `Iterator` and `Closeable` wherever is needed.
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSSTDumpIterator.java:
##########
@@ -116,13 +116,20 @@ public boolean hasNext() {
return nextKey != null;
}
+ /**
+ * Transform function to transform key to a certain value.
Review Comment:
```suggestion
* Transforms key to a certain value.
```
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSSTDumpIterator.java:
##########
@@ -139,8 +146,9 @@ public KeyValue next() {
if (currentKey != null) {
currentKey.setValue(stdoutString.substring(0,
Math.max(stdoutString.length() - 1, 0)));
+ return getTransformedValue(currentKey);
}
- return currentKey;
+ throw new NoSuchElementException("No more records found");
Review Comment:
```suggestion
throw new NoSuchElementException("No more elements found.");
```
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/ManagedSstFileReader.java:
##########
@@ -46,41 +47,130 @@ public class ManagedSstFileReader {
public ManagedSstFileReader(final Collection<String> sstFiles) {
this.sstFiles = sstFiles;
}
- public Stream<String> getKeyStream() throws RocksDBException {
- final ManagedSstFileIterator itr = new ManagedSstFileIterator(sstFiles);
- final Spliterator<String> spliterator = Spliterators
- .spliteratorUnknownSize(itr, 0);
- return StreamSupport.stream(spliterator, false).onClose(itr::close);
+
+ public Stream<String> getKeyStream() throws RocksDBException,
+ NativeLibraryNotLoadedException, IOException {
+ //TODO: [SNAPSHOT] Check if default Options and ReadOptions is enough.
+ ManagedOptions options = new ManagedOptions();
Review Comment:
```suggestion
final MultipleSstFileIterator<String> itr =
new MultipleSstFileIterator<String>(sstFiles) {
ManagedOptions options = new ManagedOptions();
ManagedReadOptions readOptions = new ManagedReadOptions();
@Override
protected HddsUtils.CloseableIterator<String>
initNewKeyIteratorForFile(String file) throws RocksDBException {
return new ManagedSstFileIterator<String>(file, options,
readOptions) {
@Override
protected String getIteratorValue(
SstFileReaderIterator iterator) {
return new String(iterator.key(), UTF_8);
}
};
}
@Override
public void close() throws IOException {
super.close();
options.close();
readOptions.close();
}
};
return RdbUtil.getStreamFromIterator(itr);
```
1. Defining `options` and `readOptions` inside implementation of MultipleSstFileIterator would be better. Otherwise if something gets added in between and don't close it properly, there will be a memory leak.
1. Use `ManagedReadOptions` instead of `ReadOptions`.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/ManagedSstFileReader.java:
##########
@@ -46,41 +47,130 @@ public class ManagedSstFileReader {
public ManagedSstFileReader(final Collection<String> sstFiles) {
this.sstFiles = sstFiles;
}
- public Stream<String> getKeyStream() throws RocksDBException {
- final ManagedSstFileIterator itr = new ManagedSstFileIterator(sstFiles);
- final Spliterator<String> spliterator = Spliterators
- .spliteratorUnknownSize(itr, 0);
- return StreamSupport.stream(spliterator, false).onClose(itr::close);
+
+ public Stream<String> getKeyStream() throws RocksDBException,
+ NativeLibraryNotLoadedException, IOException {
+ //TODO: [SNAPSHOT] Check if default Options and ReadOptions is enough.
+ ManagedOptions options = new ManagedOptions();
+ ReadOptions readOptions = new ReadOptions();
+ final MultipleSstFileIterator<String> itr =
+ new MultipleSstFileIterator<String>(sstFiles) {
+ @Override
+ protected HddsUtils.CloseableIterator<String>
+ initNewKeyIteratorForFile(String file) throws RocksDBException {
+ return new ManagedSstFileIterator<String>(file, options,
+ readOptions) {
+ @Override
+ protected String getIteratorValue(
+ SstFileReaderIterator iterator) {
+ return new String(iterator.key(), UTF_8);
+ }
+ };
+ }
+
+ @Override
+ public void close() throws IOException {
+ super.close();
+ options.close();
+ readOptions.close();
+ }
+ };
+ return RdbUtil.getStreamFromIterator(itr);
+ }
+ public Stream<String> getKeyStreamWithTombstone(
+ ManagedSSTDumpTool sstDumpTool) throws IOException, RocksDBException,
+ NativeLibraryNotLoadedException {
+ //TODO: [SNAPSHOT] Check if default Options is enough.
+ ManagedOptions options = new ManagedOptions();
Review Comment:
Move this inside implementation (after line 85).
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/ManagedSstFileReader.java:
##########
@@ -46,41 +47,130 @@ public class ManagedSstFileReader {
public ManagedSstFileReader(final Collection<String> sstFiles) {
this.sstFiles = sstFiles;
}
- public Stream<String> getKeyStream() throws RocksDBException {
- final ManagedSstFileIterator itr = new ManagedSstFileIterator(sstFiles);
- final Spliterator<String> spliterator = Spliterators
- .spliteratorUnknownSize(itr, 0);
- return StreamSupport.stream(spliterator, false).onClose(itr::close);
+
+ public Stream<String> getKeyStream() throws RocksDBException,
+ NativeLibraryNotLoadedException, IOException {
+ //TODO: [SNAPSHOT] Check if default Options and ReadOptions is enough.
+ ManagedOptions options = new ManagedOptions();
+ ReadOptions readOptions = new ReadOptions();
+ final MultipleSstFileIterator<String> itr =
+ new MultipleSstFileIterator<String>(sstFiles) {
+ @Override
+ protected HddsUtils.CloseableIterator<String>
+ initNewKeyIteratorForFile(String file) throws RocksDBException {
+ return new ManagedSstFileIterator<String>(file, options,
+ readOptions) {
+ @Override
+ protected String getIteratorValue(
+ SstFileReaderIterator iterator) {
+ return new String(iterator.key(), UTF_8);
+ }
+ };
+ }
+
+ @Override
+ public void close() throws IOException {
+ super.close();
+ options.close();
+ readOptions.close();
+ }
+ };
+ return RdbUtil.getStreamFromIterator(itr);
+ }
+ public Stream<String> getKeyStreamWithTombstone(
+ ManagedSSTDumpTool sstDumpTool) throws IOException, RocksDBException,
+ NativeLibraryNotLoadedException {
+ //TODO: [SNAPSHOT] Check if default Options is enough.
+ ManagedOptions options = new ManagedOptions();
+ final MultipleSstFileIterator<String> itr =
+ new MultipleSstFileIterator<String>(sstFiles) {
+ @Override
+ protected HddsUtils.CloseableIterator<String>
+ initNewKeyIteratorForFile(String file)
+ throws NativeLibraryNotLoadedException, IOException {
+ return new ManagedSSTDumpIterator<String>(sstDumpTool, file,
+ options) {
+ @Override
+ protected String getTransformedValue(KeyValue value) {
+ return value.getKey();
+ }
+ };
+ }
+
+ @Override
+ public void close() throws IOException {
+ super.close();
+ options.close();
+ }
+ };
+ return RdbUtil.getStreamFromIterator(itr);
+ }
+
+ private abstract static class ManagedSstFileIterator<T> implements
+ HddsUtils.CloseableIterator<T> {
+ private SstFileReader fileReader;
+ private SstFileReaderIterator fileReaderIterator;
+
+ ManagedSstFileIterator(String path, ManagedOptions options,
+ ReadOptions readOptions)
+ throws RocksDBException {
+ this.fileReader = new SstFileReader(options);
+ this.fileReader.open(path);
+ this.fileReaderIterator = fileReader.newIterator(readOptions);
+ fileReaderIterator.seekToFirst();
+ }
+
+ @Override
+ public void close() throws IOException {
+ this.fileReaderIterator.close();
+ this.fileReader.close();
+ }
+
+ @Override
+ public boolean hasNext() {
+ return fileReaderIterator.isValid();
+ }
+
+ protected abstract T getIteratorValue(SstFileReaderIterator iterator);
+
+ @Override
+ public T next() {
+ T value = getIteratorValue(fileReaderIterator);
+ fileReaderIterator.next();
+ return value;
+ }
}
- private static final class ManagedSstFileIterator implements
- Iterator<String>, Closeable {
+ private abstract static class MultipleSstFileIterator<T> implements
+ HddsUtils.CloseableIterator<T> {
private final Iterator<String> fileNameIterator;
- private final Options options;
- private final ReadOptions readOptions;
+
private String currentFile;
private SstFileReader currentFileReader;
- private SstFileReaderIterator currentFileIterator;
+ private HddsUtils.CloseableIterator<T> currentFileIterator;
- private ManagedSstFileIterator(Collection<String> files)
- throws RocksDBException {
- // TODO: Check if default Options and ReadOptions is enough.
- this.options = new Options();
- this.readOptions = new ReadOptions();
+ private MultipleSstFileIterator(Collection<String> files)
+ throws IOException, RocksDBException,
+ NativeLibraryNotLoadedException {
this.fileNameIterator = files.iterator();
moveToNextFile();
}
+ protected abstract HddsUtils.CloseableIterator<T> initNewKeyIteratorForFile(
+ String file) throws RocksDBException,
+ NativeLibraryNotLoadedException, IOException;
+
@Override
public boolean hasNext() {
try {
do {
- if (currentFileIterator.isValid()) {
+ if (currentFileIterator.hasNext()) {
return true;
}
} while (moveToNextFile());
- } catch (RocksDBException e) {
+ } catch (IOException | RocksDBException |
+ NativeLibraryNotLoadedException e) {
// TODO: This exception has to be handled by the caller.
Review Comment:
```suggestion
// TODO: [Snapshot] This exception has to be handled by the caller.
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -217,37 +270,91 @@ public SnapshotDiffReport getSnapshotDiffReport(final String volume,
Map<String, String> tablePrefixes =
getTablePrefixes(toSnapshot.getMetadataManager(), volume, bucket);
-
- final Set<String> deltaFilesForKeyOrFileTable =
- getDeltaFiles(fromSnapshot, toSnapshot,
- Collections.singletonList(fsKeyTable.getName()), fsInfo, tsInfo,
+ List<String> tablesToLookUp =
+ Collections.singletonList(fsKeyTable.getName());
+ final Set<String> deltaFilesForKeyOrFileTable = getDeltaFiles(
+ fromSnapshot, toSnapshot, tablesToLookUp, fsInfo, tsInfo,
useFullDiff, tablePrefixes);
- addToObjectIdMap(fsKeyTable,
- tsKeyTable,
- deltaFilesForKeyOrFileTable,
- objectIdToKeyNameMapForFromSnapshot,
- objectIdToKeyNameMapForToSnapshot,
- objectIDsToCheckMap,
- tablePrefixes);
+ // Workaround to handle deletes if native rockstools for reading
+ // tombstone is not loaded.
+ // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read to
+ // read tombstone
+ if (!isNativeRocksToolsLoaded) {
+ deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
+ fromSnapshot, tablesToLookUp));
+ }
+ try {
+ addToObjectIdMap(fsKeyTable,
+ tsKeyTable,
+ Pair.of(isNativeRocksToolsLoaded, deltaFilesForKeyOrFileTable),
+ objectIdToKeyNameMapForFromSnapshot,
+ objectIdToKeyNameMapForToSnapshot,
+ objectIDsToCheckMap,
+ tablePrefixes);
+ } catch (NativeLibraryNotLoadedException e) {
+ // Workaround to handle deletes if use of native rockstools for reading
+ // tombstone fails.
+ // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read to
+ // read tombstone
+ deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
+ fromSnapshot, tablesToLookUp));
+ try {
+ addToObjectIdMap(fsKeyTable,
+ tsKeyTable,
+ Pair.of(false, deltaFilesForKeyOrFileTable),
+ objectIdToKeyNameMapForFromSnapshot,
+ objectIdToKeyNameMapForToSnapshot,
+ objectIDsToCheckMap,
+ tablePrefixes);
+ } catch (NativeLibraryNotLoadedException ex) {
+ //This code should never be never executed.
+ throw new RuntimeException(ex);
+ }
+ }
if (bucketLayout.isFileSystemOptimized()) {
// add to object ID map for directory.
final Table<String, OmDirectoryInfo> fsDirTable =
fromSnapshot.getMetadataManager().getDirectoryTable();
final Table<String, OmDirectoryInfo> tsDirTable =
toSnapshot.getMetadataManager().getDirectoryTable();
+ tablesToLookUp = Collections.singletonList(fsDirTable.getName());
final Set<String> deltaFilesForDirTable =
- getDeltaFiles(fromSnapshot, toSnapshot,
- Collections.singletonList(fsDirTable.getName()), fsInfo, tsInfo,
- useFullDiff, tablePrefixes);
- addToObjectIdMap(fsDirTable,
- tsDirTable,
- deltaFilesForDirTable,
- objectIdToKeyNameMapForFromSnapshot,
- objectIdToKeyNameMapForToSnapshot,
- objectIDsToCheckMap,
- tablePrefixes);
+ getDeltaFiles(fromSnapshot, toSnapshot, tablesToLookUp, fsInfo,
+ tsInfo, useFullDiff, tablePrefixes);
+ if (!isNativeRocksToolsLoaded) {
+ deltaFilesForDirTable.addAll(getSSTFileListForSnapshot(
+ fromSnapshot, tablesToLookUp));
+ }
+ try {
+ addToObjectIdMap(fsDirTable,
+ tsDirTable,
+ Pair.of(isNativeRocksToolsLoaded, deltaFilesForDirTable),
+ objectIdToKeyNameMapForFromSnapshot,
+ objectIdToKeyNameMapForToSnapshot,
+ objectIDsToCheckMap,
+ tablePrefixes);
+ } catch (NativeLibraryNotLoadedException e) {
+ try {
+ // Workaround to handle deletes if use of native rockstools for
+ // reading tombstone fails.
+ // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read to
+ // read tombstone
+ deltaFilesForDirTable.addAll(getSSTFileListForSnapshot(
+ fromSnapshot, tablesToLookUp));
+ addToObjectIdMap(fsDirTable,
+ tsDirTable,
+ Pair.of(false, deltaFilesForDirTable),
+ objectIdToKeyNameMapForFromSnapshot,
+ objectIdToKeyNameMapForToSnapshot,
+ objectIDsToCheckMap,
+ tablePrefixes);
+ } catch (NativeLibraryNotLoadedException ex) {
+ //This code should never be never executed.
Review Comment:
Same as above. Use `IllegalStateException`.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/ManagedSstFileReader.java:
##########
@@ -46,41 +47,130 @@ public class ManagedSstFileReader {
public ManagedSstFileReader(final Collection<String> sstFiles) {
this.sstFiles = sstFiles;
}
- public Stream<String> getKeyStream() throws RocksDBException {
- final ManagedSstFileIterator itr = new ManagedSstFileIterator(sstFiles);
- final Spliterator<String> spliterator = Spliterators
- .spliteratorUnknownSize(itr, 0);
- return StreamSupport.stream(spliterator, false).onClose(itr::close);
+
+ public Stream<String> getKeyStream() throws RocksDBException,
+ NativeLibraryNotLoadedException, IOException {
+ //TODO: [SNAPSHOT] Check if default Options and ReadOptions is enough.
+ ManagedOptions options = new ManagedOptions();
+ ReadOptions readOptions = new ReadOptions();
+ final MultipleSstFileIterator<String> itr =
+ new MultipleSstFileIterator<String>(sstFiles) {
+ @Override
+ protected HddsUtils.CloseableIterator<String>
+ initNewKeyIteratorForFile(String file) throws RocksDBException {
+ return new ManagedSstFileIterator<String>(file, options,
+ readOptions) {
+ @Override
+ protected String getIteratorValue(
+ SstFileReaderIterator iterator) {
+ return new String(iterator.key(), UTF_8);
+ }
+ };
+ }
+
+ @Override
+ public void close() throws IOException {
+ super.close();
+ options.close();
+ readOptions.close();
+ }
+ };
+ return RdbUtil.getStreamFromIterator(itr);
+ }
+ public Stream<String> getKeyStreamWithTombstone(
+ ManagedSSTDumpTool sstDumpTool) throws IOException, RocksDBException,
+ NativeLibraryNotLoadedException {
+ //TODO: [SNAPSHOT] Check if default Options is enough.
+ ManagedOptions options = new ManagedOptions();
+ final MultipleSstFileIterator<String> itr =
+ new MultipleSstFileIterator<String>(sstFiles) {
+ @Override
+ protected HddsUtils.CloseableIterator<String>
+ initNewKeyIteratorForFile(String file)
+ throws NativeLibraryNotLoadedException, IOException {
+ return new ManagedSSTDumpIterator<String>(sstDumpTool, file,
+ options) {
+ @Override
+ protected String getTransformedValue(KeyValue value) {
+ return value.getKey();
+ }
+ };
+ }
+
+ @Override
+ public void close() throws IOException {
+ super.close();
+ options.close();
+ }
+ };
+ return RdbUtil.getStreamFromIterator(itr);
+ }
+
+ private abstract static class ManagedSstFileIterator<T> implements
Review Comment:
`ManagedSstFileIterator` and `MultipleSstFileIterator` don't need to be generic. Both of them are internal to `ManagedSstFileReader` and `String` type. Making it generic is adding readability complexity.
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java:
##########
@@ -597,6 +597,21 @@ public final class OzoneConfigKeys {
OZONE_OM_SNAPSHOT_COMPACTION_DAG_PRUNE_DAEMON_RUN_INTERVAL =
"ozone.om.snapshot.compaction.dag.prune.daemon.run.interval";
+ public static final String
+ OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE =
+ "ozone.om.snapshot.sst_dumptool.pool.size";
+
+ public static final int
+ OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT = 1;
+
+ public static final String
Review Comment:
```suggestion
public static final String
OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE =
"ozone.om.snapshot.sst.dumptool.buffer.size";
```
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/RdbUtil.java:
##########
@@ -58,4 +65,17 @@ public static Set<String> getSSTFilesForComparison(final String dbLocation,
}
}
+ public static <T> Stream<T> getStreamFromIterator(
Review Comment:
I don't think it has to be a Util as of now and can be internal to `ManagedSstFileReader`.
Same goes for `getSSTFilesForComparison` but you can leave that as it is for now.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -92,6 +107,37 @@ public SnapshotDiffManager(ManagedRocksDB db,
// Need for Diff Report
this.codecRegistry.addCodec(DiffReportEntry.class,
new OmDBDiffReportEntryCodec());
+ isNativeRocksToolsLoaded = NativeLibraryLoader.getInstance()
+ .loadLibrary(NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME);
+ if (isNativeRocksToolsLoaded) {
+ try {
+ initSSTDumpTool(configuration);
+ isNativeRocksToolsLoaded = true;
+ } catch (NativeLibraryNotLoadedException e) {
+ LOG.error("Unable to load SSTDumpTool ", e);
+ isNativeRocksToolsLoaded = false;
+ }
+ }
+ }
+
+ private void initSSTDumpTool(OzoneConfiguration conf)
+ throws NativeLibraryNotLoadedException {
+ int threadPoolSize = conf.getInt(
+ OzoneConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE,
+ OzoneConfigKeys
+ .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT);
+ int bufferSize = (int)conf.getStorageSize(
+ OzoneConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE,
+ OzoneConfigKeys
+ .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE_DEFAULT,
+ StorageUnit.BYTES);
+ ExecutorService executorService = new ThreadPoolExecutor(0,
+ threadPoolSize, 60, TimeUnit.SECONDS,
+ new SynchronousQueue<>(), new ThreadFactoryBuilder()
Review Comment:
FYI: [SynchronousQueue](https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/SynchronousQueue.html) is unbounded key and can cause memory pressure.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/ManagedSstFileReader.java:
##########
@@ -89,37 +179,33 @@ public boolean hasNext() {
}
@Override
- public String next() {
+ public T next() {
if (hasNext()) {
- final String value = new String(currentFileIterator.key(), UTF_8);
- currentFileIterator.next();
- return value;
+ return currentFileIterator.next();
+// final String value = new String(currentFileIterator.key(), UTF_8);
}
throw new NoSuchElementException("No more keys");
Review Comment:
nit:
```suggestion
throw new NoSuchElementException("No more elements found.");
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org