]> git-server-git.apps.pok.os.sepia.ceph.com Git - rocksdb.git/commitdiff
JNI support for ReadOptions::iterate_upper_bound
authorBen Clay <bclay@twitter.com>
Fri, 15 Sep 2017 01:18:49 +0000 (18:18 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Fri, 15 Sep 2017 01:28:20 +0000 (18:28 -0700)
Summary:
Plumbed ReadOptions::iterate_upper_bound through JNI.

Made the following design choices:
* Used Slice instead of AbstractSlice due to the anticipated usecase (key / key prefix). Can change this if anyone disagrees.
* Used Slice instead of raw byte[] which seemed cleaner but necessitated the package-private handle-based Slice constructor. Followed WriteBatch as an example.
* We need a copy constructor for ReadOptions, as we create one base ReadOptions for a particular usecase and clone -> change the iterate_upper_bound on each slice operation. Shallow copy seemed cleanest.
* Hold a reference to the upper bound slice on ReadOptions, in contrast to Snapshot.

Signed a Facebook CLA this morning.
Closes https://github.com/facebook/rocksdb/pull/2872

Differential Revision: D5824446

Pulled By: sagar0

fbshipit-source-id: 74fc51313a10a81ecd348625e2a50ca5b7766888

java/rocksjni/options.cc
java/src/main/java/org/rocksdb/ReadOptions.java
java/src/main/java/org/rocksdb/Slice.java
java/src/test/java/org/rocksdb/ReadOptionsTest.java

index 8194abaf6b61633cf28a3aa5780d9f0a289e89f9..f52171244c65e66932d274ee75b06016ac0959ff 100644 (file)
@@ -5728,6 +5728,19 @@ jlong Java_org_rocksdb_ReadOptions_newReadOptions(
   return reinterpret_cast<jlong>(read_options);
 }
 
+/*
+ * Class:     org_rocksdb_ReadOptions
+ * Method:    copyReadOptions
+ * Signature: (J)J
+ */
+jlong Java_org_rocksdb_ReadOptions_copyReadOptions(
+    JNIEnv* env, jclass jcls, jlong jhandle) {
+  auto old_read_opt = reinterpret_cast<rocksdb::ReadOptions*>(jhandle);
+  auto new_read_opt = new rocksdb::ReadOptions();
+  *new_read_opt = *old_read_opt;
+  return reinterpret_cast<jlong>(new_read_opt);
+}
+
 /*
  * Class:     org_rocksdb_ReadOptions
  * Method:    disposeInternal
@@ -6003,6 +6016,29 @@ void Java_org_rocksdb_ReadOptions_setReadTier(
       static_cast<rocksdb::ReadTier>(jread_tier);
 }
 
+/*
+ * Class:     org_rocksdb_ReadOptions
+ * Method:    setIterateUpperBound
+ * Signature: (JJ)I
+ */
+void Java_org_rocksdb_ReadOptions_setIterateUpperBound(
+    JNIEnv* env, jobject jobj, jlong jhandle, jlong jupper_bound_slice_handle) {
+  reinterpret_cast<rocksdb::ReadOptions*>(jhandle)->iterate_upper_bound =
+      reinterpret_cast<rocksdb::Slice*>(jupper_bound_slice_handle);
+}
+
+/*
+ * Class:     org_rocksdb_ReadOptions
+ * Method:    iterateUpperBound
+ * Signature: (J)J
+ */
+jlong Java_org_rocksdb_ReadOptions_iterateUpperBound(
+    JNIEnv* env, jobject jobj, jlong jhandle) {
+  auto& upper_bound_slice_handle =
+      reinterpret_cast<rocksdb::ReadOptions*>(jhandle)->iterate_upper_bound;
+  return reinterpret_cast<jlong>(upper_bound_slice_handle);
+}
+
 /////////////////////////////////////////////////////////////////////
 // rocksdb::ComparatorOptions
 
index 9d7b999561b37f81d38920da4eab546ee65e888f..30c67a0e8e1e644e7274b3af276eaa1bca112e73 100644 (file)
@@ -16,6 +16,19 @@ public class ReadOptions extends RocksObject {
     super(newReadOptions());
   }
 
+  /**
+   * Copy constructor.
+   *
+   * NOTE: This does a shallow copy, which means snapshot, iterate_upper_bound
+   * and other pointers will be cloned!
+   *
+   * @param other The ReadOptions to copy.
+   */
+  public ReadOptions(ReadOptions other) {
+    super(copyReadOptions(other.nativeHandle_));
+    iterateUpperBoundSlice_ = other.iterateUpperBoundSlice_;
+  }
+
   /**
    * If true, all data read from underlying storage will be
    * verified against corresponding checksums.
@@ -363,7 +376,62 @@ public class ReadOptions extends RocksObject {
     return this;
   }
 
+  /**
+   * Defines the extent upto which the forward iterator can returns entries.
+   * Once the bound is reached, Valid() will be false. iterate_upper_bound
+   * is exclusive ie the bound value is not a valid entry. If
+   * iterator_extractor is not null, the Seek target and iterator_upper_bound
+   * need to have the same prefix. This is because ordering is not guaranteed
+   * outside of prefix domain. There is no lower bound on the iterator.
+   *
+   * Default: nullptr
+   *
+   * @param iterateUpperBound Slice representing the upper bound
+   * @return the reference to the current ReadOptions.
+   */
+  public ReadOptions setIterateUpperBound(final Slice iterateUpperBound) {
+    assert(isOwningHandle());
+    if (iterateUpperBound != null) {
+      // Hold onto a reference so it doesn't get garbaged collected out from under us.
+      iterateUpperBoundSlice_ = iterateUpperBound;
+      setIterateUpperBound(nativeHandle_, iterateUpperBoundSlice_.getNativeHandle());
+    }
+    return this;
+  }
+
+  /**
+   * Defines the extent upto which the forward iterator can returns entries.
+   * Once the bound is reached, Valid() will be false. iterate_upper_bound
+   * is exclusive ie the bound value is not a valid entry. If
+   * iterator_extractor is not null, the Seek target and iterator_upper_bound
+   * need to have the same prefix. This is because ordering is not guaranteed
+   * outside of prefix domain. There is no lower bound on the iterator.
+   *
+   * Default: nullptr
+   *
+   * @return Slice representing current iterate_upper_bound setting, or null if
+   *         one does not exist.
+   */
+  public Slice iterateUpperBound() {
+    assert(isOwningHandle());
+    long upperBoundSliceHandle = iterateUpperBound(nativeHandle_);
+    if (upperBoundSliceHandle != 0) {
+      // Disown the new slice - it's owned by the C++ side of the JNI boundary
+      // from the perspective of this method.
+      return new Slice(upperBoundSliceHandle, false);
+    }
+    return null;
+  }
+
+  // Hold a reference to any iterate upper bound that was set on this object
+  // until we're destroyed or it's overwritten.  That way the caller can freely
+  // leave scope without us losing the Java Slice object, which during close()
+  // would also reap its associated rocksdb::Slice native object since it's
+  // possibly (likely) to be an owning handle.
+  protected Slice iterateUpperBoundSlice_;
+
   private native static long newReadOptions();
+  private native static long copyReadOptions(long handle);
   private native boolean verifyChecksums(long handle);
   private native void setVerifyChecksums(long handle, boolean verifyChecksums);
   private native boolean fillCache(long handle);
@@ -391,6 +459,9 @@ public class ReadOptions extends RocksObject {
   private native boolean ignoreRangeDeletions(final long handle);
   private native void setIgnoreRangeDeletions(final long handle,
       final boolean ignoreRangeDeletions);
+  private native void setIterateUpperBound(final long handle,
+      final long upperBoundSliceHandle);
+  private native long iterateUpperBound(final long handle);
 
   @Override protected final native void disposeInternal(final long handle);
 
index a122c3769d8419f5b508799640059093a27e8e0f..08a940c3f3fc49adfafcbca342c8ae456a2d54b5 100644 (file)
@@ -39,6 +39,29 @@ public class Slice extends AbstractSlice<byte[]> {
     super();
   }
 
+  /**
+   * <p>Package-private Slice constructor which is used to construct
+   * Slice instances from C++ side. As the reference to this
+   * object is also managed from C++ side the handle will be disowned.</p>
+   *
+   * @param nativeHandle address of native instance.
+   */
+  Slice(final long nativeHandle) {
+    this(nativeHandle, false);
+  }
+
+  /**
+   * <p>Package-private Slice constructor which is used to construct
+   * Slice instances using a handle. </p>
+   *
+   * @param nativeHandle address of native instance.
+   * @param owningNativeHandle whether to own this reference from the C++ side or not
+   */
+  Slice(final long nativeHandle, final boolean owningNativeHandle) {
+    super();
+    setNativeHandle(nativeHandle, owningNativeHandle);
+  }
+
   /**
    * <p>Constructs a slice where the data is taken from
    * a String.</p>
index da048c4431e181dd90ce5e15fe8a843102105e70..f7d799909d95b63e526522750e818d193eea1bb6 100644 (file)
@@ -5,6 +5,7 @@
 
 package org.rocksdb;
 
+import java.util.Arrays;
 import java.util.Random;
 
 import org.junit.ClassRule;
@@ -127,6 +128,35 @@ public class ReadOptionsTest {
     }
   }
 
+  @Test
+  public void iterateUpperBound() {
+    try (final ReadOptions opt = new ReadOptions()) {
+      Slice upperBound = buildRandomSlice();
+      opt.setIterateUpperBound(upperBound);
+      assertThat(Arrays.equals(upperBound.data(), opt.iterateUpperBound().data())).isTrue();
+    }
+  }
+
+  @Test
+  public void iterateUpperBoundNull() {
+    try (final ReadOptions opt = new ReadOptions()) {
+      assertThat(opt.iterateUpperBound()).isNull();
+    }
+  }
+
+  @Test
+  public void copyConstructor() {
+    try (final ReadOptions opt = new ReadOptions()) {
+      opt.setVerifyChecksums(false);
+      opt.setFillCache(false);
+      opt.setIterateUpperBound(buildRandomSlice());
+      ReadOptions other = new ReadOptions(opt);
+      assertThat(opt.verifyChecksums()).isEqualTo(other.verifyChecksums());
+      assertThat(opt.fillCache()).isEqualTo(other.fillCache());
+      assertThat(Arrays.equals(opt.iterateUpperBound().data(), other.iterateUpperBound().data())).isTrue();
+    }
+  }
+
   @Test
   public void failSetVerifyChecksumUninitialized() {
     try (final ReadOptions readOptions =
@@ -191,6 +221,22 @@ public class ReadOptionsTest {
     }
   }
 
+  @Test
+  public void failSetIterateUpperBoundUninitialized() {
+    try (final ReadOptions readOptions =
+             setupUninitializedReadOptions(exception)) {
+      readOptions.setIterateUpperBound(null);
+    }
+  }
+
+  @Test
+  public void failIterateUpperBoundUninitialized() {
+    try (final ReadOptions readOptions =
+             setupUninitializedReadOptions(exception)) {
+      readOptions.iterateUpperBound();
+    }
+  }
+
   private ReadOptions setupUninitializedReadOptions(
       ExpectedException exception) {
     final ReadOptions readOptions = new ReadOptions();
@@ -198,4 +244,12 @@ public class ReadOptionsTest {
     exception.expect(AssertionError.class);
     return readOptions;
   }
+
+  private Slice buildRandomSlice() {
+    final Random rand = new Random();
+    byte[] sliceBytes = new byte[rand.nextInt(100) + 1];
+    rand.nextBytes(sliceBytes);
+    return new Slice(sliceBytes);
+  }
+
 }