Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-35576]Fix exception on RocksDB.getColumnFamilyMetaData() (#12474) #79

Merged
merged 2 commits into from
Jul 23, 2024
Merged

[FLINK-35576]Fix exception on RocksDB.getColumnFamilyMetaData() (#12474) #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8e5639
Access SST full file checksum via RocksDB#getLiveFilesMetadata (#11770)
npepinpe Dec 26, 2023
9264b99
Fix exception on RocksDB.getColumnFamilyMetaData() (#12474)
rhubner Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions java/rocksjni/portal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7447,7 +7447,7 @@ class LiveFileMetaDataJni : public JavaClass {

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

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

jobject jlive_file_meta_data = env->NewObject(
jclazz, mid, jcolumn_family_name,
static_cast<jint>(live_file_meta_data->level), jfile_name, jpath,
Expand All @@ -7508,14 +7520,15 @@ class LiveFileMetaDataJni : public JavaClass {
static_cast<jlong>(live_file_meta_data->num_reads_sampled),
static_cast<jboolean>(live_file_meta_data->being_compacted),
static_cast<jlong>(live_file_meta_data->num_entries),
static_cast<jlong>(live_file_meta_data->num_deletions));
static_cast<jlong>(live_file_meta_data->num_deletions), jfile_checksum);

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

Expand All @@ -7525,6 +7538,7 @@ class LiveFileMetaDataJni : public JavaClass {
env->DeleteLocalRef(jpath);
env->DeleteLocalRef(jsmallest_key);
env->DeleteLocalRef(jlargest_key);
env->DeleteLocalRef(jfile_checksum);

return jlive_file_meta_data;
}
Expand Down Expand Up @@ -7555,7 +7569,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 @@ -7595,6 +7610,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 @@ -7603,13 +7629,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 @@ -7618,6 +7645,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
23 changes: 7 additions & 16 deletions java/src/main/java/org/rocksdb/LiveFileMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,13 @@ public class LiveFileMetaData extends SstFileMetaData {
/**
* Called from JNI C++
*/
private LiveFileMetaData(
final byte[] columnFamilyName,
final int level,
final String fileName,
final String path,
final long size,
final long smallestSeqno,
final long largestSeqno,
final byte[] smallestKey,
final byte[] largestKey,
final long numReadsSampled,
final boolean beingCompacted,
final long numEntries,
final long numDeletions) {
super(fileName, path, size, smallestSeqno, largestSeqno, smallestKey,
largestKey, numReadsSampled, beingCompacted, numEntries, numDeletions);
private LiveFileMetaData(final byte[] columnFamilyName, final int level, final String fileName,
final String path, final long size, final long smallestSeqno, final long largestSeqno,
final byte[] smallestKey, final byte[] largestKey, final long numReadsSampled,
final boolean beingCompacted, final long numEntries, final long numDeletions,
final byte[] fileChecksum) {
super(fileName, path, size, smallestSeqno, largestSeqno, smallestKey, largestKey,
numReadsSampled, beingCompacted, numEntries, numDeletions, fileChecksum);
this.columnFamilyName = columnFamilyName;
this.level = level;
}
Expand Down
15 changes: 14 additions & 1 deletion java/src/main/java/org/rocksdb/SstFileMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class SstFileMetaData {
private final boolean beingCompacted;
private final long numEntries;
private final long numDeletions;
private final byte[] fileChecksum;

/**
* Called from JNI C++
Expand All @@ -35,12 +36,13 @@ public class SstFileMetaData {
* @param beingCompacted true if the file is being compacted, false otherwise
* @param numEntries the number of entries
* @param numDeletions the number of deletions
* @param fileChecksum the full file checksum (if enabled)
*/
@SuppressWarnings("PMD.ArrayIsStoredDirectly")
protected SstFileMetaData(final String fileName, final String path, final long size,
final long smallestSeqno, final long largestSeqno, final byte[] smallestKey,
final byte[] largestKey, final long numReadsSampled, final boolean beingCompacted,
final long numEntries, final long numDeletions) {
final long numEntries, final long numDeletions, final byte[] fileChecksum) {
this.fileName = fileName;
this.path = path;
this.size = size;
Expand All @@ -52,6 +54,7 @@ protected SstFileMetaData(final String fileName, final String path, final long s
this.beingCompacted = beingCompacted;
this.numEntries = numEntries;
this.numDeletions = numDeletions;
this.fileChecksum = fileChecksum;
}

/**
Expand Down Expand Up @@ -154,4 +157,14 @@ public long numEntries() {
public long numDeletions() {
return numDeletions;
}

/**
* Get the full file checksum iff full file checksum is enabled.
*
* @return the file's checksum
*/
@SuppressWarnings("PMD.MethodReturnsInternalArray")
public byte[] fileChecksum() {
return fileChecksum;
}
}
55 changes: 55 additions & 0 deletions java/src/test/java/org/rocksdb/RocksDBTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1381,6 +1381,61 @@ public void getApproximateMemTableStatsSingleKey() throws RocksDBException {
}
}

@Test
public void getLiveFilesMetadataWithChecksum() throws RocksDBException {
final Properties props = new Properties();
final byte[] key1 = "key1".getBytes(UTF_8);
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)) {
final String dbPath = dbFolder.getRoot().getAbsolutePath();

// disable WAL so we have a deterministic checksum
try (final RocksDB db = RocksDB.open(options, dbPath);
final WriteOptions writeOptions = new WriteOptions().setDisableWAL(true)) {
db.put(writeOptions, key1, key1);
}

try (final RocksDB db = RocksDB.open(options, dbPath)) {
final List<LiveFileMetaData> expectedFileMetadata = db.getLiveFilesMetaData();
assertThat(expectedFileMetadata).hasSize(1);
// ideally we could re-compute here, but CRC32C is a Java 9 feature, so we have no CRC32C
// implementation available here
final LiveFileMetaData sstFile = expectedFileMetadata.get(0);
assertThat(sstFile.fileChecksum()).isNotEmpty();
}
}
}

@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
Loading