[FLINK-35576]Fix exception on RocksDB.getColumnFamilyMetaData() (#12474) #79
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mayuehappy
force-pushed
the
cherry-pick-12474
branch
from
3585f32 to
2bcbf92
Compare
Summary: **Description** This PR passes along the native `LiveFileMetaData#file_checksum` field from the C++ class to the Java API as a copied byte array. If there is no file checksum generator factory set beforehand, then the array will empty. Please advise if you'd rather it be null - an empty array means one extra allocation, but it avoids possible null pointer exceptions. > **Note** > This functionality complements but does not supersede facebook/rocksdb#11736 It's outside the scope here to add support for Java based `FileChecksumGenFactory` implementations. As a workaround, users can already use the built-in one by creating their initial `DBOptions` via properties: ```java final Properties props = new Properties(); props.put("file_checksum_gen_factory", "FileChecksumGenCrc32cFactory"); try (final DBOptions dbOptions = DBOptions.getDBOptionsFromProps(props); final ColumnFamilyOptions cfOptions = new ColumnFamilyOptions(); final Options options = new Options(dbOptions, cfOptions).setCreateIfMissing(true)) { // do stuff } ``` I wanted to add a better test, but unfortunately there's no available CRC32C implementation available in Java 8 without adding a dependency or adding a JNI helper for RocksDB's own implementation (or bumping the minimum version for tests to Java 9). That said, I understand the test is rather poor, so happy to change it to whatever you'd like. **Context** To give some context, we replicate RocksDB checkpoints to other nodes. Part of this is verifying the integrity of each file during replication. With a large enough RocksDB, computing the checksum ourselves is prohibitively expensive. Since SST files comprise the bulk of the data, we'd much rather delegate this to RocksDB on file write, and read it back after to compare. It's likely we will provide a follow up to read the file checksum list directly from the manifest without having to open the DB, but this was the easiest first step to get it working for us. Pull Request resolved: facebook/rocksdb#11770 Reviewed By: hx235 Differential Revision: D52420729 Pulled By: ajkr fbshipit-source-id: a873de35a48aaf315e125733091cd221a97b9073 (cherry picked from commit 5b073a7)
Summary: facebook/rocksdb#12466 reported a bug when `RocksDB.getColumnFamilyMetaData()` is called on an existing database(With files stored on disk). As neilramaswamy mentioned, this was caused by facebook/rocksdb#11770 where the signature of `SstFileMetaData` constructor was changed, but JNI code wasn't updated. This PR fix JNI code, and also properly populate `fileChecksum` on `SstFileMetaData`. Pull Request resolved: facebook/rocksdb#12474 Reviewed By: jowlyzhang Differential Revision: D55811808 Pulled By: ajkr fbshipit-source-id: 2ab156f41eaf4a4f30c49e6df421b61e8451230e (cherry picked from commit a8035ebc0b22f079a447bdc6b0aeeb2f1cf09d47)
mayuehappy
force-pushed
the
cherry-pick-12474
branch
from
2bcbf92 to
9264b99
Compare
rkhachatryan
approved these changes
Jul 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
facebook/rocksdb#12466 reported a bug when
RocksDB.getColumnFamilyMetaData()is called on an existing database(With files stored on disk). As neilramaswamy mentioned, this was caused by facebook/rocksdb#11770 where the signature ofSstFileMetaDataconstructor was changed, but JNI code wasn't updated.This PR fix JNI code, and also properly populate
fileChecksumonSstFileMetaData.Pull Request resolved: facebook/rocksdb#12474
Reviewed By: jowlyzhang
Differential Revision: D55811808
Pulled By: ajkr
fbshipit-source-id: 2ab156f41eaf4a4f30c49e6df421b61e8451230e
(cherry picked from commit a8035ebc0b22f079a447bdc6b0aeeb2f1cf09d47)