Promote CompactionFilter* accessors to ColumnFamilyOptionsInterface (#3461)

Summary:
When adding CompactionFilter and CompactionFilterFactory settings to the Java layer, ColumnFamilyOptions was modified directly instead of ColumnFamilyOptionsInterface. This meant that the old-stye Options monolith was left behind.

This patch fixes that, by:
- promoting the CompactionFilter + CompactionFilterFactory setters from ColumnFamilyOptions -> ColumnFamilyOptionsInterface
- adding getters in ColumnFamilyOptionsInterface
- implementing setters in Options
- implementing getters in both ColumnFamilyOptions and Options
- adding testcases
- reusing a test CompactionFilterFactory by moving it to a common location
Pull Request resolved: https://github.com/facebook/rocksdb/pull/3461

Differential Revision: D13278788

Pulled By: sagar0

fbshipit-source-id: 72602c6eb97dc80734e718abb5e2e9958d3c753b

java/rocksjni/options.cc

@@ -233,6 +233,34 @@ void Java_org_rocksdb_Options_setMergeOperator(JNIEnv* /*env*/,
           mergeOperatorHandle));
 }
 
+/*
+ * Class:     org_rocksdb_Options
+ * Method:    setCompactionFilterHandle
+ * Signature: (JJ)V
+ */
+void Java_org_rocksdb_Options_setCompactionFilterHandle(
+    JNIEnv* /*env*/, jobject /*jobj*/, jlong jopt_handle,
+    jlong jcompactionfilter_handle) {
+  reinterpret_cast<rocksdb::Options*>(jopt_handle)->
+      compaction_filter = reinterpret_cast<rocksdb::CompactionFilter*>
+  (jcompactionfilter_handle);
+}
+
+/*
+ * Class:     org_rocksdb_Options
+ * Method:    setCompactionFilterFactoryHandle
+ * Signature: (JJ)V
+ */
+void JNICALL Java_org_rocksdb_Options_setCompactionFilterFactoryHandle(
+    JNIEnv* /* env */, jobject /* jobj */, jlong jopt_handle,
+    jlong jcompactionfilterfactory_handle) {
+  auto* cff_factory =
+      reinterpret_cast<std::shared_ptr<rocksdb::CompactionFilterFactory> *>(
+          jcompactionfilterfactory_handle);
+  reinterpret_cast<rocksdb::Options*>(jopt_handle)->
+      compaction_filter_factory = *cff_factory;
+}
+
 /*
  * Class:     org_rocksdb_Options
  * Method:    setWriteBufferSize

java/src/main/java/org/rocksdb/ColumnFamilyOptions.java

@@ -186,25 +186,7 @@ public class ColumnFamilyOptions extends RocksObject
     return this;
   }
 
-  /**
-   * A single CompactionFilter instance to call into during compaction.
-   * Allows an application to modify/delete a key-value during background
-   * compaction.
-   *
-   * If the client requires a new compaction filter to be used for different
-   * compaction runs, it can specify call
-   * {@link #setCompactionFilterFactory(AbstractCompactionFilterFactory)}
-   * instead.
-   *
-   * The client should specify only set one of the two.
-   * {@link #setCompactionFilter(AbstractCompactionFilter)} takes precedence
-   * over {@link #setCompactionFilterFactory(AbstractCompactionFilterFactory)}
-   * if the client specifies both.
-   *
-   * @param compactionFilter The compaction filter called during compaction.
-   * @return the reference to {@link org.rocksdb.ColumnFamilyOptions instance}.
-   */
-  //TODO(AR) need to set a note on the concurrency of the compaction filter used from this method
+  @Override
   public ColumnFamilyOptions setCompactionFilter(
         final AbstractCompactionFilter<? extends AbstractSlice<?>>
             compactionFilter) {
@@ -213,18 +195,13 @@ public class ColumnFamilyOptions extends RocksObject
     return this;
   }
 
