This is an automated email from the ASF dual-hosted git repository. lhotari pushed a commit to branch branch-4.15 in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 841d598a5ea1b316e89a91229d945a99bb9e430a Author: Hang Chen <[email protected]> AuthorDate: Mon Mar 6 15:02:44 2023 +0800 Make RrocksDB checksum type configurable (#3793) Fix https://github.com/apache/bookkeeper/issues/3734#issuecomment-1407626941 We have two rocksDB tables, one for the ledger index, and another for the entry log location. - ledger index RocksDB table: Use the default table option, and the checksum is `kCRC32c` - entry log location RocksDb table: Use configured table option, and the checksum is `kxxHash` When we upgrade the RocksDB version from 6.10.2 to 7.9.2, the new RocksDB version's default table checksum has changed from `kCRC32c` to `kXXH3`, and `kXXH3` only supported since RocksDB 6.27. The RocksDB version rollback to 6.10.2 will be failed due to RocksDB 6.10.2 doesn't support the `kXXH3` checksum type. In this PR, I make the RocksDB checksum type configurable. But there is one change that will change the ledger index RocksDB table's checksum type from the default `kCRC32c` to `kxxHash`. I have tested the compatibility of the two checksum types in and between multiple RocksDB versions, it works fine. After setting the two RocksDB table's checksum type to `kxxHash`, the RocksDB's version upgraded from 6.10.2 to 7.9.2, and rolling back to 6.10.2 works fine. When writing the unit test to read the table checksum type from RocksDB configuration files, it failed. I found the related issue on RocksDB: https://github.com/facebook/rocksdb/issues/5297 The related PR: https://github.com/facebook/rocksdb/pull/10826 It means we still can't load RocksDB table options from configuration files. Maybe I missed some parts about reading RocksDB table options from the configuration file. If this issue exists, we do **NOT** recommend users configure RocksDB configurations through configuration files. @merlimat @eolivelli @dlg99 Please help take a look, thanks. (cherry picked from commit 3844bf128cbd07ffd0f77eb8ec9dcedc163a839b) --- .../bookie/storage/ldb/KeyValueStorageRocksDB.java | 7 +++++- .../storage/ldb/KeyValueStorageRocksDBTest.java | 27 ++++++++++++++++++---- .../resources/test_entry_location_rocksdb.conf | 18 +++++++++++++-- conf/default_rocksdb.conf.default | 6 ++++- conf/ledger_metadata_rocksdb.conf.default | 6 ++++- 5 files changed, 55 insertions(+), 9 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java index 361f4e80ce..613b1c187e 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java @@ -97,6 +97,7 @@ public class KeyValueStorageRocksDB implements KeyValueStorage { private static final String ROCKSDB_NUM_FILES_IN_LEVEL0 = "dbStorage_rocksDB_numFilesInLevel0"; private static final String ROCKSDB_MAX_SIZE_IN_LEVEL1_MB = "dbStorage_rocksDB_maxSizeInLevel1MB"; private static final String ROCKSDB_FORMAT_VERSION = "dbStorage_rocksDB_format_version"; + private static final String ROCKSDB_CHECKSUM_TYPE = "dbStorage_rocksDB_checksum_type"; public KeyValueStorageRocksDB(String basePath, String subPath, DbConfigType dbConfigType, ServerConfiguration conf) throws IOException { @@ -177,6 +178,7 @@ public class KeyValueStorageRocksDB implements KeyValueStorage { ServerConfiguration conf, boolean readOnly) throws IOException { Options options = new Options(); options.setCreateIfMissing(true); + ChecksumType checksumType = ChecksumType.valueOf(conf.getString(ROCKSDB_CHECKSUM_TYPE, "kxxHash")); if (dbConfigType == DbConfigType.EntryLocation) { /* Set default RocksDB block-cache size to 10% / numberOfLedgers of direct memory, unless override */ @@ -217,7 +219,7 @@ public class KeyValueStorageRocksDB implements KeyValueStorage { tableOptions.setBlockSize(blockSize); tableOptions.setBlockCache(cache); tableOptions.setFormatVersion(formatVersion); - tableOptions.setChecksumType(ChecksumType.kxxHash); + tableOptions.setChecksumType(checksumType); if (bloomFilterBitsPerKey > 0) { tableOptions.setFilterPolicy(new BloomFilter(bloomFilterBitsPerKey, false)); } @@ -229,6 +231,9 @@ public class KeyValueStorageRocksDB implements KeyValueStorage { options.setTableFormatConfig(tableOptions); } else { this.cache = null; + BlockBasedTableConfig tableOptions = new BlockBasedTableConfig(); + tableOptions.setChecksumType(checksumType); + options.setTableFormatConfig(tableOptions); } // Configure file path diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDBTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDBTest.java index c30bca19a7..873b2dfc24 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDBTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDBTest.java @@ -30,6 +30,8 @@ import java.nio.file.Paths; import java.util.List; import org.apache.bookkeeper.conf.ServerConfiguration; import org.junit.Test; +import org.rocksdb.BlockBasedTableConfig; +import org.rocksdb.ChecksumType; import org.rocksdb.ColumnFamilyDescriptor; import org.rocksdb.ColumnFamilyOptions; import org.rocksdb.CompressionType; @@ -85,18 +87,35 @@ public class KeyValueStorageRocksDBTest { } @Test - public void testLevelCompactionDynamicLevelBytesFromConfigurationFile() throws Exception { + public void testReadChecksumTypeFromBookieConfiguration() throws Exception { ServerConfiguration configuration = new ServerConfiguration(); - URL url = getClass().getClassLoader().getResource("conf/entry_location_rocksdb.conf"); + configuration.setEntryLocationRocksdbConf("entry_location_rocksdb.conf"); + File tmpDir = Files.createTempDirectory("bk-kv-rocksdbtest-conf").toFile(); + Files.createDirectory(Paths.get(tmpDir.toString(), "subDir")); + KeyValueStorageRocksDB rocksDB = new KeyValueStorageRocksDB(tmpDir.toString(), "subDir", + KeyValueStorageFactory.DbConfigType.EntryLocation, configuration); + assertNull(rocksDB.getColumnFamilyDescriptors()); + + Options options = (Options) rocksDB.getOptions(); + assertEquals(ChecksumType.kxxHash, ((BlockBasedTableConfig) options.tableFormatConfig()).checksumType()); + } + + //@Test + public void testReadChecksumTypeFromConfigurationFile() throws Exception { + ServerConfiguration configuration = new ServerConfiguration(); + URL url = getClass().getClassLoader().getResource("test_entry_location_rocksdb.conf"); configuration.setEntryLocationRocksdbConf(url.getPath()); File tmpDir = Files.createTempDirectory("bk-kv-rocksdbtest-file").toFile(); Files.createDirectory(Paths.get(tmpDir.toString(), "subDir")); KeyValueStorageRocksDB rocksDB = new KeyValueStorageRocksDB(tmpDir.toString(), "subDir", - KeyValueStorageFactory.DbConfigType.EntryLocation, configuration); + KeyValueStorageFactory.DbConfigType.EntryLocation, configuration); assertNotNull(rocksDB.getColumnFamilyDescriptors()); List<ColumnFamilyDescriptor> columnFamilyDescriptorList = rocksDB.getColumnFamilyDescriptors(); ColumnFamilyOptions familyOptions = columnFamilyDescriptorList.get(0).getOptions(); - assertEquals(true, familyOptions.levelCompactionDynamicLevelBytes()); + // There is a bug in RocksDB, which can't load BlockedBasedTableConfig from Options file. + // https://github.com/facebook/rocksdb/issues/5297 + // After the PR: https://github.com/facebook/rocksdb/pull/10826 merge, we can turn on this test. + assertEquals(ChecksumType.kxxHash, ((BlockBasedTableConfig) familyOptions.tableFormatConfig()).checksumType()); } } diff --git a/bookkeeper-server/src/test/resources/test_entry_location_rocksdb.conf b/bookkeeper-server/src/test/resources/test_entry_location_rocksdb.conf index 7cf44a10f1..fa3cf9c8d9 100644 --- a/bookkeeper-server/src/test/resources/test_entry_location_rocksdb.conf +++ b/bookkeeper-server/src/test/resources/test_entry_location_rocksdb.conf @@ -31,5 +31,19 @@ write_buffer_size=1024 # set by jni: options.setMaxWriteBufferNumber max_write_buffer_number=1 - # set by jni: options.setLevelCompactionDynamicLevelBytes - level_compaction_dynamic_level_bytes=true + +[TableOptions/BlockBasedTable "default"] + # set by jni: tableOptions.setBlockSize + block_size=65536 + # set by jni: tableOptions.setBlockCache + block_cache=206150041 + # set by jni: tableOptions.setFormatVersion + format_version=2 + # set by jni: tableOptions.setChecksumType + checksum=kxxHash + # set by jni: tableOptions.setFilterPolicy, bloomfilter:[bits_per_key]:[use_block_based_builder] + filter_policy=rocksdb.BloomFilter:10:false + # set by jni: tableOptions.setCacheIndexAndFilterBlocks + cache_index_and_filter_blocks=true + # set by jni: options.setLevelCompactionDynamicLevelBytes + level_compaction_dynamic_level_bytes=true \ No newline at end of file diff --git a/conf/default_rocksdb.conf.default b/conf/default_rocksdb.conf.default index 0f3a08779e..e9b8e7c3ec 100644 --- a/conf/default_rocksdb.conf.default +++ b/conf/default_rocksdb.conf.default @@ -26,4 +26,8 @@ [CFOptions "default"] # set by jni: options.setLogFileTimeToRoll - log_file_time_to_roll=86400 \ No newline at end of file + log_file_time_to_roll=86400 + +[TableOptions/BlockBasedTable "default"] + # set by jni: tableOptions.setChecksumType + checksum=kxxHash \ No newline at end of file diff --git a/conf/ledger_metadata_rocksdb.conf.default b/conf/ledger_metadata_rocksdb.conf.default index 0f3a08779e..e9b8e7c3ec 100644 --- a/conf/ledger_metadata_rocksdb.conf.default +++ b/conf/ledger_metadata_rocksdb.conf.default @@ -26,4 +26,8 @@ [CFOptions "default"] # set by jni: options.setLogFileTimeToRoll - log_file_time_to_roll=86400 \ No newline at end of file + log_file_time_to_roll=86400 + +[TableOptions/BlockBasedTable "default"] + # set by jni: tableOptions.setChecksumType + checksum=kxxHash \ No newline at end of file
