forked from facebook/rocksdb
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
RocksJava API - fix Transaction.multiGet() size limit, remove bogus E…
…nsureLocalCapacity() calls (facebook#10674) Summary: Resolves see facebook#9006 Fixes 2 related issues with JNI local references in the RocksJava API. 1. Some instances of RocksJava API JNI code appear to have misunderstood the reason for `JNIEnv->EnsureLocalCapacity()` and are carrying out bogus checks which happen to fail with some larger parameter values (many column families in a single call, very long key names or values). Remove these checks and add some regression tests for the previous failures. 2. The helper for Transaction multiGet operations (`multiGet()`, `multiGetForUpdate()`,...) is limited in the number of keys it can `get()` for because it requires a corresponding number of live local references. Refactor the helper slightly, copying out the key contents within a loop so that the references don't have to exist at the same time. Pull Request resolved: facebook#10674 Reviewed By: ajkr Differential Revision: D40515361 Pulled By: jay-zhuang fbshipit-source-id: f1be0126181a698b3ad27c0945a39c54d950aa25
- Loading branch information
1 parent
bf78380
commit 17553bd
Showing
7 changed files
with
526 additions
and
97 deletions.
There are no files selected for viewing
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
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
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
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
140 changes: 140 additions & 0 deletions
140
java/src/test/java/org/rocksdb/MultiColumnRegressionTest.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| package org.rocksdb; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import org.junit.Rule; | ||
| import org.junit.Test; | ||
| import org.junit.rules.TemporaryFolder; | ||
| import org.junit.runner.RunWith; | ||
| import org.junit.runners.Parameterized; | ||
|
|
||
| /** | ||
| * Test for changes made by | ||
| * <a link="https://github.com/facebook/rocksdb/issues/9006">transactional multiGet problem</a> | ||
| * the tests here were previously broken by the nonsense removed by that change. | ||
| */ | ||
| @RunWith(Parameterized.class) | ||
| public class MultiColumnRegressionTest { | ||
| @Parameterized.Parameters | ||
| public static List<Params> data() { | ||
| return Arrays.asList(new Params(3, 100), new Params(3, 1000000)); | ||
| } | ||
|
|
||
| public static class Params { | ||
| final int numColumns; | ||
| final int keySize; | ||
|
|
||
| public Params(final int numColumns, final int keySize) { | ||
| this.numColumns = numColumns; | ||
| this.keySize = keySize; | ||
| } | ||
| } | ||
|
|
||
| @Rule public TemporaryFolder dbFolder = new TemporaryFolder(); | ||
|
|
||
| private final Params params; | ||
|
|
||
| public MultiColumnRegressionTest(final Params params) { | ||
| this.params = params; | ||
| } | ||
|
|
||
| @Test | ||
| public void transactionDB() throws RocksDBException { | ||
| final List<ColumnFamilyDescriptor> columnFamilyDescriptors = new ArrayList<>(); | ||
| for (int i = 0; i < params.numColumns; i++) { | ||
| StringBuilder sb = new StringBuilder(); | ||
| sb.append("cf" + i); | ||
| for (int j = 0; j < params.keySize; j++) sb.append("_cf"); | ||
| columnFamilyDescriptors.add(new ColumnFamilyDescriptor(sb.toString().getBytes())); | ||
| } | ||
| try (final Options opt = new Options().setCreateIfMissing(true); | ||
| final RocksDB db = RocksDB.open(opt, dbFolder.getRoot().getAbsolutePath())) { | ||
| final List<ColumnFamilyHandle> columnFamilyHandles = | ||
| db.createColumnFamilies(columnFamilyDescriptors); | ||
| } | ||
|
|
||
| columnFamilyDescriptors.add(new ColumnFamilyDescriptor("default".getBytes())); | ||
| final List<ColumnFamilyHandle> columnFamilyHandles = new ArrayList<>(); | ||
| try (final TransactionDB tdb = TransactionDB.open(new DBOptions().setCreateIfMissing(true), | ||
| new TransactionDBOptions(), dbFolder.getRoot().getAbsolutePath(), | ||
| columnFamilyDescriptors, columnFamilyHandles)) { | ||
| final WriteOptions writeOptions = new WriteOptions(); | ||
| try (Transaction transaction = tdb.beginTransaction(writeOptions)) { | ||
| for (int i = 0; i < params.numColumns; i++) { | ||
| transaction.put( | ||
| columnFamilyHandles.get(i), ("key" + i).getBytes(), ("value" + (i - 7)).getBytes()); | ||
| } | ||
| transaction.put("key".getBytes(), "value".getBytes()); | ||
| transaction.commit(); | ||
| } | ||
| for (ColumnFamilyHandle columnFamilyHandle : columnFamilyHandles) { | ||
| columnFamilyHandle.close(); | ||
| } | ||
| } | ||
|
|
||
| final List<ColumnFamilyHandle> columnFamilyHandles2 = new ArrayList<>(); | ||
| try (final TransactionDB tdb = TransactionDB.open(new DBOptions().setCreateIfMissing(true), | ||
| new TransactionDBOptions(), dbFolder.getRoot().getAbsolutePath(), | ||
| columnFamilyDescriptors, columnFamilyHandles2)) { | ||
| try (Transaction transaction = tdb.beginTransaction(new WriteOptions())) { | ||
| final ReadOptions readOptions = new ReadOptions(); | ||
| for (int i = 0; i < params.numColumns; i++) { | ||
| final byte[] value = | ||
| transaction.get(columnFamilyHandles2.get(i), readOptions, ("key" + i).getBytes()); | ||
| assertThat(value).isEqualTo(("value" + (i - 7)).getBytes()); | ||
| } | ||
| transaction.commit(); | ||
| } | ||
| for (ColumnFamilyHandle columnFamilyHandle : columnFamilyHandles2) { | ||
| columnFamilyHandle.close(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void optimisticDB() throws RocksDBException { | ||
| final List<ColumnFamilyDescriptor> columnFamilyDescriptors = new ArrayList<>(); | ||
| for (int i = 0; i < params.numColumns; i++) { | ||
| columnFamilyDescriptors.add(new ColumnFamilyDescriptor("default".getBytes())); | ||
| } | ||
|
|
||
| columnFamilyDescriptors.add(new ColumnFamilyDescriptor("default".getBytes())); | ||
| final List<ColumnFamilyHandle> columnFamilyHandles = new ArrayList<>(); | ||
| try (final OptimisticTransactionDB otdb = OptimisticTransactionDB.open( | ||
| new DBOptions().setCreateIfMissing(true), dbFolder.getRoot().getAbsolutePath(), | ||
| columnFamilyDescriptors, columnFamilyHandles)) { | ||
| try (Transaction transaction = otdb.beginTransaction(new WriteOptions())) { | ||
| for (int i = 0; i < params.numColumns; i++) { | ||
| transaction.put( | ||
| columnFamilyHandles.get(i), ("key" + i).getBytes(), ("value" + (i - 7)).getBytes()); | ||
| } | ||
| transaction.put("key".getBytes(), "value".getBytes()); | ||
| transaction.commit(); | ||
| } | ||
| for (ColumnFamilyHandle columnFamilyHandle : columnFamilyHandles) { | ||
| columnFamilyHandle.close(); | ||
| } | ||
| } | ||
|
|
||
| final List<ColumnFamilyHandle> columnFamilyHandles2 = new ArrayList<>(); | ||
| try (final OptimisticTransactionDB otdb = OptimisticTransactionDB.open( | ||
| new DBOptions().setCreateIfMissing(true), dbFolder.getRoot().getAbsolutePath(), | ||
| columnFamilyDescriptors, columnFamilyHandles2)) { | ||
| try (Transaction transaction = otdb.beginTransaction(new WriteOptions())) { | ||
| final ReadOptions readOptions = new ReadOptions(); | ||
| for (int i = 0; i < params.numColumns; i++) { | ||
| final byte[] value = | ||
| transaction.get(columnFamilyHandles2.get(i), readOptions, ("key" + i).getBytes()); | ||
| assertThat(value).isEqualTo(("value" + (i - 7)).getBytes()); | ||
| } | ||
| transaction.commit(); | ||
| } | ||
| for (ColumnFamilyHandle columnFamilyHandle : columnFamilyHandles2) { | ||
| columnFamilyHandle.close(); | ||
| } | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.