From 05fba969278306bbc7edd61af1b8cf638a369c8a Mon Sep 17 00:00:00 2001 From: Tomasz Posluszny Date: Wed, 14 Oct 2020 14:37:55 -0700 Subject: [PATCH] Make RocksDB instance responsible for closing associated ColumnFamilyHandle instances (#7428) Summary: - Takes the burden off developer to close ColumnFamilyHandle instances before closing RocksDB instance - The change is backward-compatible ---- Previously the pattern for working with Column Families was: ```java try (final ColumnFamilyOptions cfOpts = new ColumnFamilyOptions().optimizeUniversalStyleCompaction()) { // list of column family descriptors, first entry must always be default column family final List cfDescriptors = Arrays.asList( new ColumnFamilyDescriptor(RocksDB.DEFAULT_COLUMN_FAMILY, cfOpts), new ColumnFamilyDescriptor("my-first-columnfamily".getBytes(), cfOpts) ); // a list which will hold the handles for the column families once the db is opened final List columnFamilyHandleList = new ArrayList<>(); try (final DBOptions options = new DBOptions() .setCreateIfMissing(true) .setCreateMissingColumnFamilies(true); final RocksDB db = RocksDB.open(options, "path/to/do", cfDescriptors, columnFamilyHandleList)) { try { // do something } finally { // NOTE user must explicitly frees the column family handles before freeing the db for (final ColumnFamilyHandle columnFamilyHandle : columnFamilyHandleList) { columnFamilyHandle.close(); } } // frees the column family options } } // frees the db and the db options ``` With the changes in this PR, the Java user no longer has to worry about manually closing the Column Families, which allows them to write simpler symmetrical create/free oriented code like this: ```java try (final ColumnFamilyOptions cfOpts = new ColumnFamilyOptions().optimizeUniversalStyleCompaction()) { // list of column family descriptors, first entry must always be default column family final List cfDescriptors = Arrays.asList( new ColumnFamilyDescriptor(RocksDB.DEFAULT_COLUMN_FAMILY, cfOpts), new ColumnFamilyDescriptor("my-first-columnfamily".getBytes(), cfOpts) ); // a list which will hold the handles for the column families once the db is opened final List columnFamilyHandleList = new ArrayList<>(); try (final DBOptions options = new DBOptions() .setCreateIfMissing(true) .setCreateMissingColumnFamilies(true); final RocksDB db = RocksDB.open(options, "path/to/do", cfDescriptors, columnFamilyHandleList)) { // do something } // frees the column family options, then frees the db and the db options } } ``` **NOTE**: The changes in this PR are backwards API compatible, which means existing code using the original approach will also continue to function correctly. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7428 Reviewed By: cheng-chang Differential Revision: D24063348 Pulled By: jay-zhuang fbshipit-source-id: 648d7526669923128c863ead94516bf4d50ac658 --- .../java/org/rocksdb/ColumnFamilyHandle.java | 6 + java/src/main/java/org/rocksdb/RocksDB.java | 63 ++- .../java/org/rocksdb/ColumnFamilyTest.java | 502 +++++++----------- .../rocksdb/CompactionFilterFactoryTest.java | 31 +- 4 files changed, 258 insertions(+), 344 deletions(-) diff --git a/java/src/main/java/org/rocksdb/ColumnFamilyHandle.java b/java/src/main/java/org/rocksdb/ColumnFamilyHandle.java index a67919b1e..0f1ed3e00 100644 --- a/java/src/main/java/org/rocksdb/ColumnFamilyHandle.java +++ b/java/src/main/java/org/rocksdb/ColumnFamilyHandle.java @@ -13,6 +13,12 @@ import java.util.Objects; * ColumnFamily Pointers. */ public class ColumnFamilyHandle extends RocksObject { + /** + * Constructs column family Java object, which operates on underlying native object. + * + * @param rocksDB db instance associated with this column family + * @param nativeHandle native handle to underlying native ColumnFamily object + */ ColumnFamilyHandle(final RocksDB rocksDB, final long nativeHandle) { super(nativeHandle); diff --git a/java/src/main/java/org/rocksdb/RocksDB.java b/java/src/main/java/org/rocksdb/RocksDB.java index 471a9bbd9..230723367 100644 --- a/java/src/main/java/org/rocksdb/RocksDB.java +++ b/java/src/main/java/org/rocksdb/RocksDB.java @@ -38,6 +38,8 @@ public class RocksDB extends RocksObject { RocksDB.loadLibrary(); } + private List ownedColumnFamilyHandles = new ArrayList<>(); + /** * Loads the necessary library files. * Calling this method twice will have no effect. @@ -307,9 +309,12 @@ public class RocksDB extends RocksObject { db.storeOptionsInstance(options); for (int i = 1; i < handles.length; i++) { - columnFamilyHandles.add(new ColumnFamilyHandle(db, handles[i])); + final ColumnFamilyHandle columnFamilyHandle = new ColumnFamilyHandle(db, handles[i]); + columnFamilyHandles.add(columnFamilyHandle); } + db.ownedColumnFamilyHandles.addAll(columnFamilyHandles); + return db; } @@ -484,9 +489,12 @@ public class RocksDB extends RocksObject { db.storeOptionsInstance(options); for (int i = 1; i < handles.length; i++) { - columnFamilyHandles.add(new ColumnFamilyHandle(db, handles[i])); + final ColumnFamilyHandle columnFamilyHandle = new ColumnFamilyHandle(db, handles[i]); + columnFamilyHandles.add(columnFamilyHandle); } + db.ownedColumnFamilyHandles.addAll(columnFamilyHandles); + return db; } @@ -577,9 +585,12 @@ public class RocksDB extends RocksObject { db.storeOptionsInstance(options); for (int i = 1; i < handles.length; i++) { - columnFamilyHandles.add(new ColumnFamilyHandle(db, handles[i])); + final ColumnFamilyHandle columnFamilyHandle = new ColumnFamilyHandle(db, handles[i]); + columnFamilyHandles.add(columnFamilyHandle); } + db.ownedColumnFamilyHandles.addAll(columnFamilyHandles); + return db; } @@ -597,6 +608,11 @@ public class RocksDB extends RocksObject { * @throws RocksDBException if an error occurs whilst closing. */ public void closeE() throws RocksDBException { + for (final ColumnFamilyHandle columnFamilyHandle : ownedColumnFamilyHandles) { + columnFamilyHandle.close(); + } + ownedColumnFamilyHandles.clear(); + if (owningHandle_.compareAndSet(true, false)) { try { closeDatabase(nativeHandle_); @@ -619,6 +635,11 @@ public class RocksDB extends RocksObject { */ @Override public void close() { + for (final ColumnFamilyHandle columnFamilyHandle : ownedColumnFamilyHandles) { + columnFamilyHandle.close(); + } + ownedColumnFamilyHandles.clear(); + if (owningHandle_.compareAndSet(true, false)) { try { closeDatabase(nativeHandle_); @@ -661,10 +682,12 @@ public class RocksDB extends RocksObject { public ColumnFamilyHandle createColumnFamily( final ColumnFamilyDescriptor columnFamilyDescriptor) throws RocksDBException { - return new ColumnFamilyHandle(this, createColumnFamily(nativeHandle_, - columnFamilyDescriptor.getName(), - columnFamilyDescriptor.getName().length, - columnFamilyDescriptor.getOptions().nativeHandle_)); + final ColumnFamilyHandle columnFamilyHandle = new ColumnFamilyHandle(this, + createColumnFamily(nativeHandle_, columnFamilyDescriptor.getName(), + columnFamilyDescriptor.getName().length, + columnFamilyDescriptor.getOptions().nativeHandle_)); + ownedColumnFamilyHandles.add(columnFamilyHandle); + return columnFamilyHandle; } /** @@ -688,8 +711,10 @@ public class RocksDB extends RocksObject { final List columnFamilyHandles = new ArrayList<>(cfHandles.length); for (int i = 0; i < cfHandles.length; i++) { - columnFamilyHandles.add(new ColumnFamilyHandle(this, cfHandles[i])); + final ColumnFamilyHandle columnFamilyHandle = new ColumnFamilyHandle(this, cfHandles[i]); + columnFamilyHandles.add(columnFamilyHandle); } + ownedColumnFamilyHandles.addAll(columnFamilyHandles); return columnFamilyHandles; } @@ -719,8 +744,10 @@ public class RocksDB extends RocksObject { final List columnFamilyHandles = new ArrayList<>(cfHandles.length); for (int i = 0; i < cfHandles.length; i++) { - columnFamilyHandles.add(new ColumnFamilyHandle(this, cfHandles[i])); + final ColumnFamilyHandle columnFamilyHandle = new ColumnFamilyHandle(this, cfHandles[i]); + columnFamilyHandles.add(columnFamilyHandle); } + ownedColumnFamilyHandles.addAll(columnFamilyHandles); return columnFamilyHandles; } @@ -753,7 +780,22 @@ public class RocksDB extends RocksObject { dropColumnFamilies(nativeHandle_, cfHandles); } - //TODO(AR) what about DestroyColumnFamilyHandle + /** + * Deletes native column family handle of given {@link ColumnFamilyHandle} Java object + * and removes reference from {@link RocksDB#ownedColumnFamilyHandles}. + * + * @param columnFamilyHandle column family handle object. + */ + public void destroyColumnFamilyHandle(final ColumnFamilyHandle columnFamilyHandle) { + for (int i = 0; i < ownedColumnFamilyHandles.size(); ++i) { + final ColumnFamilyHandle ownedHandle = ownedColumnFamilyHandles.get(i); + if (ownedHandle.equals(columnFamilyHandle)) { + columnFamilyHandle.close(); + ownedColumnFamilyHandles.remove(i); + return; + } + } + } /** * Set the database entry for "key" to "value". @@ -4479,7 +4521,6 @@ public class RocksDB extends RocksObject { final long handle, final long cfHandle) throws RocksDBException; private native void dropColumnFamilies(final long handle, final long[] cfHandles) throws RocksDBException; - //TODO(AR) best way to express DestroyColumnFamilyHandle? ...maybe in ColumnFamilyHandle? private native void put(final long handle, final byte[] key, final int keyOffset, final int keyLength, final byte[] value, final int valueOffset, int valueLength) throws RocksDBException; diff --git a/java/src/test/java/org/rocksdb/ColumnFamilyTest.java b/java/src/test/java/org/rocksdb/ColumnFamilyTest.java index a9a087635..9fab479b2 100644 --- a/java/src/test/java/org/rocksdb/ColumnFamilyTest.java +++ b/java/src/test/java/org/rocksdb/ColumnFamilyTest.java @@ -5,16 +5,17 @@ package org.rocksdb; -import java.util.*; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import java.util.*; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import static java.nio.charset.StandardCharsets.UTF_8; -import static org.assertj.core.api.Assertions.assertThat; - public class ColumnFamilyTest { @ClassRule @@ -141,33 +142,19 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfNames, columnFamilyHandleList)) { + assertThat(columnFamilyHandleList.size()).isEqualTo(2); + db.put("dfkey1".getBytes(), "dfvalue".getBytes()); + db.put(columnFamilyHandleList.get(0), "dfkey2".getBytes(), "dfvalue".getBytes()); + db.put(columnFamilyHandleList.get(1), "newcfkey1".getBytes(), "newcfvalue".getBytes()); - try { - assertThat(columnFamilyHandleList.size()).isEqualTo(2); - db.put("dfkey1".getBytes(), "dfvalue".getBytes()); - db.put(columnFamilyHandleList.get(0), "dfkey2".getBytes(), - "dfvalue".getBytes()); - db.put(columnFamilyHandleList.get(1), "newcfkey1".getBytes(), - "newcfvalue".getBytes()); - - String retVal = new String(db.get(columnFamilyHandleList.get(1), - "newcfkey1".getBytes())); - assertThat(retVal).isEqualTo("newcfvalue"); - assertThat((db.get(columnFamilyHandleList.get(1), - "dfkey1".getBytes()))).isNull(); - db.delete(columnFamilyHandleList.get(1), "newcfkey1".getBytes()); - assertThat((db.get(columnFamilyHandleList.get(1), - "newcfkey1".getBytes()))).isNull(); - db.delete(columnFamilyHandleList.get(0), new WriteOptions(), - "dfkey2".getBytes()); - assertThat(db.get(columnFamilyHandleList.get(0), new ReadOptions(), - "dfkey2".getBytes())).isNull(); - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + String retVal = new String(db.get(columnFamilyHandleList.get(1), "newcfkey1".getBytes())); + assertThat(retVal).isEqualTo("newcfvalue"); + assertThat((db.get(columnFamilyHandleList.get(1), "dfkey1".getBytes()))).isNull(); + db.delete(columnFamilyHandleList.get(1), "newcfkey1".getBytes()); + assertThat((db.get(columnFamilyHandleList.get(1), "newcfkey1".getBytes()))).isNull(); + db.delete(columnFamilyHandleList.get(0), new WriteOptions(), "dfkey2".getBytes()); + assertThat(db.get(columnFamilyHandleList.get(0), new ReadOptions(), "dfkey2".getBytes())) + .isNull(); } } @@ -184,30 +171,22 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - try { - db.put(columnFamilyHandleList.get(0), new WriteOptions(), - "key1".getBytes(), "value".getBytes()); - db.put("key2".getBytes(), "12345678".getBytes()); - final byte[] outValue = new byte[5]; - // not found value - int getResult = db.get("keyNotFound".getBytes(), outValue); - assertThat(getResult).isEqualTo(RocksDB.NOT_FOUND); - // found value which fits in outValue - getResult = db.get(columnFamilyHandleList.get(0), "key1".getBytes(), - outValue); - assertThat(getResult).isNotEqualTo(RocksDB.NOT_FOUND); - assertThat(outValue).isEqualTo("value".getBytes()); - // found value which fits partially - getResult = db.get(columnFamilyHandleList.get(0), new ReadOptions(), - "key2".getBytes(), outValue); - assertThat(getResult).isNotEqualTo(RocksDB.NOT_FOUND); - assertThat(outValue).isEqualTo("12345".getBytes()); - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + db.put( + columnFamilyHandleList.get(0), new WriteOptions(), "key1".getBytes(), "value".getBytes()); + db.put("key2".getBytes(), "12345678".getBytes()); + final byte[] outValue = new byte[5]; + // not found value + int getResult = db.get("keyNotFound".getBytes(), outValue); + assertThat(getResult).isEqualTo(RocksDB.NOT_FOUND); + // found value which fits in outValue + getResult = db.get(columnFamilyHandleList.get(0), "key1".getBytes(), outValue); + assertThat(getResult).isNotEqualTo(RocksDB.NOT_FOUND); + assertThat(outValue).isEqualTo("value".getBytes()); + // found value which fits partially + getResult = + db.get(columnFamilyHandleList.get(0), new ReadOptions(), "key2".getBytes(), outValue); + assertThat(getResult).isNotEqualTo(RocksDB.NOT_FOUND); + assertThat(outValue).isEqualTo("12345".getBytes()); } } @@ -223,22 +202,12 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - ColumnFamilyHandle tmpColumnFamilyHandle = null; - try { - tmpColumnFamilyHandle = db.createColumnFamily( - new ColumnFamilyDescriptor("tmpCF".getBytes(), - new ColumnFamilyOptions())); - db.put(tmpColumnFamilyHandle, "key".getBytes(), "value".getBytes()); - db.dropColumnFamily(tmpColumnFamilyHandle); - assertThat(tmpColumnFamilyHandle.isOwningHandle()).isTrue(); - } finally { - if (tmpColumnFamilyHandle != null) { - tmpColumnFamilyHandle.close(); - } - for (ColumnFamilyHandle columnFamilyHandle : columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + ColumnFamilyHandle tmpColumnFamilyHandle; + tmpColumnFamilyHandle = db.createColumnFamily( + new ColumnFamilyDescriptor("tmpCF".getBytes(), new ColumnFamilyOptions())); + db.put(tmpColumnFamilyHandle, "key".getBytes(), "value".getBytes()); + db.dropColumnFamily(tmpColumnFamilyHandle); + assertThat(tmpColumnFamilyHandle.isOwningHandle()).isTrue(); } } @@ -256,29 +225,15 @@ public class ColumnFamilyTest { columnFamilyHandleList)) { ColumnFamilyHandle tmpColumnFamilyHandle = null; ColumnFamilyHandle tmpColumnFamilyHandle2 = null; - try { - tmpColumnFamilyHandle = db.createColumnFamily( - new ColumnFamilyDescriptor("tmpCF".getBytes(), - new ColumnFamilyOptions())); - tmpColumnFamilyHandle2 = db.createColumnFamily( - new ColumnFamilyDescriptor("tmpCF2".getBytes(), - new ColumnFamilyOptions())); - db.put(tmpColumnFamilyHandle, "key".getBytes(), "value".getBytes()); - db.put(tmpColumnFamilyHandle2, "key".getBytes(), "value".getBytes()); - db.dropColumnFamilies(Arrays.asList(tmpColumnFamilyHandle, tmpColumnFamilyHandle2)); - assertThat(tmpColumnFamilyHandle.isOwningHandle()).isTrue(); - assertThat(tmpColumnFamilyHandle2.isOwningHandle()).isTrue(); - } finally { - if (tmpColumnFamilyHandle != null) { - tmpColumnFamilyHandle.close(); - } - if (tmpColumnFamilyHandle2 != null) { - tmpColumnFamilyHandle2.close(); - } - for (ColumnFamilyHandle columnFamilyHandle : columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + tmpColumnFamilyHandle = db.createColumnFamily( + new ColumnFamilyDescriptor("tmpCF".getBytes(), new ColumnFamilyOptions())); + tmpColumnFamilyHandle2 = db.createColumnFamily( + new ColumnFamilyDescriptor("tmpCF2".getBytes(), new ColumnFamilyOptions())); + db.put(tmpColumnFamilyHandle, "key".getBytes(), "value".getBytes()); + db.put(tmpColumnFamilyHandle2, "key".getBytes(), "value".getBytes()); + db.dropColumnFamilies(Arrays.asList(tmpColumnFamilyHandle, tmpColumnFamilyHandle2)); + assertThat(tmpColumnFamilyHandle.isOwningHandle()).isTrue(); + assertThat(tmpColumnFamilyHandle2.isOwningHandle()).isTrue(); } } @@ -300,36 +255,24 @@ public class ColumnFamilyTest { cfDescriptors, columnFamilyHandleList); final WriteBatch writeBatch = new WriteBatch(); final WriteOptions writeOpt = new WriteOptions()) { - try { - writeBatch.put("key".getBytes(), "value".getBytes()); - writeBatch.put(db.getDefaultColumnFamily(), - "mergeKey".getBytes(), "merge".getBytes()); - writeBatch.merge(db.getDefaultColumnFamily(), "mergeKey".getBytes(), - "merge".getBytes()); - writeBatch.put(columnFamilyHandleList.get(1), "newcfkey".getBytes(), - "value".getBytes()); - writeBatch.put(columnFamilyHandleList.get(1), "newcfkey2".getBytes(), - "value2".getBytes()); - writeBatch.delete("xyz".getBytes()); - writeBatch.delete(columnFamilyHandleList.get(1), "xyz".getBytes()); - db.write(writeOpt, writeBatch); + writeBatch.put("key".getBytes(), "value".getBytes()); + writeBatch.put(db.getDefaultColumnFamily(), "mergeKey".getBytes(), "merge".getBytes()); + writeBatch.merge(db.getDefaultColumnFamily(), "mergeKey".getBytes(), "merge".getBytes()); + writeBatch.put(columnFamilyHandleList.get(1), "newcfkey".getBytes(), "value".getBytes()); + writeBatch.put(columnFamilyHandleList.get(1), "newcfkey2".getBytes(), "value2".getBytes()); + writeBatch.delete("xyz".getBytes()); + writeBatch.delete(columnFamilyHandleList.get(1), "xyz".getBytes()); + db.write(writeOpt, writeBatch); - assertThat(db.get(columnFamilyHandleList.get(1), - "xyz".getBytes()) == null); - assertThat(new String(db.get(columnFamilyHandleList.get(1), - "newcfkey".getBytes()))).isEqualTo("value"); - assertThat(new String(db.get(columnFamilyHandleList.get(1), - "newcfkey2".getBytes()))).isEqualTo("value2"); - assertThat(new String(db.get("key".getBytes()))).isEqualTo("value"); - // check if key is merged - assertThat(new String(db.get(db.getDefaultColumnFamily(), - "mergeKey".getBytes()))).isEqualTo("merge,merge"); - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + assertThat(db.get(columnFamilyHandleList.get(1), "xyz".getBytes()) == null); + assertThat(new String(db.get(columnFamilyHandleList.get(1), "newcfkey".getBytes()))) + .isEqualTo("value"); + assertThat(new String(db.get(columnFamilyHandleList.get(1), "newcfkey2".getBytes()))) + .isEqualTo("value2"); + assertThat(new String(db.get("key".getBytes()))).isEqualTo("value"); + // check if key is merged + assertThat(new String(db.get(db.getDefaultColumnFamily(), "mergeKey".getBytes()))) + .isEqualTo("merge,merge"); } } } @@ -346,32 +289,21 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - try { - - db.put(columnFamilyHandleList.get(1), "newcfkey".getBytes(), - "value".getBytes()); - db.put(columnFamilyHandleList.get(1), "newcfkey2".getBytes(), - "value2".getBytes()); - try (final RocksIterator rocksIterator = - db.newIterator(columnFamilyHandleList.get(1))) { - rocksIterator.seekToFirst(); - Map refMap = new HashMap<>(); - refMap.put("newcfkey", "value"); - refMap.put("newcfkey2", "value2"); - int i = 0; - while (rocksIterator.isValid()) { - i++; - assertThat(refMap.get(new String(rocksIterator.key()))). - isEqualTo(new String(rocksIterator.value())); - rocksIterator.next(); - } - assertThat(i).isEqualTo(2); - } - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); + db.put(columnFamilyHandleList.get(1), "newcfkey".getBytes(), "value".getBytes()); + db.put(columnFamilyHandleList.get(1), "newcfkey2".getBytes(), "value2".getBytes()); + try (final RocksIterator rocksIterator = db.newIterator(columnFamilyHandleList.get(1))) { + rocksIterator.seekToFirst(); + Map refMap = new HashMap<>(); + refMap.put("newcfkey", "value"); + refMap.put("newcfkey2", "value2"); + int i = 0; + while (rocksIterator.isValid()) { + i++; + assertThat(refMap.get(new String(rocksIterator.key()))) + .isEqualTo(new String(rocksIterator.value())); + rocksIterator.next(); } + assertThat(i).isEqualTo(2); } } } @@ -388,35 +320,20 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - try { - db.put(columnFamilyHandleList.get(0), "key".getBytes(), - "value".getBytes()); - db.put(columnFamilyHandleList.get(1), "newcfkey".getBytes(), - "value".getBytes()); + db.put(columnFamilyHandleList.get(0), "key".getBytes(), "value".getBytes()); + db.put(columnFamilyHandleList.get(1), "newcfkey".getBytes(), "value".getBytes()); - final List keys = Arrays.asList(new byte[][]{ - "key".getBytes(), "newcfkey".getBytes() - }); + final List keys = + Arrays.asList(new byte[][] {"key".getBytes(), "newcfkey".getBytes()}); - List retValues = db.multiGetAsList(columnFamilyHandleList, keys); - assertThat(retValues.size()).isEqualTo(2); - assertThat(new String(retValues.get(0))) - .isEqualTo("value"); - assertThat(new String(retValues.get(1))) - .isEqualTo("value"); - retValues = db.multiGetAsList(new ReadOptions(), columnFamilyHandleList, - keys); - assertThat(retValues.size()).isEqualTo(2); - assertThat(new String(retValues.get(0))) - .isEqualTo("value"); - assertThat(new String(retValues.get(1))) - .isEqualTo("value"); - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + List retValues = db.multiGetAsList(columnFamilyHandleList, keys); + assertThat(retValues.size()).isEqualTo(2); + assertThat(new String(retValues.get(0))).isEqualTo("value"); + assertThat(new String(retValues.get(1))).isEqualTo("value"); + retValues = db.multiGetAsList(new ReadOptions(), columnFamilyHandleList, keys); + assertThat(retValues.size()).isEqualTo(2); + assertThat(new String(retValues.get(0))).isEqualTo("value"); + assertThat(new String(retValues.get(1))).isEqualTo("value"); } } @@ -432,35 +349,19 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - try { - db.put(columnFamilyHandleList.get(0), "key".getBytes(), - "value".getBytes()); - db.put(columnFamilyHandleList.get(1), "newcfkey".getBytes(), - "value".getBytes()); + db.put(columnFamilyHandleList.get(0), "key".getBytes(), "value".getBytes()); + db.put(columnFamilyHandleList.get(1), "newcfkey".getBytes(), "value".getBytes()); - final List keys = Arrays.asList(new byte[][]{ - "key".getBytes(), "newcfkey".getBytes() - }); - List retValues = db.multiGetAsList(columnFamilyHandleList, - keys); - assertThat(retValues.size()).isEqualTo(2); - assertThat(new String(retValues.get(0))) - .isEqualTo("value"); - assertThat(new String(retValues.get(1))) - .isEqualTo("value"); - retValues = db.multiGetAsList(new ReadOptions(), columnFamilyHandleList, - keys); - assertThat(retValues.size()).isEqualTo(2); - assertThat(new String(retValues.get(0))) - .isEqualTo("value"); - assertThat(new String(retValues.get(1))) - .isEqualTo("value"); - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + final List keys = + Arrays.asList(new byte[][] {"key".getBytes(), "newcfkey".getBytes()}); + List retValues = db.multiGetAsList(columnFamilyHandleList, keys); + assertThat(retValues.size()).isEqualTo(2); + assertThat(new String(retValues.get(0))).isEqualTo("value"); + assertThat(new String(retValues.get(1))).isEqualTo("value"); + retValues = db.multiGetAsList(new ReadOptions(), columnFamilyHandleList, keys); + assertThat(retValues.size()).isEqualTo(2); + assertThat(new String(retValues.get(0))).isEqualTo("value"); + assertThat(new String(retValues.get(1))).isEqualTo("value"); } } @@ -476,30 +377,18 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - try { - assertThat(db.getProperty("rocksdb.estimate-num-keys")). - isNotNull(); - assertThat(db.getLongProperty(columnFamilyHandleList.get(0), - "rocksdb.estimate-num-keys")).isGreaterThanOrEqualTo(0); - assertThat(db.getProperty("rocksdb.stats")).isNotNull(); - assertThat(db.getProperty(columnFamilyHandleList.get(0), - "rocksdb.sstables")).isNotNull(); - assertThat(db.getProperty(columnFamilyHandleList.get(1), - "rocksdb.estimate-num-keys")).isNotNull(); - assertThat(db.getProperty(columnFamilyHandleList.get(1), - "rocksdb.stats")).isNotNull(); - assertThat(db.getProperty(columnFamilyHandleList.get(1), - "rocksdb.sstables")).isNotNull(); - assertThat(db.getAggregatedLongProperty("rocksdb.estimate-num-keys")). - isNotNull(); - assertThat(db.getAggregatedLongProperty("rocksdb.estimate-num-keys")). - isGreaterThanOrEqualTo(0); - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + assertThat(db.getProperty("rocksdb.estimate-num-keys")).isNotNull(); + assertThat(db.getLongProperty(columnFamilyHandleList.get(0), "rocksdb.estimate-num-keys")) + .isGreaterThanOrEqualTo(0); + assertThat(db.getProperty("rocksdb.stats")).isNotNull(); + assertThat(db.getProperty(columnFamilyHandleList.get(0), "rocksdb.sstables")).isNotNull(); + assertThat(db.getProperty(columnFamilyHandleList.get(1), "rocksdb.estimate-num-keys")) + .isNotNull(); + assertThat(db.getProperty(columnFamilyHandleList.get(1), "rocksdb.stats")).isNotNull(); + assertThat(db.getProperty(columnFamilyHandleList.get(1), "rocksdb.sstables")).isNotNull(); + assertThat(db.getAggregatedLongProperty("rocksdb.estimate-num-keys")).isNotNull(); + assertThat(db.getAggregatedLongProperty("rocksdb.estimate-num-keys")) + .isGreaterThanOrEqualTo(0); } } @@ -547,10 +436,6 @@ public class ColumnFamilyTest { rocksIterator.close(); } } - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } } } } @@ -566,15 +451,8 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - try { - db.dropColumnFamily(columnFamilyHandleList.get(1)); - db.put(columnFamilyHandleList.get(1), "key".getBytes(), - "value".getBytes()); - } finally { - for (ColumnFamilyHandle columnFamilyHandle : columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + db.dropColumnFamily(columnFamilyHandleList.get(1)); + db.put(columnFamilyHandleList.get(1), "key".getBytes(), "value".getBytes()); } } @@ -589,15 +467,8 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - try { - db.dropColumnFamily(columnFamilyHandleList.get(1)); - db.delete(columnFamilyHandleList.get(1), "key".getBytes()); - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + db.dropColumnFamily(columnFamilyHandleList.get(1)); + db.delete(columnFamilyHandleList.get(1), "key".getBytes()); } } @@ -612,15 +483,8 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - try { - db.dropColumnFamily(columnFamilyHandleList.get(1)); - db.get(columnFamilyHandleList.get(1), "key".getBytes()); - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + db.dropColumnFamily(columnFamilyHandleList.get(1)); + db.get(columnFamilyHandleList.get(1), "key".getBytes()); } } @@ -635,19 +499,11 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfDescriptors, columnFamilyHandleList)) { - try { - final List keys = new ArrayList<>(); - keys.add("key".getBytes()); - keys.add("newcfkey".getBytes()); - final List cfCustomList = new ArrayList<>(); - db.multiGetAsList(cfCustomList, keys); - - } finally { - for (final ColumnFamilyHandle columnFamilyHandle : - columnFamilyHandleList) { - columnFamilyHandle.close(); - } - } + final List keys = new ArrayList<>(); + keys.add("key".getBytes()); + keys.add("newcfkey".getBytes()); + final List cfCustomList = new ArrayList<>(); + db.multiGetAsList(cfCustomList, keys); } } @@ -661,25 +517,12 @@ public class ColumnFamilyTest { final byte[] b0 = new byte[]{(byte) 0x00}; final byte[] b1 = new byte[]{(byte) 0x01}; final byte[] b2 = new byte[]{(byte) 0x02}; - ColumnFamilyHandle cf1 = null, cf2 = null, cf3 = null; - try { - cf1 = db.createColumnFamily(new ColumnFamilyDescriptor(b0)); - cf2 = db.createColumnFamily(new ColumnFamilyDescriptor(b1)); - final List families = RocksDB.listColumnFamilies(options, - dbFolder.getRoot().getAbsolutePath()); - assertThat(families).contains("default".getBytes(), b0, b1); - cf3 = db.createColumnFamily(new ColumnFamilyDescriptor(b2)); - } finally { - if (cf1 != null) { - cf1.close(); - } - if (cf2 != null) { - cf2.close(); - } - if (cf3 != null) { - cf3.close(); - } - } + db.createColumnFamily(new ColumnFamilyDescriptor(b0)); + db.createColumnFamily(new ColumnFamilyDescriptor(b1)); + final List families = + RocksDB.listColumnFamilies(options, dbFolder.getRoot().getAbsolutePath()); + assertThat(families).contains("default".getBytes(), b0, b1); + db.createColumnFamily(new ColumnFamilyDescriptor(b2)); } } @@ -690,22 +533,13 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath()); ) { - try { - final byte[] b0 = new byte[]{0, 0}; - final byte[] b1 = new byte[]{0, 1}; - cf1 = db.createColumnFamily(new ColumnFamilyDescriptor(b0)); - cf2 = db.createColumnFamily(new ColumnFamilyDescriptor(b1)); - final List families = RocksDB.listColumnFamilies(options, - dbFolder.getRoot().getAbsolutePath()); - assertThat(families).contains("default".getBytes(), b0, b1); - } finally { - if (cf1 != null) { - cf1.close(); - } - if (cf2 != null) { - cf2.close(); - } - } + final byte[] b0 = new byte[] {0, 0}; + final byte[] b1 = new byte[] {0, 1}; + cf1 = db.createColumnFamily(new ColumnFamilyDescriptor(b0)); + cf2 = db.createColumnFamily(new ColumnFamilyDescriptor(b1)); + final List families = + RocksDB.listColumnFamilies(options, dbFolder.getRoot().getAbsolutePath()); + assertThat(families).contains("default".getBytes(), b0, b1); } } @@ -716,17 +550,57 @@ public class ColumnFamilyTest { final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath()); ) { - try { - final String simplifiedChinese = "\u7b80\u4f53\u5b57"; - columnFamilyHandle = db.createColumnFamily( - new ColumnFamilyDescriptor(simplifiedChinese.getBytes())); + final String simplifiedChinese = "\u7b80\u4f53\u5b57"; + columnFamilyHandle = + db.createColumnFamily(new ColumnFamilyDescriptor(simplifiedChinese.getBytes())); - final List families = RocksDB.listColumnFamilies(options, - dbFolder.getRoot().getAbsolutePath()); - assertThat(families).contains("default".getBytes(), - simplifiedChinese.getBytes()); + final List families = + RocksDB.listColumnFamilies(options, dbFolder.getRoot().getAbsolutePath()); + assertThat(families).contains("default".getBytes(), simplifiedChinese.getBytes()); + } + } + + @Test + public void testDestroyColumnFamilyHandle() throws RocksDBException { + try (final Options options = new Options().setCreateIfMissing(true); + final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath());) { + final byte[] name1 = "cf1".getBytes(); + final byte[] name2 = "cf2".getBytes(); + final ColumnFamilyDescriptor desc1 = new ColumnFamilyDescriptor(name1); + final ColumnFamilyDescriptor desc2 = new ColumnFamilyDescriptor(name2); + final ColumnFamilyHandle cf1 = db.createColumnFamily(desc1); + final ColumnFamilyHandle cf2 = db.createColumnFamily(desc2); + assertTrue(cf1.isOwningHandle()); + assertTrue(cf2.isOwningHandle()); + assertFalse(cf1.isDefaultColumnFamily()); + db.destroyColumnFamilyHandle(cf1); + // At this point cf1 should not be used! + assertFalse(cf1.isOwningHandle()); + assertTrue(cf2.isOwningHandle()); + } + } + + @Test + @Deprecated + /** + * @deprecated Now explicitly closing instances of ColumnFamilyHandle is not required. + * RocksDB instance will take care of closing its associated ColumnFamilyHandle objects. + */ + public void testColumnFamilyCloseBeforeDb() throws RocksDBException { + final List cfNames = + Arrays.asList(new ColumnFamilyDescriptor(RocksDB.DEFAULT_COLUMN_FAMILY), + new ColumnFamilyDescriptor("new_cf".getBytes())); + final List columnFamilyHandleList = new ArrayList<>(); + + try (final DBOptions options = + new DBOptions().setCreateIfMissing(true).setCreateMissingColumnFamilies(true); + final RocksDB db = RocksDB.open( + options, dbFolder.getRoot().getAbsolutePath(), cfNames, columnFamilyHandleList)) { + try { + db.put("testKey".getBytes(), "tstValue".getBytes()); + // Do something... } finally { - if (columnFamilyHandle != null) { + for (final ColumnFamilyHandle columnFamilyHandle : columnFamilyHandleList) { columnFamilyHandle.close(); } } diff --git a/java/src/test/java/org/rocksdb/CompactionFilterFactoryTest.java b/java/src/test/java/org/rocksdb/CompactionFilterFactoryTest.java index e05f1eef3..35a14eb54 100644 --- a/java/src/test/java/org/rocksdb/CompactionFilterFactoryTest.java +++ b/java/src/test/java/org/rocksdb/CompactionFilterFactoryTest.java @@ -39,29 +39,22 @@ public class CompactionFilterFactoryTest { final List cfHandles = new ArrayList<>(); - try (final RocksDB rocksDb = RocksDB.open(options, - dbFolder.getRoot().getAbsolutePath(), cfNames, cfHandles); - ) { - try { - final byte[] key1 = "key1".getBytes(); - final byte[] key2 = "key2".getBytes(); + try (final RocksDB rocksDb = + RocksDB.open(options, dbFolder.getRoot().getAbsolutePath(), cfNames, cfHandles)) { + final byte[] key1 = "key1".getBytes(); + final byte[] key2 = "key2".getBytes(); - final byte[] value1 = "value1".getBytes(); - final byte[] value2 = new byte[0]; + final byte[] value1 = "value1".getBytes(); + final byte[] value2 = new byte[0]; - rocksDb.put(cfHandles.get(1), key1, value1); - rocksDb.put(cfHandles.get(1), key2, value2); + rocksDb.put(cfHandles.get(1), key1, value1); + rocksDb.put(cfHandles.get(1), key2, value2); - rocksDb.compactRange(cfHandles.get(1)); + rocksDb.compactRange(cfHandles.get(1)); - assertThat(rocksDb.get(cfHandles.get(1), key1)).isEqualTo(value1); - final boolean exists = rocksDb.keyMayExist(cfHandles.get(1), key2, null); - assertThat(exists).isFalse(); - } finally { - for (final ColumnFamilyHandle cfHandle : cfHandles) { - cfHandle.close(); - } - } + assertThat(rocksDb.get(cfHandles.get(1), key1)).isEqualTo(value1); + final boolean exists = rocksDb.keyMayExist(cfHandles.get(1), key2, null); + assertThat(exists).isFalse(); } } }