-  /**
-   * This is a factory that provides {@link AbstractCompactionFilter} objects
-   * which allow an application to modify/delete a key-value during background
-   * compaction.
-   *
-   * A new filter will be created on each compaction run.  If multithreaded
-   * compaction is being used, each created CompactionFilter will only be used
-   * from a single thread and so does not need to be thread-safe.
-   *
-   * @param compactionFilterFactory The factory used for creating a new filter on each compaction run.
-   * @return the reference to {@link org.rocksdb.ColumnFamilyOptions instance}.
-   */
+  @Override
+  public AbstractCompactionFilter<? extends AbstractSlice<?>> compactionFilter() {
+    assert (isOwningHandle());
+    return compactionFilter_;
+  }
+
+  @Override
   public ColumnFamilyOptions setCompactionFilterFactory(final AbstractCompactionFilterFactory<? extends AbstractCompactionFilter<?>> compactionFilterFactory) {
     assert (isOwningHandle());
     setCompactionFilterFactoryHandle(nativeHandle_, compactionFilterFactory.nativeHandle_);
@@ -232,6 +209,12 @@ public class ColumnFamilyOptions extends RocksObject
     return this;
   }
 
+  @Override
+  public AbstractCompactionFilterFactory<? extends AbstractCompactionFilter<?>> compactionFilterFactory() {
+    assert (isOwningHandle());
+    return compactionFilterFactory_;
+  }
+
   @Override
   public ColumnFamilyOptions setWriteBufferSize(final long writeBufferSize) {
     assert(isOwningHandle());
@@ -967,7 +950,7 @@ public class ColumnFamilyOptions extends RocksObject
   private TableFormatConfig tableFormatConfig_;
   private AbstractComparator<? extends AbstractSlice<?>> comparator_;
   private AbstractCompactionFilter<? extends AbstractSlice<?>> compactionFilter_;
-  AbstractCompactionFilterFactory<? extends AbstractCompactionFilter<?>>
+  private AbstractCompactionFilterFactory<? extends AbstractCompactionFilter<?>>
       compactionFilterFactory_;
   private CompactionOptionsUniversal compactionOptionsUniversal_;
   private CompactionOptionsFIFO compactionOptionsFIFO_;

java/src/main/java/org/rocksdb/ColumnFamilyOptionsInterface.java

@@ -151,6 +151,60 @@ public interface ColumnFamilyOptionsInterface
    */
   T setMergeOperator(MergeOperator mergeOperator);
 
+  /**
+   * A single CompactionFilter instance to call into during compaction.
+   * Allows an application to modify/delete a key-value during background
+   * compaction.
+   *
+   * If the client requires a new compaction filter to be used for different
+   * compaction runs, it can specify call
+   * {@link #setCompactionFilterFactory(AbstractCompactionFilterFactory)}
+   * instead.
+   *
+   * The client should specify only set one of the two.
+   * {@link #setCompactionFilter(AbstractCompactionFilter)} takes precedence
+   * over {@link #setCompactionFilterFactory(AbstractCompactionFilterFactory)}
+   * if the client specifies both.
+   *
+   * If multithreaded compaction is being used, the supplied CompactionFilter
+   * instance may be used from different threads concurrently and so should be thread-safe.
+   *
+   * @param compactionFilter {@link AbstractCompactionFilter} instance.
+   * @return the instance of the current object.
+   */
+  T setCompactionFilter(
+          final AbstractCompactionFilter<? extends AbstractSlice<?>> compactionFilter);
+
+  /**
+   * Accessor for the CompactionFilter instance in use.
+   *
+   * @return  Reference to the CompactionFilter, or null if one hasn't been set.
+   */
+  AbstractCompactionFilter<? extends AbstractSlice<?>> compactionFilter();
+
+  /**
+   * This is a factory that provides {@link AbstractCompactionFilter} objects
+   * which allow an application to modify/delete a key-value during background
+   * compaction.
+   *
+   * A new filter will be created on each compaction run.  If multithreaded
+   * compaction is being used, each created CompactionFilter will only be used
+   * from a single thread and so does not need to be thread-safe.
+   *
+   * @param compactionFilterFactory {@link AbstractCompactionFilterFactory} instance.
+   * @return the instance of the current object.
+   */
+  T setCompactionFilterFactory(
+          final AbstractCompactionFilterFactory<? extends AbstractCompactionFilter<?>>
+                  compactionFilterFactory);
+
+  /**
+   * Accessor for the CompactionFilterFactory instance in use.
+   *
+   * @return  Reference to the CompactionFilterFactory, or null if one hasn't been set.
+   */
+  AbstractCompactionFilterFactory<? extends AbstractCompactionFilter<?>> compactionFilterFactory();
+
   /**
    * This prefix-extractor uses the first n bytes of a key as its prefix.
    *

java/src/main/java/org/rocksdb/Options.java

@@ -66,6 +66,8 @@ public class Options extends RocksObject
     this.tableFormatConfig_ = other.tableFormatConfig_;
     this.rateLimiter_ = other.rateLimiter_;
     this.comparator_ = other.comparator_;
+    this.compactionFilter_ = other.compactionFilter_;
+    this.compactionFilterFactory_ = other.compactionFilterFactory_;
     this.compactionOptionsUniversal_ = other.compactionOptionsUniversal_;
     this.compactionOptionsFIFO_ = other.compactionOptionsFIFO_;
     this.compressionOptions_ = other.compressionOptions_;
@@ -214,6 +216,35 @@ public class Options extends RocksObject
     return this;
   }
 
+  @Override
+  public Options setCompactionFilter(
+          final AbstractCompactionFilter<? extends AbstractSlice<?>>
+                  compactionFilter) {
+    setCompactionFilterHandle(nativeHandle_, compactionFilter.nativeHandle_);
+    compactionFilter_ = compactionFilter;
+    return this;
+  }
+
+  @Override
+  public AbstractCompactionFilter<? extends AbstractSlice<?>> compactionFilter() {
+    assert (isOwningHandle());
+    return compactionFilter_;
+  }
+
+  @Override
+  public Options setCompactionFilterFactory(final AbstractCompactionFilterFactory<? extends AbstractCompactionFilter<?>> compactionFilterFactory) {
+    assert (isOwningHandle());
+    setCompactionFilterFactoryHandle(nativeHandle_, compactionFilterFactory.nativeHandle_);
+    compactionFilterFactory_ = compactionFilterFactory;
+    return this;
+  }
+
+  @Override
+  public AbstractCompactionFilterFactory<? extends AbstractCompactionFilter<?>> compactionFilterFactory() {
+    assert (isOwningHandle());
+    return compactionFilterFactory_;
+  }
+
   @Override
   public Options setWriteBufferSize(final long writeBufferSize) {
     assert(isOwningHandle());
@@ -1787,6 +1818,10 @@ public class Options extends RocksObject
       long handle, String name);
   private native void setMergeOperator(
       long handle, long mergeOperatorHandle);
+  private native void setCompactionFilterHandle(
+          long handle, long compactionFilterHandle);
+  private native void setCompactionFilterFactoryHandle(
+          long handle, long compactionFilterFactoryHandle);
   private native void setWriteBufferSize(long handle, long writeBufferSize)
       throws IllegalArgumentException;
   private native long writeBufferSize(long handle);
@@ -1922,6 +1957,9 @@ public class Options extends RocksObject
   private TableFormatConfig tableFormatConfig_;
   private RateLimiter rateLimiter_;
   private AbstractComparator<? extends AbstractSlice<?>> comparator_;
+  private AbstractCompactionFilter<? extends AbstractSlice<?>> compactionFilter_;
+  private AbstractCompactionFilterFactory<? extends AbstractCompactionFilter<?>>
+          compactionFilterFactory_;
   private CompactionOptionsUniversal compactionOptionsUniversal_;
   private CompactionOptionsFIFO compactionOptionsFIFO_;
   private CompressionOptions compressionOptions_;

java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java

@@ -7,6 +7,7 @@ package org.rocksdb;
 
 import org.junit.ClassRule;
 import org.junit.Test;
+import org.rocksdb.test.RemoveEmptyValueCompactionFilterFactory;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -576,4 +577,23 @@ public class ColumnFamilyOptionsTest {
           isEqualTo(booleanValue);
     }
   }
+
+  @Test
+  public void compactionFilter() {
+    try(final ColumnFamilyOptions options = new ColumnFamilyOptions();
+        final RemoveEmptyValueCompactionFilter cf = new RemoveEmptyValueCompactionFilter()) {
+      options.setCompactionFilter(cf);
+      assertThat(options.compactionFilter()).isEqualTo(cf);
+    }
+  }
+
+  @Test
+  public void compactionFilterFactory() {
+    try(final ColumnFamilyOptions options = new ColumnFamilyOptions();
+        final RemoveEmptyValueCompactionFilterFactory cff = new RemoveEmptyValueCompactionFilterFactory()) {
+      options.setCompactionFilterFactory(cff);
+      assertThat(options.compactionFilterFactory()).isEqualTo(cff);
+    }
+  }
+
 }

java/src/test/java/org/rocksdb/CompactionFilterFactoryTest.java

@@ -8,6 +8,7 @@ package org.rocksdb;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.rocksdb.test.RemoveEmptyValueCompactionFilterFactory;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -63,16 +64,4 @@ public class CompactionFilterFactoryTest {
       }
     }
   }
-
-  private static class RemoveEmptyValueCompactionFilterFactory extends AbstractCompactionFilterFactory<RemoveEmptyValueCompactionFilter> {
-    @Override
-    public RemoveEmptyValueCompactionFilter createCompactionFilter(final AbstractCompactionFilter.Context context) {
-      return new RemoveEmptyValueCompactionFilter();
-    }
-
-    @Override
-    public String name() {
-      return "RemoveEmptyValueCompactionFilterFactory";
-    }
-  }
 }

java/src/test/java/org/rocksdb/OptionsTest.java

@@ -13,6 +13,7 @@ import java.util.Random;
 
 import org.junit.ClassRule;
 import org.junit.Test;
+import org.rocksdb.test.RemoveEmptyValueCompactionFilterFactory;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -1142,4 +1143,23 @@ public class OptionsTest {
           isEqualTo(booleanValue);
     }
   }
+
+  @Test
+  public void compactionFilter() {
+    try(final Options options = new Options();
+        final RemoveEmptyValueCompactionFilter cf = new RemoveEmptyValueCompactionFilter()) {
+      options.setCompactionFilter(cf);
+      assertThat(options.compactionFilter()).isEqualTo(cf);
+    }
+  }
+
+  @Test
+  public void compactionFilterFactory() {
+    try(final Options options = new Options();
+        final RemoveEmptyValueCompactionFilterFactory cff = new RemoveEmptyValueCompactionFilterFactory()) {
+      options.setCompactionFilterFactory(cff);
+      assertThat(options.compactionFilterFactory()).isEqualTo(cff);
+    }
+  }
+
 }

java/src/test/java/org/rocksdb/test/RemoveEmptyValueCompactionFilterFactory.java

@@ -0,0 +1,20 @@
+package org.rocksdb.test;
+
+import org.rocksdb.AbstractCompactionFilter;
+import org.rocksdb.AbstractCompactionFilterFactory;
+import org.rocksdb.RemoveEmptyValueCompactionFilter;
+
+/**
+ * Simple CompactionFilterFactory class used in tests. Generates RemoveEmptyValueCompactionFilters.
+ */
+public class RemoveEmptyValueCompactionFilterFactory extends AbstractCompactionFilterFactory<RemoveEmptyValueCompactionFilter> {
+    @Override
+    public RemoveEmptyValueCompactionFilter createCompactionFilter(final AbstractCompactionFilter.Context context) {
+        return new RemoveEmptyValueCompactionFilter();
+    }
+
+    @Override
+    public String name() {
+        return "RemoveEmptyValueCompactionFilterFactory";
+    }
+}