hemantk-12 commented on code in PR #6182: URL: https://github.com/apache/ozone/pull/6182#discussion_r1496715891
########## hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRawSSTFileReader.java: ########## @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.utils.db.managed; + +import org.apache.hadoop.hdds.utils.NativeLibraryLoader; +import org.apache.hadoop.hdds.utils.NativeLibraryNotLoadedException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.Closeable; +import java.util.function.Function; + +import static org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME; + +/** + * JNI for RocksDB RawSSTFileReader. + */ +public class ManagedRawSSTFileReader<T> implements Closeable { + + public static boolean loadLibrary() throws NativeLibraryNotLoadedException { + ManagedRocksObjectUtils.loadRocksDBLibrary(); + if (!NativeLibraryLoader.getInstance() + .loadLibrary(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) { + throw new NativeLibraryNotLoadedException( + ROCKS_TOOLS_NATIVE_LIBRARY_NAME); + } + return true; + } Review Comment: nit: 1. I think, this code should be moved to `SnapshotDiffManager#initSSTDumpTool()`. And in test should load lib wherever is needed in @BeforeAll. `ManagedRawSSTFileReader` class in itself doesn't care if native lib is loaded or not. And rely on the caller that they should load it before using it. 2. May be call `loadNativeLib` rather than `initSSTDumpTool`. ########## hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRawSSTFileReaderIterator.java: ########## @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.utils.db.managed; + +import com.google.common.primitives.UnsignedLong; +import org.apache.hadoop.hdds.StringUtils; +import org.apache.hadoop.util.ClosableIterator; + +import java.util.Arrays; +import java.util.function.Function; + +/** + * Iterator for SSTFileReader which would read all entries including tombstones. + */ +public class ManagedRawSSTFileReaderIterator<T> + implements ClosableIterator<T> { + private final long nativeHandle; + private final Function<KeyValue, T> transformer; + + ManagedRawSSTFileReaderIterator(long nativeHandle, + Function<KeyValue, T> transformer) { + this.nativeHandle = nativeHandle; + this.transformer = transformer; + } + + private native boolean hasNext(long handle); + private native void next(long handle); + private native byte[] getKey(long handle); + private native byte[] getValue(long handle); + private native long getSequenceNumber(long handle); + private native int getType(long handle); + + @Override + public boolean hasNext() { + return this.hasNext(nativeHandle); + } + + @Override + public T next() { + if (!hasNext()) { + return null; Review Comment: nit: this should throw `NoSuchElementException` in this case. ########## hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRawSSTFileReader.java: ########## @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.utils.db.managed; + +import org.apache.hadoop.hdds.utils.NativeLibraryLoader; +import org.apache.hadoop.hdds.utils.NativeLibraryNotLoadedException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.Closeable; +import java.util.function.Function; + +import static org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME; + +/** + * JNI for RocksDB RawSSTFileReader. + */ +public class ManagedRawSSTFileReader<T> implements Closeable { + + public static boolean loadLibrary() throws NativeLibraryNotLoadedException { + ManagedRocksObjectUtils.loadRocksDBLibrary(); + if (!NativeLibraryLoader.getInstance() + .loadLibrary(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) { + throw new NativeLibraryNotLoadedException( + ROCKS_TOOLS_NATIVE_LIBRARY_NAME); + } + return true; + } + + private final ManagedOptions options; Review Comment: 1. We are not using `options` and `readAheadSize` inside `ManagedRawSSTFileReader`, they are just passed to `newRawSSTFileReader`. There is no need to keep them as class variable of `ManagedRawSSTFileReader`. 2. Directly pass `options.getNativeHandle()` to `ManagedRawSSTFileReader`'s constructor. ########## hadoop-hdds/rocks-native/src/main/native/ManagedRawSSTFileReaderIterator.cpp: ########## @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReaderIterator.h" +#include "rocksdb/options.h" +#include "rocksdb/raw_iterator.h" +#include <string> +#include "cplusplus_to_java_convert.h" +#include <iostream> + +jboolean Java_org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReaderIterator_hasNext(JNIEnv *env, jobject obj, + jlong native_handle) { + return static_cast<jboolean>(reinterpret_cast<ROCKSDB_NAMESPACE::RawIterator*>(native_handle)->has_next()); +} + +void Java_org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReaderIterator_next(JNIEnv *env, jobject obj, + jlong native_handle) { + reinterpret_cast<ROCKSDB_NAMESPACE::RawIterator*>(native_handle)->next(); +} + +jbyteArray Java_org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReaderIterator_getKey(JNIEnv *env, + jobject obj, + jlong native_handle) { + ROCKSDB_NAMESPACE::Slice slice = reinterpret_cast<ROCKSDB_NAMESPACE::RawIterator*>(native_handle)->getKey(); + jbyteArray jkey = env->NewByteArray(static_cast<jsize>(slice.size())); + if (jkey == nullptr) { + // exception thrown: OutOfMemoryError + return nullptr; + } + env->SetByteArrayRegion( + jkey, 0, static_cast<jsize>(slice.size()), + const_cast<jbyte*>(reinterpret_cast<const jbyte*>(slice.data()))); + return jkey; +} + + +jbyteArray Java_org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReaderIterator_getValue(JNIEnv *env, + jobject obj, + jlong native_handle) { + ROCKSDB_NAMESPACE::Slice slice = reinterpret_cast<ROCKSDB_NAMESPACE::RawIterator*>(native_handle)->getValue(); + jbyteArray jkey = env->NewByteArray(static_cast<jsize>(slice.size())); + if (jkey == nullptr) { + // exception thrown: OutOfMemoryError + return nullptr; + } + env->SetByteArrayRegion( + jkey, 0, static_cast<jsize>(slice.size()), + const_cast<jbyte*>(reinterpret_cast<const jbyte*>(slice.data()))); + return jkey; +} + +jlong Java_org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReaderIterator_getSequenceNumber(JNIEnv *env, + jobject obj, + jlong native_handle) { + uint64_t sequence_number = + reinterpret_cast<ROCKSDB_NAMESPACE::RawIterator*>(native_handle)->getSequenceNumber(); + jlong result; + std::memcpy(&result, &sequence_number, sizeof(jlong)); + return result; +} + + +jint Java_org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReaderIterator_getType(JNIEnv *env, + jobject obj, + jlong native_handle) { + uint32_t type = reinterpret_cast<ROCKSDB_NAMESPACE::RawIterator*>(native_handle)->getType(); + return static_cast<jint>(type); +} + + +void Java_org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReaderIterator_closeInternal(JNIEnv *env, + jobject obj, + jlong native_handle) { + delete reinterpret_cast<ROCKSDB_NAMESPACE::RawIterator*>(native_handle); +} Review Comment: nit: please add an empty line as EOF here. ########## pom.xml: ########## @@ -297,21 +297,16 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs <aspectj.version>1.9.7</aspectj.version> <aspectj-plugin.version>1.14.0</aspectj-plugin.version> <restrict-imports.enforcer.version>2.4.0</restrict-imports.enforcer.version> - <bzip2.version>1.0.8</bzip2.version> - <zlib.version>1.2.13</zlib.version> Review Comment: nit: we need to remove these checks as well: https://github.com/apache/ozone/blob/master/hadoop-ozone/dev-support/checks/native.sh#L22-L32 ########## hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRawSSTFileReader.java: ########## @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.utils.db.managed; + +import org.apache.hadoop.hdds.utils.NativeLibraryLoader; +import org.apache.hadoop.hdds.utils.NativeLibraryNotLoadedException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.Closeable; +import java.util.function.Function; + +import static org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME; + +/** + * JNI for RocksDB RawSSTFileReader. + */ +public class ManagedRawSSTFileReader<T> implements Closeable { + + public static boolean loadLibrary() throws NativeLibraryNotLoadedException { + ManagedRocksObjectUtils.loadRocksDBLibrary(); + if (!NativeLibraryLoader.getInstance() + .loadLibrary(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) { + throw new NativeLibraryNotLoadedException( + ROCKS_TOOLS_NATIVE_LIBRARY_NAME); + } + return true; + } + + private final ManagedOptions options; + private final String fileName; + private final int readAheadSize; + private final long nativeHandle; Review Comment: Please add java doc for this and other non-intuitive variables. ########## hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRawSSTFileReader.java: ########## @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.utils.db.managed; + +import org.apache.hadoop.hdds.utils.NativeLibraryLoader; +import org.apache.hadoop.hdds.utils.NativeLibraryNotLoadedException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.Closeable; +import java.util.function.Function; + +import static org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME; + +/** + * JNI for RocksDB RawSSTFileReader. + */ +public class ManagedRawSSTFileReader<T> implements Closeable { + + public static boolean loadLibrary() throws NativeLibraryNotLoadedException { + ManagedRocksObjectUtils.loadRocksDBLibrary(); + if (!NativeLibraryLoader.getInstance() + .loadLibrary(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) { + throw new NativeLibraryNotLoadedException( + ROCKS_TOOLS_NATIVE_LIBRARY_NAME); + } + return true; + } + + private final ManagedOptions options; + private final String fileName; + private final int readAheadSize; + private final long nativeHandle; + private static final Logger LOG = + LoggerFactory.getLogger(ManagedRawSSTFileReader.class); + + public ManagedRawSSTFileReader(final ManagedOptions options, + final String fileName, + final int readAheadSize) { + this.options = options; + this.fileName = fileName; + this.readAheadSize = readAheadSize; + this.nativeHandle = this.newRawSSTFileReader(options.getNativeHandle(), + fileName, readAheadSize); + } + + public ManagedRawSSTFileReaderIterator<T> newIterator( + Function<ManagedRawSSTFileReaderIterator.KeyValue, T> transformerFunction, + ManagedSlice fromSlice, ManagedSlice toSlice) { + long fromNativeHandle = fromSlice == null ? 0 : fromSlice.getNativeHandle(); + long toNativeHandle = toSlice == null ? 0 : toSlice.getNativeHandle(); Review Comment: nit: should we use -1 to represent null? Then we don't need to `hasFrom` and `hasTo`. ########## hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRawSSTFileReader.java: ########## @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.utils.db.managed; + +import org.apache.hadoop.hdds.utils.NativeLibraryLoader; +import org.apache.hadoop.hdds.utils.NativeLibraryNotLoadedException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.Closeable; +import java.util.function.Function; + +import static org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME; + +/** + * JNI for RocksDB RawSSTFileReader. + */ +public class ManagedRawSSTFileReader<T> implements Closeable { Review Comment: Do we need this to be generic? Since we know key is always going to be String as long as it is SST file. Correct me if I'm wrong. ########## hadoop-hdds/rocks-native/src/main/native/ManagedRawSSTFileReader.cpp: ########## @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReader.h" +#include "rocksdb/options.h" +#include "rocksdb/raw_sst_file_reader.h" +#include "rocksdb/raw_iterator.h" +#include <string> +#include "cplusplus_to_java_convert.h" +#include <iostream> + +jlong Java_org_apache_hadoop_hdds_utils_db_managed_ManagedRawSSTFileReader_newRawSSTFileReader(JNIEnv *env, jobject obj, + jlong optionsHandle, Review Comment: nit: let's stick to either `snake_case` or `camelCase` for variable desecration in whole cpp code rather than using randomly `snake_case`, `camelCase` and `smallcase`. ########## hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRawSSTFileReaderIterator.java: ########## @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.utils.db.managed; + +import com.google.common.primitives.UnsignedLong; +import org.apache.hadoop.hdds.StringUtils; +import org.apache.hadoop.util.ClosableIterator; + +import java.util.Arrays; +import java.util.function.Function; + +/** + * Iterator for SSTFileReader which would read all entries including tombstones. + */ +public class ManagedRawSSTFileReaderIterator<T> Review Comment: nit: since now we allow 120 chars. ```suggestion public class ManagedRawSSTFileReaderIterator<T> implements ClosableIterator<T> { ``` Other places as well to improve readability where we had to trim line in between because 80 chars limit. ########## hadoop-hdds/rocks-native/src/main/patches/rocks-native.patch: ########## @@ -16,592 +16,557 @@ * limitations under the License. */ -diff --git a/include/rocksdb/sst_dump_tool.h b/include/rocksdb/sst_dump_tool.h -index 9261ba47d..1e62b88a3 100644 ---- a/include/rocksdb/sst_dump_tool.h -+++ b/include/rocksdb/sst_dump_tool.h -@@ -11,7 +11,8 @@ namespace ROCKSDB_NAMESPACE { - - class SSTDumpTool { - public: -- int Run(int argc, char const* const* argv, Options options = Options()); -+ int Run(int argc, char const* const* argv, Options options = Options(), -+ FILE* out = stdout, FILE* err = stderr); - }; - - } // namespace ROCKSDB_NAMESPACE -diff --git a/table/sst_file_dumper.cc b/table/sst_file_dumper.cc -index eefbaaeee..734a2f0dd 100644 ---- a/table/sst_file_dumper.cc -+++ b/table/sst_file_dumper.cc -@@ -45,7 +45,7 @@ SstFileDumper::SstFileDumper(const Options& options, - Temperature file_temp, size_t readahead_size, - bool verify_checksum, bool output_hex, - bool decode_blob_index, const EnvOptions& soptions, -- bool silent) -+ bool silent, FILE* out, FILE* err) - : file_name_(file_path), - read_num_(0), - file_temp_(file_temp), -@@ -57,10 +57,13 @@ SstFileDumper::SstFileDumper(const Options& options, - ioptions_(options_), - moptions_(ColumnFamilyOptions(options_)), - read_options_(verify_checksum, false), -- internal_comparator_(BytewiseComparator()) { -+ internal_comparator_(BytewiseComparator()), -+ out_(out), -+ err_(err) -+ { - read_options_.readahead_size = readahead_size; - if (!silent_) { -- fprintf(stdout, "Process %s\n", file_path.c_str()); -+ fprintf(out_, "Process %s\n", file_path.c_str()); - } - init_result_ = GetTableReader(file_name_); - } -@@ -253,17 +256,17 @@ Status SstFileDumper::ShowAllCompressionSizes( - int32_t compress_level_from, int32_t compress_level_to, - uint32_t max_dict_bytes, uint32_t zstd_max_train_bytes, - uint64_t max_dict_buffer_bytes, bool use_zstd_dict_trainer) { -- fprintf(stdout, "Block Size: %" ROCKSDB_PRIszt "\n", block_size); -+ fprintf(out_, "Block Size: %" ROCKSDB_PRIszt "\n", block_size); - for (auto& i : compression_types) { - if (CompressionTypeSupported(i.first)) { -- fprintf(stdout, "Compression: %-24s\n", i.second); -+ fprintf(out_, "Compression: %-24s\n", i.second); - CompressionOptions compress_opt; - compress_opt.max_dict_bytes = max_dict_bytes; - compress_opt.zstd_max_train_bytes = zstd_max_train_bytes; - compress_opt.max_dict_buffer_bytes = max_dict_buffer_bytes; - compress_opt.use_zstd_dict_trainer = use_zstd_dict_trainer; - for (int32_t j = compress_level_from; j <= compress_level_to; j++) { -- fprintf(stdout, "Compression level: %d", j); -+ fprintf(out_, "Compression level: %d", j); - compress_opt.level = j; - Status s = ShowCompressionSize(block_size, i.first, compress_opt); - if (!s.ok()) { -@@ -271,7 +274,7 @@ Status SstFileDumper::ShowAllCompressionSizes( - } - } - } else { -- fprintf(stdout, "Unsupported compression type: %s.\n", i.second); -+ fprintf(err_, "Unsupported compression type: %s.\n", i.second); - } - } - return Status::OK(); -@@ -307,9 +310,9 @@ Status SstFileDumper::ShowCompressionSize( - } - - std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now(); -- fprintf(stdout, " Size: %10" PRIu64, file_size); -- fprintf(stdout, " Blocks: %6" PRIu64, num_data_blocks); -- fprintf(stdout, " Time Taken: %10s microsecs", -+ fprintf(out_, " Size: %10" PRIu64, file_size); -+ fprintf(out_, " Blocks: %6" PRIu64, num_data_blocks); -+ fprintf(out_, " Time Taken: %10s microsecs", - std::to_string( - std::chrono::duration_cast<std::chrono::microseconds>(end - start) - .count()) -@@ -342,11 +345,11 @@ Status SstFileDumper::ShowCompressionSize( - : ((static_cast<double>(not_compressed_blocks) / - static_cast<double>(num_data_blocks)) * - 100.0); -- fprintf(stdout, " Compressed: %6" PRIu64 " (%5.1f%%)", compressed_blocks, -+ fprintf(out_, " Compressed: %6" PRIu64 " (%5.1f%%)", compressed_blocks, - compressed_pcnt); -- fprintf(stdout, " Not compressed (ratio): %6" PRIu64 " (%5.1f%%)", -+ fprintf(out_, " Not compressed (ratio): %6" PRIu64 " (%5.1f%%)", - ratio_not_compressed_blocks, ratio_not_compressed_pcnt); -- fprintf(stdout, " Not compressed (abort): %6" PRIu64 " (%5.1f%%)\n", -+ fprintf(out_, " Not compressed (abort): %6" PRIu64 " (%5.1f%%)\n", - not_compressed_blocks, not_compressed_pcnt); - return Status::OK(); - } -@@ -362,7 +365,7 @@ Status SstFileDumper::ReadTableProperties(uint64_t table_magic_number, - /* memory_allocator= */ nullptr, prefetch_buffer); - if (!s.ok()) { - if (!silent_) { -- fprintf(stdout, "Not able to read table properties\n"); -+ fprintf(err_, "Not able to read table properties\n"); - } - } - return s; -@@ -410,7 +413,7 @@ Status SstFileDumper::SetTableOptionsByMagicNumber( - - options_.table_factory.reset(NewPlainTableFactory(plain_table_options)); - if (!silent_) { -- fprintf(stdout, "Sst file format: plain table\n"); -+ fprintf(out_, "Sst file format: plain table\n"); - } - } else { - char error_msg_buffer[80]; -@@ -427,15 +430,56 @@ Status SstFileDumper::SetOldTableOptions() { - assert(table_properties_ == nullptr); - options_.table_factory = std::make_shared<BlockBasedTableFactory>(); - if (!silent_) { -- fprintf(stdout, "Sst file format: block-based(old version)\n"); -+ fprintf(out_, "Sst file format: block-based(old version)\n"); - } - - return Status::OK(); - } - -+void write(int value, FILE* file) { -+ char b[4]; -+ b[3] = value & 0x000000ff; -+ b[2] = (value & 0x0000ff00) >> 8; -+ b[1] = (value & 0x00ff0000) >> 16; -+ b[0] = (value & 0xff000000) >> 24; -+ std::fwrite(b, 4, 1, file); +diff --git a/include/rocksdb/raw_iterator.h b/include/rocksdb/raw_iterator.h +new file mode 100644 +index 000000000..ec3c05d6d +--- /dev/null ++++ b/include/rocksdb/raw_iterator.h +@@ -0,0 +1,25 @@ ++// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. ++// This source code is licensed under both the GPLv2 (found in the ++// COPYING file in the root directory) and Apache 2.0 License ++// (found in the LICENSE.Apache file in the root directory). ++#pragma once ++#ifndef ROCKSDB_LITE ++ ++ ++#include "rocksdb/advanced_options.h" ++namespace ROCKSDB_NAMESPACE { ++ ++class RawIterator { ++ public: ++ virtual ~RawIterator() {} ++ virtual bool has_next() const = 0; ++ virtual Slice getKey() const = 0; ++ virtual Slice getValue() const = 0; ++ virtual uint64_t getSequenceNumber() const = 0; ++ virtual uint32_t getType() const = 0; ++ virtual void next() = 0; ++}; ++ ++} // namespace ROCKSDB_NAMESPACE ++ ++#endif // ROCKSDB_LITE +diff --git a/include/rocksdb/raw_sst_file_reader.h b/include/rocksdb/raw_sst_file_reader.h +new file mode 100644 +index 000000000..266a0c1c8 +--- /dev/null ++++ b/include/rocksdb/raw_sst_file_reader.h +@@ -0,0 +1,62 @@ ++// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. Review Comment: nit: alignment is off here and other places as well. ```suggestion +// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. ``` ########## hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/SstFileSetReader.java: ########## @@ -223,6 +217,38 @@ public String next() { } } + private static class ManagedRawSstFileIterator implements + ClosableIterator<String> { + private ManagedRawSSTFileReader<String> fileReader; + private ManagedRawSSTFileReaderIterator<String> fileReaderIterator; + + ManagedRawSstFileIterator(String path, ManagedOptions options, + ManagedSlice lowerBound, ManagedSlice upperBound, + Function<ManagedRawSSTFileReaderIterator.KeyValue, + String> keyValueFunction) { + this.fileReader = new ManagedRawSSTFileReader<>(options, path, + 2 * 1024 * 1024); Review Comment: nit: create a constant for it with unit in its name. ########## hadoop-hdds/rocks-native/src/main/patches/rocks-native.patch: ########## @@ -16,592 +16,557 @@ * limitations under the License. */ -diff --git a/include/rocksdb/sst_dump_tool.h b/include/rocksdb/sst_dump_tool.h -index 9261ba47d..1e62b88a3 100644 ---- a/include/rocksdb/sst_dump_tool.h -+++ b/include/rocksdb/sst_dump_tool.h -@@ -11,7 +11,8 @@ namespace ROCKSDB_NAMESPACE { - - class SSTDumpTool { - public: -- int Run(int argc, char const* const* argv, Options options = Options()); -+ int Run(int argc, char const* const* argv, Options options = Options(), -+ FILE* out = stdout, FILE* err = stderr); - }; - - } // namespace ROCKSDB_NAMESPACE -diff --git a/table/sst_file_dumper.cc b/table/sst_file_dumper.cc -index eefbaaeee..734a2f0dd 100644 ---- a/table/sst_file_dumper.cc -+++ b/table/sst_file_dumper.cc -@@ -45,7 +45,7 @@ SstFileDumper::SstFileDumper(const Options& options, - Temperature file_temp, size_t readahead_size, - bool verify_checksum, bool output_hex, - bool decode_blob_index, const EnvOptions& soptions, -- bool silent) -+ bool silent, FILE* out, FILE* err) - : file_name_(file_path), - read_num_(0), - file_temp_(file_temp), -@@ -57,10 +57,13 @@ SstFileDumper::SstFileDumper(const Options& options, - ioptions_(options_), - moptions_(ColumnFamilyOptions(options_)), - read_options_(verify_checksum, false), -- internal_comparator_(BytewiseComparator()) { -+ internal_comparator_(BytewiseComparator()), -+ out_(out), -+ err_(err) -+ { - read_options_.readahead_size = readahead_size; - if (!silent_) { -- fprintf(stdout, "Process %s\n", file_path.c_str()); -+ fprintf(out_, "Process %s\n", file_path.c_str()); - } - init_result_ = GetTableReader(file_name_); - } -@@ -253,17 +256,17 @@ Status SstFileDumper::ShowAllCompressionSizes( - int32_t compress_level_from, int32_t compress_level_to, - uint32_t max_dict_bytes, uint32_t zstd_max_train_bytes, - uint64_t max_dict_buffer_bytes, bool use_zstd_dict_trainer) { -- fprintf(stdout, "Block Size: %" ROCKSDB_PRIszt "\n", block_size); -+ fprintf(out_, "Block Size: %" ROCKSDB_PRIszt "\n", block_size); - for (auto& i : compression_types) { - if (CompressionTypeSupported(i.first)) { -- fprintf(stdout, "Compression: %-24s\n", i.second); -+ fprintf(out_, "Compression: %-24s\n", i.second); - CompressionOptions compress_opt; - compress_opt.max_dict_bytes = max_dict_bytes; - compress_opt.zstd_max_train_bytes = zstd_max_train_bytes; - compress_opt.max_dict_buffer_bytes = max_dict_buffer_bytes; - compress_opt.use_zstd_dict_trainer = use_zstd_dict_trainer; - for (int32_t j = compress_level_from; j <= compress_level_to; j++) { -- fprintf(stdout, "Compression level: %d", j); -+ fprintf(out_, "Compression level: %d", j); - compress_opt.level = j; - Status s = ShowCompressionSize(block_size, i.first, compress_opt); - if (!s.ok()) { -@@ -271,7 +274,7 @@ Status SstFileDumper::ShowAllCompressionSizes( - } - } - } else { -- fprintf(stdout, "Unsupported compression type: %s.\n", i.second); -+ fprintf(err_, "Unsupported compression type: %s.\n", i.second); - } - } - return Status::OK(); -@@ -307,9 +310,9 @@ Status SstFileDumper::ShowCompressionSize( - } - - std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now(); -- fprintf(stdout, " Size: %10" PRIu64, file_size); -- fprintf(stdout, " Blocks: %6" PRIu64, num_data_blocks); -- fprintf(stdout, " Time Taken: %10s microsecs", -+ fprintf(out_, " Size: %10" PRIu64, file_size); -+ fprintf(out_, " Blocks: %6" PRIu64, num_data_blocks); -+ fprintf(out_, " Time Taken: %10s microsecs", - std::to_string( - std::chrono::duration_cast<std::chrono::microseconds>(end - start) - .count()) -@@ -342,11 +345,11 @@ Status SstFileDumper::ShowCompressionSize( - : ((static_cast<double>(not_compressed_blocks) / - static_cast<double>(num_data_blocks)) * - 100.0); -- fprintf(stdout, " Compressed: %6" PRIu64 " (%5.1f%%)", compressed_blocks, -+ fprintf(out_, " Compressed: %6" PRIu64 " (%5.1f%%)", compressed_blocks, - compressed_pcnt); -- fprintf(stdout, " Not compressed (ratio): %6" PRIu64 " (%5.1f%%)", -+ fprintf(out_, " Not compressed (ratio): %6" PRIu64 " (%5.1f%%)", - ratio_not_compressed_blocks, ratio_not_compressed_pcnt); -- fprintf(stdout, " Not compressed (abort): %6" PRIu64 " (%5.1f%%)\n", -+ fprintf(out_, " Not compressed (abort): %6" PRIu64 " (%5.1f%%)\n", - not_compressed_blocks, not_compressed_pcnt); - return Status::OK(); - } -@@ -362,7 +365,7 @@ Status SstFileDumper::ReadTableProperties(uint64_t table_magic_number, - /* memory_allocator= */ nullptr, prefetch_buffer); - if (!s.ok()) { - if (!silent_) { -- fprintf(stdout, "Not able to read table properties\n"); -+ fprintf(err_, "Not able to read table properties\n"); - } - } - return s; -@@ -410,7 +413,7 @@ Status SstFileDumper::SetTableOptionsByMagicNumber( - - options_.table_factory.reset(NewPlainTableFactory(plain_table_options)); - if (!silent_) { -- fprintf(stdout, "Sst file format: plain table\n"); -+ fprintf(out_, "Sst file format: plain table\n"); - } - } else { - char error_msg_buffer[80]; -@@ -427,15 +430,56 @@ Status SstFileDumper::SetOldTableOptions() { - assert(table_properties_ == nullptr); - options_.table_factory = std::make_shared<BlockBasedTableFactory>(); - if (!silent_) { -- fprintf(stdout, "Sst file format: block-based(old version)\n"); -+ fprintf(out_, "Sst file format: block-based(old version)\n"); - } - - return Status::OK(); - } - -+void write(int value, FILE* file) { -+ char b[4]; -+ b[3] = value & 0x000000ff; -+ b[2] = (value & 0x0000ff00) >> 8; -+ b[1] = (value & 0x00ff0000) >> 16; -+ b[0] = (value & 0xff000000) >> 24; -+ std::fwrite(b, 4, 1, file); +diff --git a/include/rocksdb/raw_iterator.h b/include/rocksdb/raw_iterator.h +new file mode 100644 +index 000000000..ec3c05d6d +--- /dev/null ++++ b/include/rocksdb/raw_iterator.h +@@ -0,0 +1,25 @@ ++// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. ++// This source code is licensed under both the GPLv2 (found in the ++// COPYING file in the root directory) and Apache 2.0 License ++// (found in the LICENSE.Apache file in the root directory). ++#pragma once ++#ifndef ROCKSDB_LITE ++ ++ ++#include "rocksdb/advanced_options.h" ++namespace ROCKSDB_NAMESPACE { ++ ++class RawIterator { ++ public: ++ virtual ~RawIterator() {} ++ virtual bool has_next() const = 0; ++ virtual Slice getKey() const = 0; ++ virtual Slice getValue() const = 0; ++ virtual uint64_t getSequenceNumber() const = 0; ++ virtual uint32_t getType() const = 0; ++ virtual void next() = 0; ++}; ++ ++} // namespace ROCKSDB_NAMESPACE ++ ++#endif // ROCKSDB_LITE +diff --git a/include/rocksdb/raw_sst_file_reader.h b/include/rocksdb/raw_sst_file_reader.h +new file mode 100644 +index 000000000..266a0c1c8 +--- /dev/null ++++ b/include/rocksdb/raw_sst_file_reader.h +@@ -0,0 +1,62 @@ ++// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. ++// This source code is licensed under both the GPLv2 (found in the ++// COPYING file in the root directory) and Apache 2.0 License ++// (found in the LICENSE.Apache file in the root directory). ++#pragma once ++#ifndef ROCKSDB_LITE ++ ++#include <memory> ++#include <string> ++ ++#include "rocksdb/raw_iterator.h" ++#include "rocksdb/advanced_options.h" ++#include "rocksdb/options.h" ++ ++ ++ ++namespace ROCKSDB_NAMESPACE { ++ ++class RawSstFileReader { ++ public: ++ ++ RawSstFileReader(const Options& options, const std::string& file_name, ++ size_t readahead_size, bool verify_checksum, ++ bool silent = false); ++ ~RawSstFileReader(); ++ ++ RawIterator* newIterator(bool has_from, Slice* from, ++ bool has_to, Slice *to); ++ Status getStatus() { return init_result_; } ++ ++ private: ++ // Get the TableReader implementation for the sst file ++ Status GetTableReader(const std::string& file_path); ++ Status ReadTableProperties(uint64_t table_magic_number, ++ uint64_t file_size); ++ ++ Status SetTableOptionsByMagicNumber(uint64_t table_magic_number); ++ Status SetOldTableOptions(); ++ ++ // Helper function to call the factory with settings specific to the ++ // factory implementation ++ Status NewTableReader(uint64_t file_size); ++ ++ std::string file_name_; ++ Temperature file_temp_; ++ ++ // less verbose in stdout/stderr ++ bool silent_; ++ ++ // options_ and internal_comparator_ will also be used in ++ // ReadSequential internally (specifically, seek-related operations) ++ Options options_; ++ ++ Status init_result_; ++ ++ struct Rep; ++ std::unique_ptr<Rep> rep_; ++}; ++ ++} // namespace ROCKSDB_NAMESPACE ++ ++#endif // ROCKSDB_LITE +diff --git a/src.mk b/src.mk +index b94bc43ca..95bfff274 100644 +--- a/src.mk ++++ b/src.mk +@@ -343,6 +343,8 @@ TOOL_LIB_SOURCES = \ + tools/ldb_tool.cc \ + tools/sst_dump_tool.cc \ Review Comment: Should this be removed? ########## hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestManagedRawSSTFileReaderIterator.java: ########## @@ -0,0 +1,176 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.utils.db.managed; + +import org.apache.commons.lang3.tuple.Pair; +import org.apache.hadoop.hdds.StringUtils; +import org.apache.hadoop.hdds.utils.TestUtils; +import org.apache.ozone.test.tag.Native; +import org.junit.jupiter.api.Named; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.TreeMap; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import static org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Test for ManagedRawSSTFileReaderIterator. + */ +class TestManagedRawSSTFileReaderIterator { + + @TempDir + private Path tempDir; + + private File createSSTFileWithKeys( + TreeMap<Pair<String, Integer>, String> keys) throws Exception { + File file = Files.createFile(tempDir.resolve("tmp_sst_file.sst")).toFile(); + try (ManagedEnvOptions envOptions = new ManagedEnvOptions(); + ManagedOptions managedOptions = new ManagedOptions(); + ManagedSstFileWriter sstFileWriter = new ManagedSstFileWriter( + envOptions, managedOptions)) { + sstFileWriter.open(file.getAbsolutePath()); + for (Map.Entry<Pair<String, Integer>, String> entry : keys.entrySet()) { + if (entry.getKey().getValue() == 0) { + sstFileWriter.delete(entry.getKey().getKey() + .getBytes(StandardCharsets.UTF_8)); + } else { + sstFileWriter.put(entry.getKey().getKey() + .getBytes(StandardCharsets.UTF_8), + entry.getValue().getBytes(StandardCharsets.UTF_8)); + } + } + sstFileWriter.finish(); + } + return file; + } + + private static Stream<? extends Arguments> keyValueFormatArgs() { + return Stream.of( + Arguments.of( + Named.of("Key starting with a single quote", + "'key%1$d=>"), + Named.of("Value starting with a number ending with a" + + " single quote", "%1$dvalue'") + ), + Arguments.of( + Named.of("Key ending with a number", "key%1$d"), + Named.of("Value starting & ending with a number", "%1$dvalue%1$d") + ), + Arguments.of( + Named.of("Key starting with a single quote & ending" + + " with a number", "'key%1$d"), + Named.of("Value starting & ending with a number " + + "& elosed within quotes", "%1$d'value%1$d'")), + Arguments.of( + Named.of("Key starting with a single quote & ending" + + " with a number", "'key%1$d"), + Named.of("Value starting & ending with a number " + + "& elosed within quotes", "%1$d'value%1$d'") + ), + Arguments.of( + Named.of("Key ending with a number", "key%1$d"), + Named.of("Value starting & ending with a number " + + "& containing null character & new line character", + "%1$dvalue\n\0%1$d") + ), + Arguments.of( + Named.of("Key ending with a number & containing" + + " a null character", "key\0%1$d"), + Named.of("Value starting & ending with a number " + + "& elosed within quotes", "%1$dvalue\r%1$d") + ) + ); + } + + @Native(ROCKS_TOOLS_NATIVE_LIBRARY_NAME) Review Comment: nit: this could be moved at class level in case more test cases are added. ########## hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java: ########## @@ -282,35 +278,17 @@ public PersistentMap<String, SnapshotDiffJob> getSnapDiffJobTable() { return snapDiffJobTable; } - private Optional<ManagedSSTDumpTool> initSSTDumpTool( + private boolean initSSTDumpTool( final OzoneConfiguration conf) { if (conf.getBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_LOAD_NATIVE_LIB, OMConfigKeys.OZONE_OM_SNAPSHOT_LOAD_NATIVE_LIB_DEFAULT)) { try { - int threadPoolSize = conf.getInt( - OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE, - OMConfigKeys - .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT); - int bufferSize = (int) conf.getStorageSize( - OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE, - OMConfigKeys - .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE_DEFAULT, - StorageUnit.BYTES); - this.sstDumpToolExecService = Optional.of(new ThreadPoolExecutor(0, - threadPoolSize, 60, TimeUnit.SECONDS, - new SynchronousQueue<>(), new ThreadFactoryBuilder() - .setNameFormat(ozoneManager.getThreadNamePrefix() + - "snapshot-diff-manager-sst-dump-tool-TID-%d") - .build(), - new ThreadPoolExecutor.DiscardPolicy())); - return Optional.of(new ManagedSSTDumpTool(sstDumpToolExecService.get(), - bufferSize)); + return ManagedRawSSTFileReader.loadLibrary(); } catch (NativeLibraryNotLoadedException e) { - this.sstDumpToolExecService.ifPresent(exec -> - closeExecutorService(exec, "SstDumpToolExecutor")); + return false; Review Comment: It would be good to log the exception as warn or something in case loading fails. Otherwise one would never know that optimize diff is not working even when native lib is enabled. ########## hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRawSSTFileReaderIterator.java: ########## @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.utils.db.managed; + +import com.google.common.primitives.UnsignedLong; +import org.apache.hadoop.hdds.StringUtils; +import org.apache.hadoop.util.ClosableIterator; + +import java.util.Arrays; +import java.util.function.Function; + +/** + * Iterator for SSTFileReader which would read all entries including tombstones. + */ +public class ManagedRawSSTFileReaderIterator<T> + implements ClosableIterator<T> { + private final long nativeHandle; Review Comment: Can you please add java doc for this? ########## hadoop-hdds/rocks-native/src/main/patches/rocks-native.patch: ########## @@ -16,592 +16,557 @@ * limitations under the License. */ -diff --git a/include/rocksdb/sst_dump_tool.h b/include/rocksdb/sst_dump_tool.h -index 9261ba47d..1e62b88a3 100644 ---- a/include/rocksdb/sst_dump_tool.h -+++ b/include/rocksdb/sst_dump_tool.h -@@ -11,7 +11,8 @@ namespace ROCKSDB_NAMESPACE { - - class SSTDumpTool { - public: -- int Run(int argc, char const* const* argv, Options options = Options()); -+ int Run(int argc, char const* const* argv, Options options = Options(), -+ FILE* out = stdout, FILE* err = stderr); - }; - - } // namespace ROCKSDB_NAMESPACE -diff --git a/table/sst_file_dumper.cc b/table/sst_file_dumper.cc -index eefbaaeee..734a2f0dd 100644 ---- a/table/sst_file_dumper.cc -+++ b/table/sst_file_dumper.cc -@@ -45,7 +45,7 @@ SstFileDumper::SstFileDumper(const Options& options, - Temperature file_temp, size_t readahead_size, - bool verify_checksum, bool output_hex, - bool decode_blob_index, const EnvOptions& soptions, -- bool silent) -+ bool silent, FILE* out, FILE* err) - : file_name_(file_path), - read_num_(0), - file_temp_(file_temp), -@@ -57,10 +57,13 @@ SstFileDumper::SstFileDumper(const Options& options, - ioptions_(options_), - moptions_(ColumnFamilyOptions(options_)), - read_options_(verify_checksum, false), -- internal_comparator_(BytewiseComparator()) { -+ internal_comparator_(BytewiseComparator()), -+ out_(out), -+ err_(err) -+ { - read_options_.readahead_size = readahead_size; - if (!silent_) { -- fprintf(stdout, "Process %s\n", file_path.c_str()); -+ fprintf(out_, "Process %s\n", file_path.c_str()); - } - init_result_ = GetTableReader(file_name_); - } -@@ -253,17 +256,17 @@ Status SstFileDumper::ShowAllCompressionSizes( - int32_t compress_level_from, int32_t compress_level_to, - uint32_t max_dict_bytes, uint32_t zstd_max_train_bytes, - uint64_t max_dict_buffer_bytes, bool use_zstd_dict_trainer) { -- fprintf(stdout, "Block Size: %" ROCKSDB_PRIszt "\n", block_size); -+ fprintf(out_, "Block Size: %" ROCKSDB_PRIszt "\n", block_size); - for (auto& i : compression_types) { - if (CompressionTypeSupported(i.first)) { -- fprintf(stdout, "Compression: %-24s\n", i.second); -+ fprintf(out_, "Compression: %-24s\n", i.second); - CompressionOptions compress_opt; - compress_opt.max_dict_bytes = max_dict_bytes; - compress_opt.zstd_max_train_bytes = zstd_max_train_bytes; - compress_opt.max_dict_buffer_bytes = max_dict_buffer_bytes; - compress_opt.use_zstd_dict_trainer = use_zstd_dict_trainer; - for (int32_t j = compress_level_from; j <= compress_level_to; j++) { -- fprintf(stdout, "Compression level: %d", j); -+ fprintf(out_, "Compression level: %d", j); - compress_opt.level = j; - Status s = ShowCompressionSize(block_size, i.first, compress_opt); - if (!s.ok()) { -@@ -271,7 +274,7 @@ Status SstFileDumper::ShowAllCompressionSizes( - } - } - } else { -- fprintf(stdout, "Unsupported compression type: %s.\n", i.second); -+ fprintf(err_, "Unsupported compression type: %s.\n", i.second); - } - } - return Status::OK(); -@@ -307,9 +310,9 @@ Status SstFileDumper::ShowCompressionSize( - } - - std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now(); -- fprintf(stdout, " Size: %10" PRIu64, file_size); -- fprintf(stdout, " Blocks: %6" PRIu64, num_data_blocks); -- fprintf(stdout, " Time Taken: %10s microsecs", -+ fprintf(out_, " Size: %10" PRIu64, file_size); -+ fprintf(out_, " Blocks: %6" PRIu64, num_data_blocks); -+ fprintf(out_, " Time Taken: %10s microsecs", - std::to_string( - std::chrono::duration_cast<std::chrono::microseconds>(end - start) - .count()) -@@ -342,11 +345,11 @@ Status SstFileDumper::ShowCompressionSize( - : ((static_cast<double>(not_compressed_blocks) / - static_cast<double>(num_data_blocks)) * - 100.0); -- fprintf(stdout, " Compressed: %6" PRIu64 " (%5.1f%%)", compressed_blocks, -+ fprintf(out_, " Compressed: %6" PRIu64 " (%5.1f%%)", compressed_blocks, - compressed_pcnt); -- fprintf(stdout, " Not compressed (ratio): %6" PRIu64 " (%5.1f%%)", -+ fprintf(out_, " Not compressed (ratio): %6" PRIu64 " (%5.1f%%)", - ratio_not_compressed_blocks, ratio_not_compressed_pcnt); -- fprintf(stdout, " Not compressed (abort): %6" PRIu64 " (%5.1f%%)\n", -+ fprintf(out_, " Not compressed (abort): %6" PRIu64 " (%5.1f%%)\n", - not_compressed_blocks, not_compressed_pcnt); - return Status::OK(); - } -@@ -362,7 +365,7 @@ Status SstFileDumper::ReadTableProperties(uint64_t table_magic_number, - /* memory_allocator= */ nullptr, prefetch_buffer); - if (!s.ok()) { - if (!silent_) { -- fprintf(stdout, "Not able to read table properties\n"); -+ fprintf(err_, "Not able to read table properties\n"); - } - } - return s; -@@ -410,7 +413,7 @@ Status SstFileDumper::SetTableOptionsByMagicNumber( - - options_.table_factory.reset(NewPlainTableFactory(plain_table_options)); - if (!silent_) { -- fprintf(stdout, "Sst file format: plain table\n"); -+ fprintf(out_, "Sst file format: plain table\n"); - } - } else { - char error_msg_buffer[80]; -@@ -427,15 +430,56 @@ Status SstFileDumper::SetOldTableOptions() { - assert(table_properties_ == nullptr); - options_.table_factory = std::make_shared<BlockBasedTableFactory>(); - if (!silent_) { -- fprintf(stdout, "Sst file format: block-based(old version)\n"); -+ fprintf(out_, "Sst file format: block-based(old version)\n"); - } - - return Status::OK(); - } - -+void write(int value, FILE* file) { -+ char b[4]; -+ b[3] = value & 0x000000ff; -+ b[2] = (value & 0x0000ff00) >> 8; -+ b[1] = (value & 0x00ff0000) >> 16; -+ b[0] = (value & 0xff000000) >> 24; -+ std::fwrite(b, 4, 1, file); +diff --git a/include/rocksdb/raw_iterator.h b/include/rocksdb/raw_iterator.h +new file mode 100644 +index 000000000..ec3c05d6d +--- /dev/null ++++ b/include/rocksdb/raw_iterator.h +@@ -0,0 +1,25 @@ ++// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. ++// This source code is licensed under both the GPLv2 (found in the ++// COPYING file in the root directory) and Apache 2.0 License ++// (found in the LICENSE.Apache file in the root directory). ++#pragma once ++#ifndef ROCKSDB_LITE ++ ++ ++#include "rocksdb/advanced_options.h" ++namespace ROCKSDB_NAMESPACE { ++ ++class RawIterator { ++ public: ++ virtual ~RawIterator() {} ++ virtual bool has_next() const = 0; ++ virtual Slice getKey() const = 0; ++ virtual Slice getValue() const = 0; ++ virtual uint64_t getSequenceNumber() const = 0; ++ virtual uint32_t getType() const = 0; ++ virtual void next() = 0; ++}; ++ ++} // namespace ROCKSDB_NAMESPACE ++ ++#endif // ROCKSDB_LITE +diff --git a/include/rocksdb/raw_sst_file_reader.h b/include/rocksdb/raw_sst_file_reader.h +new file mode 100644 +index 000000000..266a0c1c8 +--- /dev/null ++++ b/include/rocksdb/raw_sst_file_reader.h +@@ -0,0 +1,62 @@ ++// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. ++// This source code is licensed under both the GPLv2 (found in the ++// COPYING file in the root directory) and Apache 2.0 License ++// (found in the LICENSE.Apache file in the root directory). ++#pragma once ++#ifndef ROCKSDB_LITE ++ ++#include <memory> ++#include <string> ++ ++#include "rocksdb/raw_iterator.h" ++#include "rocksdb/advanced_options.h" ++#include "rocksdb/options.h" ++ ++ ++ ++namespace ROCKSDB_NAMESPACE { ++ ++class RawSstFileReader { ++ public: ++ ++ RawSstFileReader(const Options& options, const std::string& file_name, ++ size_t readahead_size, bool verify_checksum, ++ bool silent = false); ++ ~RawSstFileReader(); ++ ++ RawIterator* newIterator(bool has_from, Slice* from, ++ bool has_to, Slice *to); ++ Status getStatus() { return init_result_; } ++ ++ private: ++ // Get the TableReader implementation for the sst file ++ Status GetTableReader(const std::string& file_path); ++ Status ReadTableProperties(uint64_t table_magic_number, ++ uint64_t file_size); ++ ++ Status SetTableOptionsByMagicNumber(uint64_t table_magic_number); ++ Status SetOldTableOptions(); ++ ++ // Helper function to call the factory with settings specific to the ++ // factory implementation ++ Status NewTableReader(uint64_t file_size); ++ ++ std::string file_name_; ++ Temperature file_temp_; ++ ++ // less verbose in stdout/stderr ++ bool silent_; ++ ++ // options_ and internal_comparator_ will also be used in ++ // ReadSequential internally (specifically, seek-related operations) ++ Options options_; ++ ++ Status init_result_; ++ ++ struct Rep; ++ std::unique_ptr<Rep> rep_; ++}; ++ ++} // namespace ROCKSDB_NAMESPACE ++ ++#endif // ROCKSDB_LITE +diff --git a/src.mk b/src.mk +index b94bc43ca..95bfff274 100644 +--- a/src.mk ++++ b/src.mk +@@ -343,6 +343,8 @@ TOOL_LIB_SOURCES = \ + tools/ldb_tool.cc \ + tools/sst_dump_tool.cc \ + utilities/blob_db/blob_dump_tool.cc \ ++ tools/raw_sst_file_reader.cc \ ++ tools/raw_sst_file_reader_iterator.cc \ + + ANALYZER_LIB_SOURCES = \ + tools/block_cache_analyzer/block_cache_trace_analyzer.cc \ +diff --git a/tools/raw_sst_file_reader.cc b/tools/raw_sst_file_reader.cc +new file mode 100644 +index 000000000..1693bd1e6 +--- /dev/null ++++ b/tools/raw_sst_file_reader.cc +@@ -0,0 +1,285 @@ ++// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. ++// This source code is licensed under both the GPLv2 (found in the ++// COPYING file in the root directory) and Apache 2.0 License ++// (found in the LICENSE.Apache file in the root directory). ++// ++#ifndef ROCKSDB_LITE ++ ++#include "rocksdb/raw_sst_file_reader.h" ++ ++#include <chrono> ++#include <cinttypes> ++#include <iostream> ++#include <map> ++#include <memory> ++#include <sstream> ++#include <vector> ++ ++#include "db/blob/blob_index.h" ++#include "db/memtable.h" ++#include "db/write_batch_internal.h" ++#include "options/cf_options.h" ++#include "port/port.h" ++#include "rocksdb/db.h" ++#include "rocksdb/env.h" ++#include "rocksdb/iterator.h" ++#include "rocksdb/slice_transform.h" ++#include "rocksdb/status.h" ++#include "rocksdb/table_properties.h" ++#include "rocksdb/utilities/ldb_cmd.h" ++#include "table/block_based/block.h" ++#include "table/block_based/block_based_table_builder.h" ++#include "table/block_based/block_based_table_factory.h" ++#include "table/block_based/block_builder.h" ++#include "table/format.h" ++#include "table/meta_blocks.h" ++#include "table/plain/plain_table_factory.h" ++#include "table/table_reader.h" ++#include "tools/raw_sst_file_reader_iterator.h" ++#include "util/compression.h" ++#include "util/random.h" ++#include "db/dbformat.h" ++#include "file/writable_file_writer.h" ++#include "options/cf_options.h" ++ ++namespace ROCKSDB_NAMESPACE { ++ ++struct RawSstFileReader::Rep { ++ Options options; ++ EnvOptions soptions_; ++ ReadOptions read_options_; ++ ImmutableOptions ioptions_; ++ MutableCFOptions moptions_; ++ InternalKeyComparator internal_comparator_; ++ std::unique_ptr<TableProperties> table_properties_; ++ std::unique_ptr<TableReader> table_reader_; ++ std::unique_ptr<RandomAccessFileReader> file_; ++ ++ Rep(const Options& opts, bool verify_checksum, size_t readahead_size) Review Comment: ```suggestion + Rep(const Options& opts, bool verify_checksum, size_t read_ahead_size) ``` ########## hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/SstFileSetReader.java: ########## @@ -223,6 +217,38 @@ public String next() { } } + private static class ManagedRawSstFileIterator implements + ClosableIterator<String> { + private ManagedRawSSTFileReader<String> fileReader; + private ManagedRawSSTFileReaderIterator<String> fileReaderIterator; Review Comment: Make them final. -- 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]
