From d94932323a9cfcfcbf37d9cfd86e7306fdd02c32 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Thu, 18 Nov 2021 11:51:55 -0800 Subject: [PATCH] Check that newIteratorWithBase regardless of WBWI Overwrite Mode (#8134) Summary: The behaviour of WBWI has changed when calling newIteratorWithBase when overwrite is set to true or false. This PR simply adds tests to assert the new correct behaviour. Closes https://github.com/facebook/rocksdb/issues/7370 Closes https://github.com/facebook/rocksdb/pull/8134 Pull Request resolved: https://github.com/facebook/rocksdb/pull/9107 Reviewed By: pdillinger Differential Revision: D32099475 Pulled By: mrambacher fbshipit-source-id: 245f483f73db866cc8a51219a2bff2e09e59faa0 --- .../java/org/rocksdb/WriteBatchWithIndex.java | 4 +- .../org/rocksdb/WriteBatchWithIndexTest.java | 102 ++++++++++++++++++ .../write_batch_with_index_test.cc | 14 +++ 3 files changed, 118 insertions(+), 2 deletions(-) diff --git a/java/src/main/java/org/rocksdb/WriteBatchWithIndex.java b/java/src/main/java/org/rocksdb/WriteBatchWithIndex.java index e5b8ba011..495afd3fa 100644 --- a/java/src/main/java/org/rocksdb/WriteBatchWithIndex.java +++ b/java/src/main/java/org/rocksdb/WriteBatchWithIndex.java @@ -117,7 +117,7 @@ public class WriteBatchWithIndex extends AbstractWriteBatch { * as a delta and baseIterator as a base * * Updating write batch with the current key of the iterator is not safe. - * We strongly recommand users not to do it. It will invalidate the current + * We strongly recommend users not to do it. It will invalidate the current * key() and value() of the iterator. This invalidation happens even before * the write batch update finishes. The state may recover after Next() is * called. @@ -140,7 +140,7 @@ public class WriteBatchWithIndex extends AbstractWriteBatch { * as a delta and baseIterator as a base * * Updating write batch with the current key of the iterator is not safe. - * We strongly recommand users not to do it. It will invalidate the current + * We strongly recommend users not to do it. It will invalidate the current * key() and value() of the iterator. This invalidation happens even before * the write batch update finishes. The state may recover after Next() is * called. diff --git a/java/src/test/java/org/rocksdb/WriteBatchWithIndexTest.java b/java/src/test/java/org/rocksdb/WriteBatchWithIndexTest.java index 38074be38..528c17d4d 100644 --- a/java/src/test/java/org/rocksdb/WriteBatchWithIndexTest.java +++ b/java/src/test/java/org/rocksdb/WriteBatchWithIndexTest.java @@ -654,4 +654,106 @@ public class WriteBatchWithIndexTest { assertThat(db.get("key4".getBytes())).isEqualTo("xyz".getBytes()); } } + + @Test + public void iteratorWithBaseOverwriteTrue() throws RocksDBException { + try (final Options options = new Options().setCreateIfMissing(true); + final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath())) { + try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(true); + final RocksIterator baseIter = db.newIterator(); + final RocksIterator wbwiIter = wbwi.newIteratorWithBase(baseIter)) { + assertThat(wbwiIter).isNotNull(); + assertThat(wbwiIter.nativeHandle_).isGreaterThan(0); + wbwiIter.status(); + } + + try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(true); + final RocksIterator baseIter = db.newIterator(); + final ReadOptions readOptions = new ReadOptions(); + final RocksIterator wbwiIter = wbwi.newIteratorWithBase(baseIter, readOptions)) { + assertThat(wbwiIter).isNotNull(); + assertThat(wbwiIter.nativeHandle_).isGreaterThan(0); + wbwiIter.status(); + } + } + + 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 (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(true); + final RocksIterator baseIter = db.newIterator(); + final RocksIterator wbwiIter = + wbwi.newIteratorWithBase(columnFamilyHandleList.get(1), baseIter)) { + assertThat(wbwiIter).isNotNull(); + assertThat(wbwiIter.nativeHandle_).isGreaterThan(0); + wbwiIter.status(); + } + + try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(true); + final RocksIterator baseIter = db.newIterator(); + final ReadOptions readOptions = new ReadOptions(); + final RocksIterator wbwiIter = + wbwi.newIteratorWithBase(columnFamilyHandleList.get(1), baseIter, readOptions)) { + assertThat(wbwiIter).isNotNull(); + assertThat(wbwiIter.nativeHandle_).isGreaterThan(0); + wbwiIter.status(); + } + } + } + + @Test + public void iteratorWithBaseOverwriteFalse() throws RocksDBException { + try (final Options options = new Options().setCreateIfMissing(true); + final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath())) { + try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(false); + final RocksIterator baseIter = db.newIterator(); + final RocksIterator wbwiIter = wbwi.newIteratorWithBase(baseIter)) { + assertThat(wbwiIter).isNotNull(); + assertThat(wbwiIter.nativeHandle_).isGreaterThan(0); + wbwiIter.status(); + } + + try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(false); + final RocksIterator baseIter = db.newIterator(); + final ReadOptions readOptions = new ReadOptions(); + final RocksIterator wbwiIter = wbwi.newIteratorWithBase(baseIter, readOptions)) { + assertThat(wbwiIter).isNotNull(); + assertThat(wbwiIter.nativeHandle_).isGreaterThan(0); + wbwiIter.status(); + } + } + + 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 (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(false); + final RocksIterator baseIter = db.newIterator(); + final RocksIterator wbwiIter = + wbwi.newIteratorWithBase(columnFamilyHandleList.get(1), baseIter)) { + assertThat(wbwiIter).isNotNull(); + assertThat(wbwiIter.nativeHandle_).isGreaterThan(0); + wbwiIter.status(); + } + + try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(false); + final RocksIterator baseIter = db.newIterator(); + final ReadOptions readOptions = new ReadOptions(); + final RocksIterator wbwiIter = + wbwi.newIteratorWithBase(columnFamilyHandleList.get(1), baseIter, readOptions)) { + assertThat(wbwiIter).isNotNull(); + assertThat(wbwiIter.nativeHandle_).isGreaterThan(0); + wbwiIter.status(); + } + } + } } diff --git a/utilities/write_batch_with_index/write_batch_with_index_test.cc b/utilities/write_batch_with_index/write_batch_with_index_test.cc index f39085efa..271084ad0 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_test.cc +++ b/utilities/write_batch_with_index/write_batch_with_index_test.cc @@ -1634,6 +1634,20 @@ TEST_F(WBWIOverwriteTest, MutateWhileIteratingBaseStressTest) { ASSERT_OK(iter->status()); } +TEST_P(WriteBatchWithIndexTest, TestNewIteratorWithBaseFromWbwi) { + ColumnFamilyHandleImplDummy cf1(6, BytewiseComparator()); + KVMap map; + map["a"] = "aa"; + map["c"] = "cc"; + map["e"] = "ee"; + std::unique_ptr iter( + batch_->NewIteratorWithBase(&cf1, new KVIter(&map))); + ASSERT_NE(nullptr, iter); + iter->SeekToFirst(); + ASSERT_TRUE(iter->Valid()); + ASSERT_OK(iter->status()); +} + TEST_P(WriteBatchWithIndexTest, SavePointTest) { ColumnFamilyHandleImplDummy cf1(1, BytewiseComparator()); KVMap empty_map; -- GitLab