From a247617e6f316dbdd7273d8ee67e24c42ba04fec Mon Sep 17 00:00:00 2001 From: Ben Clay Date: Fri, 2 Feb 2018 10:46:41 -0800 Subject: [PATCH] Java: Add copy constructors for various option classes Summary: Add Java-side copy constructors for: - Options - DBOptions - ColumnFamilyOptions - WriteOptions along with unit tests to assert the copy worked. NOTE: Unit tests are failing in travis but it looks like a global timeout issue. These tests pass. Closes https://github.com/facebook/rocksdb/pull/3450 Differential Revision: D6874425 Pulled By: sagar0 fbshipit-source-id: 5bde68ea5b5225e071faea2628bf8bbf10bd65ab --- java/rocksjni/options.cc | 55 +++++++++++++++++-- .../java/org/rocksdb/ColumnFamilyOptions.java | 24 +++++++- java/src/main/java/org/rocksdb/DBOptions.java | 18 ++++++ java/src/main/java/org/rocksdb/Options.java | 23 ++++++++ .../main/java/org/rocksdb/ReadOptions.java | 3 + .../main/java/org/rocksdb/WriteOptions.java | 14 +++++ .../org/rocksdb/ColumnFamilyOptionsTest.java | 12 ++++ .../test/java/org/rocksdb/DBOptionsTest.java | 13 +++++ .../test/java/org/rocksdb/OptionsTest.java | 12 ++++ .../java/org/rocksdb/WriteOptionsTest.java | 19 +++++++ 10 files changed, 188 insertions(+), 5 deletions(-) diff --git a/java/rocksjni/options.cc b/java/rocksjni/options.cc index 45a5de865..8a55275a6 100644 --- a/java/rocksjni/options.cc +++ b/java/rocksjni/options.cc @@ -59,6 +59,18 @@ jlong Java_org_rocksdb_Options_newOptions__JJ(JNIEnv* env, jclass jcls, return reinterpret_cast(op); } +/* + * Class: org_rocksdb_Options + * Method: copyOptions + * Signature: (J)J + */ +jlong Java_org_rocksdb_Options_copyOptions(JNIEnv* env, jclass jcls, + jlong jhandle) { + auto new_opt = new rocksdb::Options( + *(reinterpret_cast(jhandle))); + return reinterpret_cast(new_opt); +} + /* * Class: org_rocksdb_Options * Method: disposeInternal @@ -2848,6 +2860,18 @@ jlong Java_org_rocksdb_ColumnFamilyOptions_newColumnFamilyOptions( return reinterpret_cast(op); } +/* + * Class: org_rocksdb_ColumnFamilyOptions + * Method: copyColumnFamilyOptions + * Signature: (J)J + */ +jlong Java_org_rocksdb_ColumnFamilyOptions_copyColumnFamilyOptions( + JNIEnv* env, jclass jcls, jlong jhandle) { + auto new_opt = new rocksdb::ColumnFamilyOptions( + *(reinterpret_cast(jhandle))); + return reinterpret_cast(new_opt); +} + /* * Class: org_rocksdb_ColumnFamilyOptions * Method: getColumnFamilyOptionsFromProps @@ -4161,6 +4185,18 @@ jlong Java_org_rocksdb_DBOptions_newDBOptions(JNIEnv* env, return reinterpret_cast(dbop); } +/* + * Class: org_rocksdb_DBOptions + * Method: copyDBOptions + * Signature: (J)J + */ +jlong Java_org_rocksdb_DBOptions_copyDBOptions(JNIEnv* env, jclass jcls, + jlong jhandle) { + auto new_opt = new rocksdb::DBOptions( + *(reinterpret_cast(jhandle))); + return reinterpret_cast(new_opt); +} + /* * Class: org_rocksdb_DBOptions * Method: getDBOptionsFromProps @@ -5690,6 +5726,18 @@ jlong Java_org_rocksdb_WriteOptions_newWriteOptions( return reinterpret_cast(op); } +/* + * Class: org_rocksdb_WriteOptions + * Method: copyWriteOptions + * Signature: (J)J + */ +jlong Java_org_rocksdb_WriteOptions_copyWriteOptions( + JNIEnv* env, jclass jcls, jlong jhandle) { + auto new_opt = new rocksdb::WriteOptions( + *(reinterpret_cast(jhandle))); + return reinterpret_cast(new_opt); +} + /* * Class: org_rocksdb_WriteOptions * Method: disposeInternal @@ -5808,10 +5856,9 @@ jlong Java_org_rocksdb_ReadOptions_newReadOptions( */ jlong Java_org_rocksdb_ReadOptions_copyReadOptions( JNIEnv* env, jclass jcls, jlong jhandle) { - auto old_read_opt = reinterpret_cast(jhandle); - auto new_read_opt = new rocksdb::ReadOptions(); - *new_read_opt = *old_read_opt; - return reinterpret_cast(new_read_opt); + auto new_opt = new rocksdb::ReadOptions( + *(reinterpret_cast(jhandle))); + return reinterpret_cast(new_opt); } /* diff --git a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java index e28a1d80d..b3890ed81 100644 --- a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java +++ b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java @@ -27,12 +27,32 @@ public class ColumnFamilyOptions extends RocksObject * Construct ColumnFamilyOptions. * * This constructor will create (by allocating a block of memory) - * an {@code rocksdb::DBOptions} in the c++ side. + * an {@code rocksdb::ColumnFamilyOptions} in the c++ side. */ public ColumnFamilyOptions() { super(newColumnFamilyOptions()); } + /** + * Copy constructor for ColumnFamilyOptions. + * + * NOTE: This does a shallow copy, which means comparator, merge_operator, compaction_filter, + * compaction_filter_factory and other pointers will be cloned! + * + * @param other The ColumnFamilyOptions to copy. + */ + public ColumnFamilyOptions(ColumnFamilyOptions other) { + super(copyColumnFamilyOptions(other.nativeHandle_)); + this.memTableConfig_ = other.memTableConfig_; + this.tableFormatConfig_ = other.tableFormatConfig_; + this.comparator_ = other.comparator_; + this.compactionFilter_ = other.compactionFilter_; + this.compactionFilterFactory_ = other.compactionFilterFactory_; + this.compactionOptionsUniversal_ = other.compactionOptionsUniversal_; + this.compactionOptionsFIFO_ = other.compactionOptionsFIFO_; + this.compressionOptions_ = other.compressionOptions_; + } + /** *

