Skip to content

Commit

Permalink
Java API - Fix handling of CF handles in DB subclasses (facebook#12417)
Browse files Browse the repository at this point in the history
Summary:
The most general `open()` method for each of RocksDB, TtlDB, OptimisticTransactionDB and TransactionDB should
- ensure the default CF is supplied in the list of descriptors
- cache the default CF handle
- store open CF handles for automatic close on DB close
The `close()` method in each of these DB subclasses should `close()` all the owned CF handles.

I can’t find a cleaner way to build some generalised open/close that does this for all DB subclasses, so it exists as cut and paste with variations in the 4 different DB subclasses.

Added some slightly paranoid testing that CF handles explicitly referred to as default in a list of CF handles in the general open methods, and the simple open that doesn’t supply a CF, end up reading and writing to the same CF. Prompted by the fact that this code is a bit opaque; the first returned handle is the DB.

As part of this, fix the bug where the Java side of `OptimisticsTransactionDB` was not setting up default column family; this was visible when setting up an iterator; add a test to validate that the iterator is OK. A single Java reference to the default column family was not being created in the OptimisticsTransactionDB RocksDB subclass; it should be created in all subclasses. The same problem had previously been fixed for TtlDB.

Pull Request resolved: facebook#12417

Reviewed By: ajkr

Differential Revision: D54807643

Pulled By: pdillinger

fbshipit-source-id: 66f34e56a822a009a8f2018d401cf8940d91aa35
  • Loading branch information
alanpaxton authored and facebook-github-bot committed Mar 12, 2024
1 parent 7622029 commit c4d37da
Show file tree
Hide file tree
Showing 8 changed files with 369 additions and 19 deletions.
22 changes: 20 additions & 2 deletions java/src/main/java/org/rocksdb/OptimisticTransactionDB.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package org.rocksdb;

import java.util.Arrays;
import java.util.List;

/**
Expand Down Expand Up @@ -45,6 +46,8 @@ public static OptimisticTransactionDB open(final Options options,
// the currently-created RocksDB.
otdb.storeOptionsInstance(options);

otdb.storeDefaultColumnFamilyHandle(otdb.makeDefaultColumnFamilyHandle());

return otdb;
}

Expand All @@ -67,14 +70,21 @@ public static OptimisticTransactionDB open(final DBOptions dbOptions,
final List<ColumnFamilyDescriptor> columnFamilyDescriptors,
final List<ColumnFamilyHandle> columnFamilyHandles)
throws RocksDBException {

int defaultColumnFamilyIndex = -1;
final byte[][] cfNames = new byte[columnFamilyDescriptors.size()][];
final long[] cfOptionHandles = new long[columnFamilyDescriptors.size()];
for (int i = 0; i < columnFamilyDescriptors.size(); i++) {
final ColumnFamilyDescriptor cfDescriptor = columnFamilyDescriptors
.get(i);
cfNames[i] = cfDescriptor.getName();
cfOptionHandles[i] = cfDescriptor.getOptions().nativeHandle_;
if (Arrays.equals(cfDescriptor.getName(), RocksDB.DEFAULT_COLUMN_FAMILY)) {
defaultColumnFamilyIndex = i;
}
}
if (defaultColumnFamilyIndex < 0) {
throw new IllegalArgumentException(
"You must provide the default column family in your columnFamilyDescriptors");
}

final long[] handles = open(dbOptions.nativeHandle_, path, cfNames,
Expand All @@ -86,12 +96,14 @@ public static OptimisticTransactionDB open(final DBOptions dbOptions,
// in RocksDB can prevent Java to GC during the life-time of
// the currently-created RocksDB.
otdb.storeOptionsInstance(dbOptions);
otdb.storeDefaultColumnFamilyHandle(otdb.makeDefaultColumnFamilyHandle());

for (int i = 1; i < handles.length; i++) {
columnFamilyHandles.add(new ColumnFamilyHandle(otdb, handles[i]));
}

otdb.ownedColumnFamilyHandles.addAll(columnFamilyHandles);
otdb.storeDefaultColumnFamilyHandle(columnFamilyHandles.get(defaultColumnFamilyIndex));

return otdb;
}

Expand Down Expand Up @@ -133,6 +145,12 @@ public void closeE() throws RocksDBException {
@SuppressWarnings("PMD.EmptyCatchBlock")
@Override
public void close() {
for (final ColumnFamilyHandle columnFamilyHandle : // NOPMD - CloseResource
ownedColumnFamilyHandles) {
columnFamilyHandle.close();
}
ownedColumnFamilyHandles.clear();

if (owningHandle_.compareAndSet(true, false)) {
try {
closeDatabase(nativeHandle_);
Expand Down
16 changes: 11 additions & 5 deletions java/src/main/java/org/rocksdb/RocksDB.java
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,7 @@ public static RocksDB open(final Options options, final String path)
*/
public static RocksDB open(final DBOptions options, final String path,
final List<ColumnFamilyDescriptor> columnFamilyDescriptors,
final List<ColumnFamilyHandle> columnFamilyHandles)
throws RocksDBException {

final List<ColumnFamilyHandle> columnFamilyHandles) throws RocksDBException {
final byte[][] cfNames = new byte[columnFamilyDescriptors.size()][];
final long[] cfOptionHandles = new long[columnFamilyDescriptors.size()];
int defaultColumnFamilyIndex = -1;
Expand All @@ -320,7 +318,7 @@ public static RocksDB open(final DBOptions options, final String path,
}
}
if (defaultColumnFamilyIndex < 0) {
new IllegalArgumentException(
throw new IllegalArgumentException(
"You must provide the default column family in your columnFamilyDescriptors");
}

Expand Down Expand Up @@ -502,11 +500,19 @@ public static RocksDB openReadOnly(final DBOptions options, final String path,

final byte[][] cfNames = new byte[columnFamilyDescriptors.size()][];
final long[] cfOptionHandles = new long[columnFamilyDescriptors.size()];
int defaultColumnFamilyIndex = -1;
for (int i = 0; i < columnFamilyDescriptors.size(); i++) {
final ColumnFamilyDescriptor cfDescriptor = columnFamilyDescriptors
.get(i);
cfNames[i] = cfDescriptor.getName();
cfOptionHandles[i] = cfDescriptor.getOptions().nativeHandle_;
if (Arrays.equals(cfDescriptor.getName(), RocksDB.DEFAULT_COLUMN_FAMILY)) {
defaultColumnFamilyIndex = i;
}
}
if (defaultColumnFamilyIndex < 0) {
throw new IllegalArgumentException(
"You must provide the default column family in your columnFamilyDescriptors");
}

final long[] handles =
Expand All @@ -521,7 +527,7 @@ public static RocksDB openReadOnly(final DBOptions options, final String path,
}

db.ownedColumnFamilyHandles.addAll(columnFamilyHandles);
db.storeDefaultColumnFamilyHandle(db.makeDefaultColumnFamilyHandle());
db.storeDefaultColumnFamilyHandle(columnFamilyHandles.get(defaultColumnFamilyIndex));

return db;
}
Expand Down
19 changes: 17 additions & 2 deletions java/src/main/java/org/rocksdb/TransactionDB.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.rocksdb;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -77,14 +78,21 @@ public static TransactionDB open(final DBOptions dbOptions,
final List<ColumnFamilyDescriptor> columnFamilyDescriptors,
final List<ColumnFamilyHandle> columnFamilyHandles)
throws RocksDBException {

int defaultColumnFamilyIndex = -1;
final byte[][] cfNames = new byte[columnFamilyDescriptors.size()][];
final long[] cfOptionHandles = new long[columnFamilyDescriptors.size()];
for (int i = 0; i < columnFamilyDescriptors.size(); i++) {
final ColumnFamilyDescriptor cfDescriptor = columnFamilyDescriptors
.get(i);
cfNames[i] = cfDescriptor.getName();
cfOptionHandles[i] = cfDescriptor.getOptions().nativeHandle_;
if (Arrays.equals(cfDescriptor.getName(), RocksDB.DEFAULT_COLUMN_FAMILY)) {
defaultColumnFamilyIndex = i;
}
}
if (defaultColumnFamilyIndex < 0) {
throw new IllegalArgumentException(
"You must provide the default column family in your columnFamilyDescriptors");
}

final long[] handles = open(dbOptions.nativeHandle_,
Expand All @@ -95,12 +103,13 @@ public static TransactionDB open(final DBOptions dbOptions,
// in RocksDB can prevent Java to GC during the life-time of
// the currently-created RocksDB.
tdb.storeOptionsInstance(dbOptions);
tdb.storeDefaultColumnFamilyHandle(tdb.makeDefaultColumnFamilyHandle());
tdb.storeTransactionDbOptions(transactionDbOptions);

for (int i = 1; i < handles.length; i++) {
columnFamilyHandles.add(new ColumnFamilyHandle(tdb, handles[i]));
}
tdb.ownedColumnFamilyHandles.addAll(columnFamilyHandles);
tdb.storeDefaultColumnFamilyHandle(columnFamilyHandles.get(defaultColumnFamilyIndex));

return tdb;
}
Expand Down Expand Up @@ -143,6 +152,12 @@ public void closeE() throws RocksDBException {
@SuppressWarnings("PMD.EmptyCatchBlock")
@Override
public void close() {
for (final ColumnFamilyHandle columnFamilyHandle : // NOPMD - CloseResource
ownedColumnFamilyHandles) {
columnFamilyHandle.close();
}
ownedColumnFamilyHandles.clear();

if (owningHandle_.compareAndSet(true, false)) {
try {
closeDatabase(nativeHandle_);
Expand Down
8 changes: 7 additions & 1 deletion java/src/main/java/org/rocksdb/TtlDB.java
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public static TtlDB open(final DBOptions options, final String db_path,
}
}
if (defaultColumnFamilyIndex < 0) {
new IllegalArgumentException(
throw new IllegalArgumentException(
"You must provide the default column family in your columnFamilyDescriptors");
}

Expand Down Expand Up @@ -195,6 +195,12 @@ public void closeE() throws RocksDBException {
@SuppressWarnings("PMD.EmptyCatchBlock")
@Override
public void close() {
for (final ColumnFamilyHandle columnFamilyHandle : // NOPMD - CloseResource
ownedColumnFamilyHandles) {
columnFamilyHandle.close();
}
ownedColumnFamilyHandles.clear();

if (owningHandle_.compareAndSet(true, false)) {
try {
closeDatabase(nativeHandle_);
Expand Down
Loading

0 comments on commit c4d37da

Please sign in to comment.