-
Notifications
You must be signed in to change notification settings - Fork 6.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix multiget throwing NPE for num of keys > 70k (#9012)
Summary: closes #8039 Unnecessary use of multiple local JNI references at the same time, 1 per key, was limiting the size of the key array. The local references don't need to be held simultaneously, so if we rearrange the code we can make it work for bigger key arrays. Incidentally, make errors throw helpful exception messages rather than returning a null pointer. Pull Request resolved: #9012 Reviewed By: mrambacher Differential Revision: D31580862 Pulled By: jay-zhuang fbshipit-source-id: ce05831d52ede332e1b20e74d2dc621d219b9616
- Loading branch information
1 parent
ffc48b6
commit f5526af
Showing
4 changed files
with
130 additions
and
21 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| // Copyright (c) 2011-present, Facebook, Inc. All rights reserved. | ||
| // This source code is licensed under both the GPLv2 (found in the | ||
| // COPYING file in the root directory) and Apache 2.0 License | ||
| // (found in the LICENSE.Apache file in the root directory). | ||
| package org.rocksdb; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.*; | ||
| import org.junit.Rule; | ||
| import org.junit.Test; | ||
| import org.junit.rules.TemporaryFolder; | ||
| import org.junit.runner.RunWith; | ||
| import org.junit.runners.Parameterized; | ||
|
|
||
| @RunWith(Parameterized.class) | ||
| public class MultiGetManyKeysTest { | ||
| @Parameterized.Parameters | ||
| public static List<Integer> data() { | ||
| return Arrays.asList(3, 250, 60000, 70000, 150000, 750000); | ||
| } | ||
|
|
||
| @Rule public TemporaryFolder dbFolder = new TemporaryFolder(); | ||
|
|
||
| private final int keySize; | ||
|
|
||
| public MultiGetManyKeysTest(final Integer keySize) { | ||
| this.keySize = keySize; | ||
| } | ||
|
|
||
| /** | ||
| * Test for https://github.com/facebook/rocksdb/issues/8039 | ||
| */ | ||
| @Test | ||
| public void multiGetAsListLarge() throws RocksDBException { | ||
| final Random rand = new Random(); | ||
| final List<byte[]> keys = new ArrayList<>(); | ||
| for (int i = 0; i < keySize; i++) { | ||
| final byte[] key = new byte[4]; | ||
| rand.nextBytes(key); | ||
| keys.add(key); | ||
| } | ||
|
|
||
| try (final Options opt = new Options().setCreateIfMissing(true); | ||
| final RocksDB db = RocksDB.open(opt, dbFolder.getRoot().getAbsolutePath())) { | ||
| final List<byte[]> values = db.multiGetAsList(keys); | ||
| assertThat(values.size()).isEqualTo(keys.size()); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void multiGetAsListCheckResults() throws RocksDBException { | ||
| try (final Options opt = new Options().setCreateIfMissing(true); | ||
| final RocksDB db = RocksDB.open(opt, dbFolder.getRoot().getAbsolutePath())) { | ||
| final List<byte[]> keys = new ArrayList<>(); | ||
| for (int i = 0; i < keySize; i++) { | ||
| byte[] key = ("key" + i + ":").getBytes(); | ||
| keys.add(key); | ||
| db.put(key, ("value" + i + ":").getBytes()); | ||
| } | ||
|
|
||
| final List<byte[]> values = db.multiGetAsList(keys); | ||
| assertThat(values.size()).isEqualTo(keys.size()); | ||
| for (int i = 0; i < keySize; i++) { | ||
| assertThat(values.get(i)).isEqualTo(("value" + i + ":").getBytes()); | ||
| } | ||
| } | ||
| } | ||
| } |
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,34 @@ | ||
| // Copyright (c) 2011-present, Facebook, Inc. All rights reserved. | ||
| // This source code is licensed under both the GPLv2 (found in the | ||
| // COPYING file in the root directory) and Apache 2.0 License | ||
| // (found in the LICENSE.Apache file in the root directory). | ||
| package org.rocksdb; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import org.junit.Rule; | ||
| import org.junit.Test; | ||
| import org.junit.rules.TemporaryFolder; | ||
|
|
||
| public class MultiGetTest { | ||
| @Rule public TemporaryFolder dbFolder = new TemporaryFolder(); | ||
|
|
||
| @Test | ||
| public void putNThenMultiGet() throws RocksDBException { | ||
| try (final Options opt = new Options().setCreateIfMissing(true); | ||
| final RocksDB db = RocksDB.open(opt, dbFolder.getRoot().getAbsolutePath())) { | ||
| db.put("key1".getBytes(), "value1ForKey1".getBytes()); | ||
| db.put("key2".getBytes(), "value2ForKey2".getBytes()); | ||
| db.put("key3".getBytes(), "value3ForKey3".getBytes()); | ||
| final List<byte[]> keys = | ||
| Arrays.asList("key1".getBytes(), "key2".getBytes(), "key3".getBytes()); | ||
| final List<byte[]> values = db.multiGetAsList(keys); | ||
| assertThat(values.size()).isEqualTo(keys.size()); | ||
| assertThat(values.get(0)).isEqualTo("value1ForKey1".getBytes()); | ||
| assertThat(values.get(1)).isEqualTo("value2ForKey2".getBytes()); | ||
| assertThat(values.get(2)).isEqualTo("value3ForKey3".getBytes()); | ||
| } | ||
| } | ||
| } |