From f453bcb40d6fe52cf9ad827852f12a3151af6e44 Mon Sep 17 00:00:00 2001 From: Jermy Li Date: Wed, 18 Dec 2019 11:45:18 -0800 Subject: [PATCH] Add unit tests for concurrent CF iteration and drop (#6180) Summary: improve https://github.com/facebook/rocksdb/issues/6147 Pull Request resolved: https://github.com/facebook/rocksdb/pull/6180 Differential Revision: D19148936 fbshipit-source-id: f691c9879fd51d54e96c1a99670cf85ca4485a89 --- HISTORY.md | 1 + db/column_family_test.cc | 39 ++++++++++++++ db/db_iterator_test.cc | 44 ++++++++++++++++ .../java/org/rocksdb/RocksIteratorTest.java | 51 +++++++++++++++++++ 4 files changed, 135 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index f9b56e6ce3..626da99082 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -7,6 +7,7 @@ ### Bug Fixes * Fix a bug that can cause unnecessary bg thread to be scheduled(#6104). * Fix a bug in which a snapshot read could be affected by a DeleteRange after the snapshot (#6062). +* Fix crash caused by concurrent CF iterations and drops(#6147). * Fixed two performance issues related to memtable history trimming. First, a new SuperVersion is now created only if some memtables were actually trimmed. Second, trimming is only scheduled if there is at least one flushed memtable that is kept in memory for the purposes of transaction conflict checking. * Fix a bug in WriteBatchWithIndex::MultiGetFromBatchAndDB, which is called by Transaction::MultiGet, that causes due to stale pointer access when the number of keys is > 32 * BlobDB no longer updates the SST to blob file mapping upon failed compactions. diff --git a/db/column_family_test.cc b/db/column_family_test.cc index 2e91f7da37..b289da7316 100644 --- a/db/column_family_test.cc +++ b/db/column_family_test.cc @@ -2365,6 +2365,45 @@ TEST_P(ColumnFamilyTest, ReadDroppedColumnFamily) { } } +TEST_P(ColumnFamilyTest, LiveIteratorWithDroppedColumnFamily) { + db_options_.create_missing_column_families = true; + db_options_.max_open_files = 20; + // delete obsolete files always + db_options_.delete_obsolete_files_period_micros = 0; + Open({"default", "one", "two"}); + ColumnFamilyOptions options; + options.level0_file_num_compaction_trigger = 100; + options.level0_slowdown_writes_trigger = 200; + options.level0_stop_writes_trigger = 200; + options.write_buffer_size = 100000; // small write buffer size + Reopen({options, options, options}); + + // 1MB should create ~10 files for each CF + int kKeysNum = 10000; + PutRandomData(1, kKeysNum, 100); + + { + std::unique_ptr iterator( + db_->NewIterator(ReadOptions(), handles_[1])); + iterator->SeekToFirst(); + + DropColumnFamilies({1}); + + // Make sure iterator created can still be used. + int count = 0; + for (; iterator->Valid(); iterator->Next()) { + ASSERT_OK(iterator->status()); + ++count; + } + ASSERT_OK(iterator->status()); + ASSERT_EQ(count, kKeysNum); + } + + Reopen(); + Close(); + Destroy(); +} + TEST_P(ColumnFamilyTest, FlushAndDropRaceCondition) { db_options_.create_missing_column_families = true; Open({"default", "one"}); diff --git a/db/db_iterator_test.cc b/db/db_iterator_test.cc index 3fadd55dd3..debb48eda3 100644 --- a/db/db_iterator_test.cc +++ b/db/db_iterator_test.cc @@ -804,6 +804,50 @@ TEST_P(DBIteratorTest, IteratorPinsRef) { } while (ChangeCompactOptions()); } +TEST_P(DBIteratorTest, IteratorDeleteAfterCfDelete) { + CreateAndReopenWithCF({"pikachu"}, CurrentOptions()); + + Put(1, "foo", "delete-cf-then-delete-iter"); + Put(1, "hello", "value2"); + + ColumnFamilyHandle* cf = handles_[1]; + ReadOptions ro; + + auto* iter = db_->NewIterator(ro, cf); + iter->SeekToFirst(); + ASSERT_EQ(IterStatus(iter), "foo->delete-cf-then-delete-iter"); + + // delete CF handle + db_->DestroyColumnFamilyHandle(cf); + handles_.erase(std::begin(handles_) + 1); + + // delete Iterator after CF handle is deleted + iter->Next(); + ASSERT_EQ(IterStatus(iter), "hello->value2"); + delete iter; +} + +TEST_P(DBIteratorTest, IteratorDeleteAfterCfDrop) { + CreateAndReopenWithCF({"pikachu"}, CurrentOptions()); + + Put(1, "foo", "drop-cf-then-delete-iter"); + + ReadOptions ro; + ColumnFamilyHandle* cf = handles_[1]; + + auto* iter = db_->NewIterator(ro, cf); + iter->SeekToFirst(); + ASSERT_EQ(IterStatus(iter), "foo->drop-cf-then-delete-iter"); + + // drop and delete CF + db_->DropColumnFamily(cf); + db_->DestroyColumnFamilyHandle(cf); + handles_.erase(std::begin(handles_) + 1); + + // delete Iterator after CF handle is dropped + delete iter; +} + // SetOptions not defined in ROCKSDB LITE #ifndef ROCKSDB_LITE TEST_P(DBIteratorTest, DBIteratorBoundTest) { diff --git a/java/src/test/java/org/rocksdb/RocksIteratorTest.java b/java/src/test/java/org/rocksdb/RocksIteratorTest.java index 45893eec11..4baac744b0 100644 --- a/java/src/test/java/org/rocksdb/RocksIteratorTest.java +++ b/java/src/test/java/org/rocksdb/RocksIteratorTest.java @@ -97,4 +97,55 @@ public class RocksIteratorTest { } } } + + @Test + public void rocksIteratorReleaseAfterCfClose() throws RocksDBException { + try (final Options options = new Options() + .setCreateIfMissing(true) + .setCreateMissingColumnFamilies(true); + final RocksDB db = RocksDB.open(options, + this.dbFolder.getRoot().getAbsolutePath())) { + db.put("key".getBytes(), "value".getBytes()); + + // Test case: release iterator after default CF close + try (final RocksIterator iterator = db.newIterator()) { + // In fact, calling close() on default CF has no effect + db.getDefaultColumnFamily().close(); + + iterator.seekToFirst(); + assertThat(iterator.isValid()).isTrue(); + assertThat(iterator.key()).isEqualTo("key".getBytes()); + assertThat(iterator.value()).isEqualTo("value".getBytes()); + } + + // Test case: release iterator after custom CF close + ColumnFamilyDescriptor cfd1 = new ColumnFamilyDescriptor("cf1".getBytes()); + ColumnFamilyHandle cfHandle1 = db.createColumnFamily(cfd1); + db.put(cfHandle1, "key1".getBytes(), "value1".getBytes()); + + try (final RocksIterator iterator = db.newIterator(cfHandle1)) { + cfHandle1.close(); + + iterator.seekToFirst(); + assertThat(iterator.isValid()).isTrue(); + assertThat(iterator.key()).isEqualTo("key1".getBytes()); + assertThat(iterator.value()).isEqualTo("value1".getBytes()); + } + + // Test case: release iterator after custom CF drop & close + ColumnFamilyDescriptor cfd2 = new ColumnFamilyDescriptor("cf2".getBytes()); + ColumnFamilyHandle cfHandle2 = db.createColumnFamily(cfd2); + db.put(cfHandle2, "key2".getBytes(), "value2".getBytes()); + + try (final RocksIterator iterator = db.newIterator(cfHandle2)) { + db.dropColumnFamily(cfHandle2); + cfHandle2.close(); + + iterator.seekToFirst(); + assertThat(iterator.isValid()).isTrue(); + assertThat(iterator.key()).isEqualTo("key2".getBytes()); + assertThat(iterator.value()).isEqualTo("value2".getBytes()); + } + } + } }