Skip to content

Commit

Permalink
Fix exception on RocksDB.getColumnFamilyMetaData() (#12474)
Browse files Browse the repository at this point in the history
Summary:
#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 #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: #12474

Reviewed By: jowlyzhang

Differential Revision: D55811808

Pulled By: ajkr

fbshipit-source-id: 2ab156f41eaf4a4f30c49e6df421b61e8451230e
  • Loading branch information
rhubner authored and ajkr committed Apr 9, 2024
1 parent f444196 commit 63e7f12
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 2 deletions.
18 changes: 16 additions & 2 deletions java/rocksjni/portal.h
Original file line number Diff line number Diff line change
Expand Up @@ -7678,7 +7678,8 @@ class SstFileMetaDataJni : public JavaClass {
}

jmethodID mid = env->GetMethodID(
jclazz, "<init>", "(Ljava/lang/String;Ljava/lang/String;JJJ[B[BJZJJ)V");
jclazz, "<init>",
"(Ljava/lang/String;Ljava/lang/String;JJJ[B[BJZJJ[B)V");
if (mid == nullptr) {
// exception thrown: NoSuchMethodException or OutOfMemoryError
return nullptr;
Expand Down Expand Up @@ -7718,6 +7719,17 @@ class SstFileMetaDataJni : public JavaClass {
return nullptr;
}

jbyteArray jfile_checksum = ROCKSDB_NAMESPACE::JniUtil::copyBytes(
env, sst_file_meta_data->file_checksum);
if (env->ExceptionCheck()) {
// exception occurred creating java string
env->DeleteLocalRef(jfile_name);
env->DeleteLocalRef(jpath);
env->DeleteLocalRef(jsmallest_key);
env->DeleteLocalRef(jlargest_key);
return nullptr;
}

jobject jsst_file_meta_data = env->NewObject(
jclazz, mid, jfile_name, jpath,
static_cast<jlong>(sst_file_meta_data->size),
Expand All @@ -7726,13 +7738,14 @@ class SstFileMetaDataJni : public JavaClass {
jlargest_key, static_cast<jlong>(sst_file_meta_data->num_reads_sampled),
static_cast<jboolean>(sst_file_meta_data->being_compacted),
static_cast<jlong>(sst_file_meta_data->num_entries),
static_cast<jlong>(sst_file_meta_data->num_deletions));
static_cast<jlong>(sst_file_meta_data->num_deletions), jfile_checksum);

if (env->ExceptionCheck()) {
env->DeleteLocalRef(jfile_name);
env->DeleteLocalRef(jpath);
env->DeleteLocalRef(jsmallest_key);
env->DeleteLocalRef(jlargest_key);
env->DeleteLocalRef(jfile_checksum);
return nullptr;
}

Expand All @@ -7741,6 +7754,7 @@ class SstFileMetaDataJni : public JavaClass {
env->DeleteLocalRef(jpath);
env->DeleteLocalRef(jsmallest_key);
env->DeleteLocalRef(jlargest_key);
env->DeleteLocalRef(jfile_checksum);

return jsst_file_meta_data;
}
Expand Down
27 changes: 27 additions & 0 deletions java/src/test/java/org/rocksdb/RocksDBTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1407,6 +1407,33 @@ public void getLiveFilesMetadataWithChecksum() throws RocksDBException {
}
}

@Test
public void getColumnFamilyMetadataWithChecksum() throws RocksDBException {
final Properties props = new Properties();
props.put("file_checksum_gen_factory", "FileChecksumGenCrc32cFactory");
final String dbPath = dbFolder.getRoot().getAbsolutePath();

try (final DBOptions dbOptions = DBOptions.getDBOptionsFromProps(props);
final ColumnFamilyOptions cfOptions = new ColumnFamilyOptions();
final Options options = new Options(dbOptions, cfOptions).setCreateIfMissing(true)) {
try (final RocksDB db = RocksDB.open(options, dbPath);
final WriteOptions writeOptions = new WriteOptions().setDisableWAL(true)) {
db.put("key".getBytes(UTF_8), "value".getBytes(UTF_8));
}

try (final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath())) {
ColumnFamilyMetaData metadata = db.getColumnFamilyMetaData(); // Exception here
List<LevelMetaData> levels = metadata.levels();
assertThat(levels).isNotEmpty();
List<SstFileMetaData> filesMetadata = levels.get(0).files();
assertThat(filesMetadata).isNotEmpty();
assertThat(filesMetadata.get(0).fileChecksum()).isNotNull();
assertThat(filesMetadata.get(0).fileChecksum()).hasSize(4);
assertThat(filesMetadata.get(0).fileChecksum()).isNotEqualTo(new byte[] {0, 0, 0, 0});
}
}
}

@Ignore("TODO(AR) re-enable when ready!")
@Test
public void compactFiles() throws RocksDBException {
Expand Down

0 comments on commit 63e7f12

Please sign in to comment.