rocksdbjni: Transaction#multiGetForUpdate returns null for large key counts

[DaMatrix]

Note: Please use Issues only for bug reports. For questions, discussions, feature requests, etc. post to dev group: https://groups.google.com/forum/#!forum/rocksdb or https://www.facebook.com/groups/rocksdb.dev

Expected behavior

Transaction#multiGetForUpdate should never return null.

Actual behavior

Transaction#multiGetForUpdate (the version which accepts a List<ColumnFamilyHandle>) returns null when called with a large number of keys (>= 65537 keys, according to my test code).

Upon further investigation, this seems to affect other methods as well: the standard RocksDB#multiGetAsList(List<byte[]>) throws a NullPointerException when called with >= 65537 keys.

Steps to reproduce the behavior

Here's a minimum reproducible example

If I'm not mistaken, the root cause is in from the following code:

rocksdb/java/rocksjni/transaction.cc

Lines 253 to 257 in 2e09a54

	if (env->EnsureLocalCapacity(len_cols) != 0) {
	// out of memory
	*has_exception = JNI_TRUE;
	return std::vector<ROCKSDB_NAMESPACE::ColumnFamilyHandle*>();
	}

To the best of my knowledge, EnsureLocalCapacity is for ensuring that the requested number of local object references can be held by the current stack frame. It therefore makes very little sense to check it with the length of a primitive array (long[] in this case).

Here are all the instances I could find of EnsureLocalCapacity being used with the length of a primitive array:

rocksdb/java/rocksjni/optimistic_transaction_db.cc

Line 90 in 2e09a54

if (env->EnsureLocalCapacity(jcf_name_len) != 0) {

rocksdb/java/rocksjni/transaction_db.cc

Line 101 in 2e09a54

if (env->EnsureLocalCapacity(jcf_name_len) != 0) {

rocksdb/java/rocksjni/transaction.cc

Line 253 in 2e09a54

if (env->EnsureLocalCapacity(len_cols) != 0) {

rocksdb/java/rocksjni/transaction.cc

Line 316 in 2e09a54

if (env->EnsureLocalCapacity(len_key) != 0) {

rocksdb/java/rocksjni/transaction.cc

Line 669 in 2e09a54

if (env->EnsureLocalCapacity(jkey_part_len) != 0) {

rocksdb/java/rocksjni/transaction.cc

Line 688 in 2e09a54

if (env->EnsureLocalCapacity(jvalue_part_len) != 0) {

rocksdb/java/rocksjni/transaction.cc

Line 901 in 2e09a54

if (env->EnsureLocalCapacity(jkey_part_len) != 0) {

I'm pretty sure these are also wrong (they're not referencing primitive arrays, but object arrays whose references seem to be safely cleaned up - one of them is even commented regarding the same issue!):

rocksdb/java/rocksjni/optimistic_transaction_db.cc

Line 66 in 2e09a54

if (env->EnsureLocalCapacity(len_cols) != 0) {

rocksdb/java/rocksjni/transaction_db.cc

Line 69 in 2e09a54

if (env->EnsureLocalCapacity(len_cols) != 0) {

rocksdb/java/rocksjni/transaction.cc

Line 300 in 2e09a54

if (env->EnsureLocalCapacity(len_key_parts) != 0) {

rocksdb/java/rocksjni/rocksjni.cc

Line 1718 in 2e09a54

if (env->EnsureLocalCapacity(len_keys) != 0) {

rocksdb/java/rocksjni/rocksjni.cc

Lines 1792 to 1794 in 2e09a54

	// TODO(AR) it is not clear to me why EnsureLocalCapacity is needed for the
	// loop as we cleanup references with env->DeleteLocalRef(jentry_value);
	if (env->EnsureLocalCapacity(static_cast<jint>(s.size())) != 0) {

[alanpaxton]

Hi @DaMatrix just reviewing PRs/issues, this should all be resolved, working and regression tested. Could you close ?