Method to get a options instance by using pre-configured * property values. If one or many values are undefined in @@ -783,6 +803,7 @@ public class ColumnFamilyOptions extends RocksObject String optString); private static native long newColumnFamilyOptions(); + private static native long copyColumnFamilyOptions(long handle); @Override protected final native void disposeInternal(final long handle); private native void optimizeForSmallDb(final long handle); @@ -934,6 +955,7 @@ public class ColumnFamilyOptions extends RocksObject private native boolean forceConsistencyChecks(final long handle); // instance variables + // NOTE: If you add new member variables, please update the copy constructor above! private MemTableConfig memTableConfig_; private TableFormatConfig tableFormatConfig_; private AbstractComparator> comparator_; diff --git a/java/src/main/java/org/rocksdb/DBOptions.java b/java/src/main/java/org/rocksdb/DBOptions.java index 0e9bb1619..d06422136 100644 --- a/java/src/main/java/org/rocksdb/DBOptions.java +++ b/java/src/main/java/org/rocksdb/DBOptions.java @@ -32,6 +32,22 @@ public class DBOptions numShardBits_ = DEFAULT_NUM_SHARD_BITS; } + /** + * Copy constructor for DBOptions. + * + * NOTE: This does a shallow copy, which means env, rate_limiter, sst_file_manager, + * info_log and other pointers will be cloned! + * + * @param other The DBOptions to copy. + */ + public DBOptions(DBOptions other) { + super(copyDBOptions(other.nativeHandle_)); + this.env_ = other.env_; + this.numShardBits_ = other.numShardBits_; + this.rateLimiter_ = other.rateLimiter_; + this.rowCache_ = other.rowCache_; + } + /** *

