Java API consistency between RocksDB.put() , .merge() and Transaction.put() , .merge()

[alanpaxton]

Implement new Java API get()/put()/merge() methods, and transactional variants.

The Java API methods are very inconsistent in terms of how they pass parameters (byte[], ByteBuffer), and what variants and defaulted parameters they support. We try to bring some consistency to this.

• All APIs should support calls with ByteBuffer parameters.
• Similar methods (RocksDB.get() vs Transaction.get()) should support as similar as possible sets of parameters for predictability.
• get()-like methods should provide variants where the caller supplies the target buffer, for the sake of efficiency. Allocation costs in Java can be significant when large buffers are repeatedly allocated and freed.

API Additions

1. RockDB.get implement indirect ByteBuffers. Added indirect ByteBuffers and supporting native methods for get().
2. RocksDB.Iterator implement missing (byte[], offset, length) variants for key() and value() parameters.
3. Transaction.get() implement missing methods, based on RocksDB.get. Added ByteBuffer.get with and without column family. Added byte[]-as-target get.
4. Transaction.iterator() implement a getIterator() which defaults ReadOptions; as per RocksDB.iterator(). Rationalize support API for this and RocksDB.iterator()
5. RocksDB.merge implement ByteBuffer methods; both direct and indirect buffers. Shadow the methods of RocksDB.put; RocksDB.put only offers ByteBuffer API with explicit WriteOptions. Duplicated this with RocksDB.merge
6. Transaction.merge implement methods as per RocksDB.merge methods. Transaction is already constructed with WriteOptions, so no explicit WriteOptions methods required.
7. Transaction.mergeUntracked implement the same API methods as Transaction.merge except the ones that use assumeTracked, because that’s not a feature of merge untracked.

Support Changes (C++)

The current JNI code in C++ supports multiple variants of methods through a number of helper functions. There are numerous TODO suggestions in the code proposing that the helpers be re-factored/shared.

We have taken a different approach for the new methods; we have created wrapper classes JDirectBufferSlice, JDirectBufferPinnableSlice, JByteArraySlice and JByteArrayPinnableSlice RAII classes which construct slices from JNI parameters and can then be passed directly to RocksDB methods. For instance, the Java_org_rocksdb_Transaction_getDirect method is implemented like this:

  try {
    ROCKSDB_NAMESPACE::JDirectBufferSlice key(env, jkey_bb, jkey_off,
                                              jkey_part_len);
    ROCKSDB_NAMESPACE::JDirectBufferPinnableSlice value(env, jval_bb, jval_off,
                                                        jval_part_len);
    ROCKSDB_NAMESPACE::KVException::ThrowOnError(
        env, txn->Get(*read_options, column_family_handle, key.slice(),
                      &value.pinnable_slice()));
    return value.Fetch();
  } catch (const ROCKSDB_NAMESPACE::KVException& e) {
    return e.Code();
  }

Notice the try/catch mechanism with the KVException class, which combined with RAII and the wrapper classes means that there is no ad-hoc cleanup necessary in the JNI methods.

We propose to extend this mechanism to existing JNI methods as further work.

Support Changes (Java)

Where there are multiple parameter-variant versions of the same method, we use fewer or just one supporting native method for all of them. This makes maintenance a bit easier and reduces the opportunity for coding errors mixing up (untyped) object handles.

In order to support this efficiently, some classes need to have default values for column families and read options added and cached so that they are not re-constructed on every method call.

This PR closes #9776

[anand1976]

@alanpaxton Please rebase. A lot of new merge conflicts due to #10951 getting merged, I think.

[alanpaxton]

@anand1976 @adamretter rebased and once again ready for review/import.

[adamretter]

@anand1976 rebased and once again ready for review/import.

[adamretter]

@anand1976 rebased and once again ready for review/import.

[alanpaxton]

@anand1976 rebased and once again ready for review/import.

[adamretter]

Hi @ajkr @ltamasi would you be able to get this one merged pleased?

[alanpaxton]

@anand1976 I have rebased, and this is now passing except for the known vs2019 issue on main which @rhubner is addressing. Are you happy to review/merge it at this point ?

[facebook-github-bot]

@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@jowlyzhang merged this pull request in 5a063ec.