apache
Upgrade RocksDB to version 9.2.1
Descriptions of the changes in this PR:
Fix #4409
Motivation
Upgrade RocksDB to v.9.2.1 to pick up latest performance and stability improvements and fixes in deleteRanges. Hopefully deleteRanges() is stable enough for us to try again perf improvements https://github.com/apache/bookkeeper/pull/3653 that were reverted previously.
Changes
Changed RocksDB version, updated code to use changed API.
Have you verified any compatible issues from 6.16.x to 9.2.1 and 7.x to 9.2.1?
@hangc0276 I fixed/updated backward compat tests, that's it. If these aren't enough we need to invest some time into these kinds of tests
@hangc0276 I rebased after the merge of downgrade compat tests; upgrade to 9.2.1 doesn't downgrade nicely. I am not yet sure what can be configured to support compatibility.
@hangc0276 I rebased after the merge of downgrade compat tests; upgrade to 9.2.1 doesn't downgrade nicely. I am not yet sure what can be configured to support compatibility.
In RocksDB 9.0.0 release notes there's "format_version=6 is the new default setting in BlockBasedTableOptions, for more robust data integrity checking. DBs and SST files written with this setting cannot be read by RocksDB versions before 8.6.0."
@dlg99 Perhaps setting dbStorage_rocksDB_format_version=5 instead of dbStorage_rocksDB_format_version=2 could help solve this issue? It seems that format_version=2 is really old and doesn't support many optimizations.
@lhotari Have you tested the downgrade? We set the format_version to 2 by default, and it may not impacted by version 6
@lhotari Have you tested the downgrade? We set the
format_versionto2by default, and it may not impacted by version6
@hangc0276 The format_version setting was missing for conf/ledger_metadata_rocksdb.conf. that's been a problem previously. I haven't tested it yet.
@lhotari Have you tested the downgrade? We set the
format_versionto2by default, and it may not impacted by version6@hangc0276 The format_version setting was missing for
conf/ledger_metadata_rocksdb.conf. that's been a problem previously. I haven't tested it yet.
@lhotari It is set to 2
- https://github.com/apache/bookkeeper/blob/8890a61418e3961e0000108b3c2e8ea176691e5f/conf/entry_location_rocksdb.conf.default#L62-L63
- https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/conf/entry_location_rocksdb.conf#L61-L62
@lhotari It is set to
2
- https://github.com/apache/bookkeeper/blob/8890a61418e3961e0000108b3c2e8ea176691e5f/conf/entry_location_rocksdb.conf.default#L62-L63
- https://github.com/apache/pulsar/blob/7636e8989f4d3fc24fce69a356d54e1c550945ed/conf/entry_location_rocksdb.conf#L61-L62
@hangc0276 Yes, you are right that format_version should already be set to 2.
I was assuming that the ledger metadata database would have been the problem. However, when conf/entry_location_rocksdb.conf is missing, the format_version set in code would get used.
(logic https://github.com/apache/bookkeeper/blob/f048e7a3b127cfa3a7e9761d589225f078d6712c/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java#L126-L140)
By default in BK, the file is missing since the provide file is conf/entry_location_rocksdb.conf.default and that has no impact.
However, since format_version 2 is ancient, you never know if it starts causing problems when continuing to use it with RocskDB 9.x+. I haven't yet investigated the problem.
PRs that have not been processed for a long time will be closed first. Please open them yourself if necessary.
@hangc0276 I rebased after the merge of downgrade compat tests; upgrade to 9.2.1 doesn't downgrade nicely. I am not yet sure what can be configured to support compatibility.
@dlg99 It seems that format version wasn't specified for all databases in Bookkeeper. #4559 will address that for the cases where configuration isn't read from the configuration files. In master branch, reading from configuration files has been broken since #4407 and that's fixed in #4560. After these are merged, it would make sense to resume this PR. Are you interested in continuing this work? Perhaps we could bump directly to latest 9.x of RocksDB, 9.10.0
@hangc0276 I rebased after the merge of downgrade compat tests; upgrade to 9.2.1 doesn't downgrade nicely. I am not yet sure what can be configured to support compatibility.
@dlg99 It seems that format version wasn't specified for all databases in Bookkeeper. #4559 will address that for the cases where configuration isn't read from the configuration files. In master branch, reading from configuration files has been broken since #4407 and that's fixed in #4560. After these are merged, it would make sense to resume this PR. Are you interested in continuing this work? Perhaps we could bump directly to latest 9.x of RocksDB, 9.10.0
@lhotari We should hold on to the upgrade before verifying the compatibility issue. https://github.com/apache/bookkeeper/pull/4559 set formatVersion to 5. Does it mean all the old BookKeeper versions are need to upgrade to a version that contains #4559, and then upgrade to the version that upgraded RocksDB to 9.x?
@lhotari We should hold on to the upgrade before verifying the compatibility issue. https://github.com/apache/bookkeeper/pull/4559 set formatVersion to 5. Does it mean all the old BookKeeper versions are need to upgrade to a version that contains #4559, and then upgrade to the version that upgraded RocksDB to 9.x?
@hangc0276 No, it doesn't mean that. formatVersion 5 has been around for a long time. The problem was that not all databases had the formatVersion set and that caused the problem in downgrading, since format 6 is the default since 8.8 (IIRC) unless explicitly set.