Method to get a options instance by using pre-configured * property values. If one or many values are undefined in @@ -954,6 +970,7 @@ public class DBOptions String optString); private native static long newDBOptions(); + private native static long copyDBOptions(long handle); @Override protected final native void disposeInternal(final long handle); private native void optimizeForSmallDb(final long handle); @@ -1127,6 +1144,7 @@ public class DBOptions private native boolean avoidFlushDuringShutdown(final long handle); // instance variables + // NOTE: If you add new member variables, please update the copy constructor above! private Env env_; private int numShardBits_; private RateLimiter rateLimiter_; diff --git a/java/src/main/java/org/rocksdb/Options.java b/java/src/main/java/org/rocksdb/Options.java index 3006304f3..4393caa75 100644 --- a/java/src/main/java/org/rocksdb/Options.java +++ b/java/src/main/java/org/rocksdb/Options.java @@ -51,6 +51,27 @@ public class Options extends RocksObject env_ = Env.getDefault(); } + /** + * Copy constructor for ColumnFamilyOptions. + * + * NOTE: This does a shallow copy, which means comparator, merge_operator + * and other pointers will be cloned! + * + * @param other The Options to copy. + */ + public Options(Options other) { + super(copyOptions(other.nativeHandle_)); + this.env_ = other.env_; + this.memTableConfig_ = other.memTableConfig_; + this.tableFormatConfig_ = other.tableFormatConfig_; + this.rateLimiter_ = other.rateLimiter_; + this.comparator_ = other.comparator_; + this.compactionOptionsUniversal_ = other.compactionOptionsUniversal_; + this.compactionOptionsFIFO_ = other.compactionOptionsFIFO_; + this.compressionOptions_ = other.compressionOptions_; + this.rowCache_ = other.rowCache_; + } + @Override public Options setIncreaseParallelism(final int totalThreads) { assert(isOwningHandle()); @@ -1548,6 +1569,7 @@ public class Options extends RocksObject private native static long newOptions(); private native static long newOptions(long dbOptHandle, long cfOptHandle); + private native static long copyOptions(long handle); @Override protected final native void disposeInternal(final long handle); private native void setEnv(long optHandle, long envHandle); private native void prepareForBulkLoad(long handle); @@ -1868,6 +1890,7 @@ public class Options extends RocksObject private native boolean forceConsistencyChecks(final long handle); // instance variables + // NOTE: If you add new member variables, please update the copy constructor above! private Env env_; private MemTableConfig memTableConfig_; private TableFormatConfig tableFormatConfig_; diff --git a/java/src/main/java/org/rocksdb/ReadOptions.java b/java/src/main/java/org/rocksdb/ReadOptions.java index 30c67a0e8..be8aec6b3 100644 --- a/java/src/main/java/org/rocksdb/ReadOptions.java +++ b/java/src/main/java/org/rocksdb/ReadOptions.java @@ -423,6 +423,9 @@ public class ReadOptions extends RocksObject { return null; } + // instance variables + // NOTE: If you add new member variables, please update the copy constructor above! + // // 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() diff --git a/java/src/main/java/org/rocksdb/WriteOptions.java b/java/src/main/java/org/rocksdb/WriteOptions.java index b9e8ad81c..f3c5aa667 100644 --- a/java/src/main/java/org/rocksdb/WriteOptions.java +++ b/java/src/main/java/org/rocksdb/WriteOptions.java @@ -20,6 +20,19 @@ public class WriteOptions extends RocksObject { } + /** + * Copy constructor for WriteOptions. + * + * NOTE: This does a shallow copy, which means comparator, merge_operator, compaction_filter, + * compaction_filter_factory and other pointers will be cloned! + * + * @param other The ColumnFamilyOptions to copy. + */ + public WriteOptions(WriteOptions other) { + super(copyWriteOptions(other.nativeHandle_)); + } + + /** * If true, the write will be flushed from the operating system * buffer cache (by calling WritableFile::Sync()) before the write @@ -145,6 +158,7 @@ public class WriteOptions extends RocksObject { } private native static long newWriteOptions(); + private native static long copyWriteOptions(long handle); private native void setSync(long handle, boolean flag); private native boolean sync(long handle); private native void setDisableWAL(long handle, boolean flag); diff --git a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java index 75749437b..43c17d52e 100644 --- a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java +++ b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java @@ -24,6 +24,18 @@ public class ColumnFamilyOptionsTest { public static final Random rand = PlatformRandomHelper. getPlatformSpecificRandomFactory(); + @Test + public void copyConstructor() { + ColumnFamilyOptions origOpts = new ColumnFamilyOptions(); + origOpts.setNumLevels(rand.nextInt(8)); + origOpts.setTargetFileSizeMultiplier(rand.nextInt(100)); + origOpts.setLevel0StopWritesTrigger(rand.nextInt(50)); + ColumnFamilyOptions copyOpts = new ColumnFamilyOptions(origOpts); + assertThat(origOpts.numLevels()).isEqualTo(copyOpts.numLevels()); + assertThat(origOpts.targetFileSizeMultiplier()).isEqualTo(copyOpts.targetFileSizeMultiplier()); + assertThat(origOpts.level0StopWritesTrigger()).isEqualTo(copyOpts.level0StopWritesTrigger()); + } + @Test public void getColumnFamilyOptionsFromProps() { Properties properties = new Properties(); diff --git a/java/src/test/java/org/rocksdb/DBOptionsTest.java b/java/src/test/java/org/rocksdb/DBOptionsTest.java index 416669e18..05c9e70c4 100644 --- a/java/src/test/java/org/rocksdb/DBOptionsTest.java +++ b/java/src/test/java/org/rocksdb/DBOptionsTest.java @@ -22,6 +22,19 @@ public class DBOptionsTest { public static final Random rand = PlatformRandomHelper. getPlatformSpecificRandomFactory(); + @Test + public void copyConstructor() { + DBOptions origOpts = new DBOptions(); + origOpts.setCreateIfMissing(rand.nextBoolean()); + origOpts.setAllow2pc(rand.nextBoolean()); + origOpts.setBaseBackgroundCompactions(rand.nextInt(10)); + DBOptions copyOpts = new DBOptions(origOpts); + assertThat(origOpts.createIfMissing()).isEqualTo(copyOpts.createIfMissing()); + assertThat(origOpts.allow2pc()).isEqualTo(copyOpts.allow2pc()); + assertThat(origOpts.baseBackgroundCompactions()).isEqualTo( + copyOpts.baseBackgroundCompactions()); + } + @Test public void getDBOptionsFromProps() { // setup sample properties diff --git a/java/src/test/java/org/rocksdb/OptionsTest.java b/java/src/test/java/org/rocksdb/OptionsTest.java index 7a8142e7c..00aac13b2 100644 --- a/java/src/test/java/org/rocksdb/OptionsTest.java +++ b/java/src/test/java/org/rocksdb/OptionsTest.java @@ -26,6 +26,18 @@ public class OptionsTest { public static final Random rand = PlatformRandomHelper. getPlatformSpecificRandomFactory(); + @Test + public void copyConstructor() { + Options origOpts = new Options(); + origOpts.setNumLevels(rand.nextInt(8)); + origOpts.setTargetFileSizeMultiplier(rand.nextInt(100)); + origOpts.setLevel0StopWritesTrigger(rand.nextInt(50)); + Options copyOpts = new Options(origOpts); + assertThat(origOpts.numLevels()).isEqualTo(copyOpts.numLevels()); + assertThat(origOpts.targetFileSizeMultiplier()).isEqualTo(copyOpts.targetFileSizeMultiplier()); + assertThat(origOpts.level0StopWritesTrigger()).isEqualTo(copyOpts.level0StopWritesTrigger()); + } + @Test public void setIncreaseParallelism() { try (final Options opt = new Options()) { diff --git a/java/src/test/java/org/rocksdb/WriteOptionsTest.java b/java/src/test/java/org/rocksdb/WriteOptionsTest.java index 72a068786..27071e8f2 100644 --- a/java/src/test/java/org/rocksdb/WriteOptionsTest.java +++ b/java/src/test/java/org/rocksdb/WriteOptionsTest.java @@ -8,6 +8,8 @@ package org.rocksdb; import org.junit.ClassRule; import org.junit.Test; +import java.util.Random; + import static org.assertj.core.api.Assertions.assertThat; public class WriteOptionsTest { @@ -16,6 +18,9 @@ public class WriteOptionsTest { public static final RocksMemoryResource rocksMemoryResource = new RocksMemoryResource(); + public static final Random rand = PlatformRandomHelper. + getPlatformSpecificRandomFactory(); + @Test public void writeOptions() { try (final WriteOptions writeOptions = new WriteOptions()) { @@ -42,4 +47,18 @@ public class WriteOptionsTest { assertThat(writeOptions.noSlowdown()).isFalse(); } } + + @Test + public void copyConstructor() { + WriteOptions origOpts = new WriteOptions(); + origOpts.setDisableWAL(rand.nextBoolean()); + origOpts.setIgnoreMissingColumnFamilies(rand.nextBoolean()); + origOpts.setSync(rand.nextBoolean()); + WriteOptions copyOpts = new WriteOptions(origOpts); + assertThat(origOpts.disableWAL()).isEqualTo(copyOpts.disableWAL()); + assertThat(origOpts.ignoreMissingColumnFamilies()).isEqualTo( + copyOpts.ignoreMissingColumnFamilies()); + assertThat(origOpts.sync()).isEqualTo(copyOpts.sync()); + } + }