From 16bff5370d6d660de8e07c354db5a8d3b2fc6bc0 Mon Sep 17 00:00:00 2001 From: Zhichao Cao Date: Tue, 13 Oct 2020 11:58:12 -0700 Subject: [PATCH] Add plain_table_db_test to ASSERT_STATUS_CHECKED list (#7482) Summary: Add plain_table_db_test to ASSERT_STATUS_CHECKED list Pull Request resolved: https://github.com/facebook/rocksdb/pull/7482 Test Plan: ASSERT_STATUS_CHECKED=1 make -j48 plain_table_db_test Reviewed By: riversand963 Differential Revision: D24034987 Pulled By: zhichao-cao fbshipit-source-id: e61c937d55ded0947cc8936937362dafed572a60 --- Makefile | 1 + db/plain_table_db_test.cc | 52 +++++++++++++++++------------- table/plain/plain_table_builder.cc | 1 - table/table_factory.cc | 2 +- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index 0f6e886d8..752b1a3ef 100644 --- a/Makefile +++ b/Makefile @@ -619,6 +619,7 @@ ifdef ASSERT_STATUS_CHECKED mock_env_test \ object_registry_test \ prefix_test \ + plain_table_db_test \ repair_test \ configurable_test \ options_settable_test \ diff --git a/db/plain_table_db_test.cc b/db/plain_table_db_test.cc index dd428da9b..0f422a2d6 100644 --- a/db/plain_table_db_test.cc +++ b/db/plain_table_db_test.cc @@ -397,7 +397,7 @@ class TestPlainTableFactory : public PlainTableFactory { TEST_P(PlainTableDBTest, BadOptions1) { // Build with a prefix extractor ASSERT_OK(Put("1000000000000foo", "v1")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); // Bad attempt to re-open without a prefix extractor Options options = CurrentOptions(); @@ -428,7 +428,9 @@ TEST_P(PlainTableDBTest, BadOptions2) { // Build without a prefix extractor // (apparently works even if hash_table_ratio > 0) ASSERT_OK(Put("1000000000000foo", "v1")); - dbfull()->TEST_FlushMemTable(); + // Build without a prefix extractor, this call will fail and returns the + // status for this bad attempt. + ASSERT_NOK(dbfull()->TEST_FlushMemTable()); // Bad attempt to re-open with hash_table_ratio > 0 and no prefix extractor Status s = TryReopen(&options); @@ -503,14 +505,15 @@ TEST_P(PlainTableDBTest, Flush) { ASSERT_OK(Put("1000000000000foo", "v1")); ASSERT_OK(Put("0000000000000bar", "v2")); ASSERT_OK(Put("1000000000000foo", "v3")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_TRUE(dbfull()->GetIntProperty( "rocksdb.estimate-table-readers-mem", &int_num)); ASSERT_GT(int_num, 0U); TablePropertiesCollection ptc; - reinterpret_cast(dbfull())->GetPropertiesOfAllTables(&ptc); + ASSERT_OK( + reinterpret_cast(dbfull())->GetPropertiesOfAllTables(&ptc)); ASSERT_EQ(1U, ptc.size()); auto row = ptc.begin(); auto tp = row->second; @@ -595,23 +598,23 @@ TEST_P(PlainTableDBTest, Flush2) { DestroyAndReopen(&options); ASSERT_OK(Put("0000000000000bar", "b")); ASSERT_OK(Put("1000000000000foo", "v1")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_OK(Put("1000000000000foo", "v2")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("v2", Get("1000000000000foo")); ASSERT_OK(Put("0000000000000eee", "v3")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("v3", Get("0000000000000eee")); ASSERT_OK(Delete("0000000000000bar")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("NOT_FOUND", Get("0000000000000bar")); ASSERT_OK(Put("0000000000000eee", "v5")); ASSERT_OK(Put("9000000000000eee", "v5")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("v5", Get("0000000000000eee")); // Test Bloom Filter @@ -651,7 +654,7 @@ TEST_P(PlainTableDBTest, Immortal) { DestroyAndReopen(&options); ASSERT_OK(Put("0000000000000bar", "b")); ASSERT_OK(Put("1000000000000foo", "v1")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); int copied = 0; ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( @@ -729,7 +732,7 @@ TEST_P(PlainTableDBTest, Iterator) { ASSERT_OK(Put("1000000000foo005", "v__5")); ASSERT_OK(Put("1000000000foo007", "v__7")); ASSERT_OK(Put("1000000000foo008", "v__8")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("v1", Get("1000000000foo001")); ASSERT_EQ("v__3", Get("1000000000foo003")); Iterator* iter = dbfull()->NewIterator(ReadOptions()); @@ -799,7 +802,7 @@ TEST_P(PlainTableDBTest, Iterator) { expect_bloom_not_match = false; } } - + ASSERT_OK(iter->status()); delete iter; } } @@ -840,7 +843,7 @@ TEST_P(PlainTableDBTest, BloomSchema) { for (unsigned i = 0; i < 2345; ++i) { ASSERT_OK(Put(NthKey(i, 'y'), "added")); } - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("added", Get(NthKey(42, 'y'))); for (unsigned i = 0; i < 32; ++i) { @@ -898,7 +901,7 @@ TEST_P(PlainTableDBTest, IteratorLargeKeys) { ASSERT_OK(Put(key_list[i], ToString(i))); } - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); Iterator* iter = dbfull()->NewIterator(ReadOptions()); iter->Seek(key_list[0]); @@ -946,7 +949,7 @@ TEST_P(PlainTableDBTest, IteratorLargeKeysWithPrefix) { ASSERT_OK(Put(key_list[i], ToString(i))); } - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); Iterator* iter = dbfull()->NewIterator(ReadOptions()); iter->Seek(key_list[0]); @@ -981,7 +984,7 @@ TEST_P(PlainTableDBTest, IteratorReverseSuffixComparator) { ASSERT_OK(Put("1000000000foo005", "v__5")); ASSERT_OK(Put("1000000000foo007", "v__7")); ASSERT_OK(Put("1000000000foo008", "v__8")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("v1", Get("1000000000foo001")); ASSERT_EQ("v__3", Get("1000000000foo003")); Iterator* iter = dbfull()->NewIterator(ReadOptions()); @@ -1059,7 +1062,7 @@ TEST_P(PlainTableDBTest, HashBucketConflict) { ASSERT_OK(Put("2000000000000fo2", "v")); ASSERT_OK(Put("2000000000000fo3", "v")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("v1", Get("5000000000000fo0")); ASSERT_EQ("v2", Get("5000000000000fo1")); @@ -1120,6 +1123,7 @@ TEST_P(PlainTableDBTest, HashBucketConflict) { iter->Seek("8000000000000fo2"); ASSERT_TRUE(!iter->Valid()); + ASSERT_OK(iter->status()); delete iter; } } @@ -1153,7 +1157,7 @@ TEST_P(PlainTableDBTest, HashBucketConflictReverseSuffixComparator) { ASSERT_OK(Put("2000000000000fo2", "v")); ASSERT_OK(Put("2000000000000fo3", "v")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("v1", Get("5000000000000fo0")); ASSERT_EQ("v2", Get("5000000000000fo1")); @@ -1213,6 +1217,7 @@ TEST_P(PlainTableDBTest, HashBucketConflictReverseSuffixComparator) { iter->Seek("8000000000000fo2"); ASSERT_TRUE(!iter->Valid()); + ASSERT_OK(iter->status()); delete iter; } } @@ -1235,7 +1240,7 @@ TEST_P(PlainTableDBTest, NonExistingKeyToNonEmptyBucket) { ASSERT_OK(Put("5000000000000fo1", "v2")); ASSERT_OK(Put("5000000000000fo2", "v3")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("v1", Get("5000000000000fo0")); ASSERT_EQ("v2", Get("5000000000000fo1")); @@ -1259,6 +1264,7 @@ TEST_P(PlainTableDBTest, NonExistingKeyToNonEmptyBucket) { iter->Seek("8000000000000fo2"); ASSERT_TRUE(!iter->Valid()); + ASSERT_OK(iter->status()); delete iter; } @@ -1286,7 +1292,7 @@ TEST_P(PlainTableDBTest, CompactionTrigger) { ASSERT_OK(Put(Key(i), values[i])); } ASSERT_OK(Put(Key(999), "")); - dbfull()->TEST_WaitForFlushMemTable(); + ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable()); ASSERT_EQ(NumTableFilesAtLevel(0), num + 1); } @@ -1297,7 +1303,7 @@ TEST_P(PlainTableDBTest, CompactionTrigger) { ASSERT_OK(Put(Key(i), values[i])); } ASSERT_OK(Put(Key(999), "")); - dbfull()->TEST_WaitForCompact(); + ASSERT_OK(dbfull()->TEST_WaitForCompact()); ASSERT_EQ(NumTableFilesAtLevel(0), 0); ASSERT_EQ(NumTableFilesAtLevel(1), 1); @@ -1313,7 +1319,7 @@ TEST_P(PlainTableDBTest, AdaptiveTable) { ASSERT_OK(Put("1000000000000foo", "v1")); ASSERT_OK(Put("0000000000000bar", "v2")); ASSERT_OK(Put("1000000000000foo", "v3")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); options.create_if_missing = false; std::shared_ptr block_based_factory( @@ -1329,7 +1335,7 @@ TEST_P(PlainTableDBTest, AdaptiveTable) { ASSERT_OK(Put("2000000000000foo", "v4")); ASSERT_OK(Put("3000000000000bar", "v5")); - dbfull()->TEST_FlushMemTable(); + ASSERT_OK(dbfull()->TEST_FlushMemTable()); ASSERT_EQ("v4", Get("2000000000000foo")); ASSERT_EQ("v5", Get("3000000000000bar")); diff --git a/table/plain/plain_table_builder.cc b/table/plain/plain_table_builder.cc index faebcfe2f..df643cf7b 100644 --- a/table/plain/plain_table_builder.cc +++ b/table/plain/plain_table_builder.cc @@ -209,7 +209,6 @@ Status PlainTableBuilder::Finish() { if (store_index_in_file_ && (properties_.num_entries > 0)) { assert(properties_.num_entries <= std::numeric_limits::max()); - Status s; BlockHandle bloom_block_handle; if (bloom_bits_per_key_ > 0) { bloom_block_.SetTotalBits( diff --git a/table/table_factory.cc b/table/table_factory.cc index 18935c859..5565202e1 100644 --- a/table/table_factory.cc +++ b/table/table_factory.cc @@ -40,7 +40,7 @@ Status TableFactory::CreateFromString(const ConfigOptions& config_options_in, status = Status::NotSupported("Could not load table factory: ", name); return status; } - if (!existing_opts.empty()) { + if (status.ok() && !existing_opts.empty()) { config_options.invoke_prepare_options = false; status = factory->get()->ConfigureFromString(config_options, existing_opts); }