Add few unit test cases in ASSERT_STATUS_CHECKED build (#7427)

Summary:
Fix few test cases and add them in ASSERT_STATUS_CHECKED build.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7427

Test Plan:
1.  ASSERT_STATUS_CHECKED=1 make -j48 check,
                 2. travis build for ASSERT_STATUS_CHECKED,
                 3. Without ASSERT_STATUS_CHECKED:  make check -j64, CircleCI build and travis build

Reviewed By: pdillinger

Differential Revision: D23909983

Pulled By: akankshamahajan15

fbshipit-source-id: 42d7e4aea972acb9fcddb7ca73fcb82f93272434

Makefile

@@ -634,6 +634,12 @@ ifdef ASSERT_STATUS_CHECKED
 		version_edit_test \
 		work_queue_test \
 		write_controller_test \
+		compaction_iterator_test \
+		compaction_job_test \
+		compaction_job_stats_test \
+	        io_tracer_test \
+		merge_helper_test \
+		memtable_list_test \
 
 ifeq ($(USE_FOLLY_DISTRIBUTED_MUTEX),1)
 TESTS_PASSING_ASC += folly_synchronization_distributed_mutex_test

db/column_family.cc

@@ -1102,10 +1102,12 @@ Status ColumnFamilyData::RangesOverlapWithMemtables(
       super_version->mem->NewRangeTombstoneIterator(read_opts, read_seq);
   range_del_agg.AddTombstones(
       std::unique_ptr<FragmentedRangeTombstoneIterator>(active_range_del_iter));
-  super_version->imm->AddRangeTombstoneIterators(read_opts, nullptr /* arena */,
-                                                 &range_del_agg);
-
   Status status;
+  status = super_version->imm->AddRangeTombstoneIterators(
+      read_opts, nullptr /* arena */, &range_del_agg);
+  // AddRangeTombstoneIterators always return Status::OK.
+  assert(status.ok());
+
   for (size_t i = 0; i < ranges.size() && status.ok() && !*overlap; ++i) {
     auto* vstorage = super_version->current->storage_info();
     auto* ucmp = vstorage->InternalComparator()->user_comparator();

db/compaction/compaction_iterator_test.cc

@@ -294,6 +294,7 @@ class CompactionIteratorTest : public testing::TestWithParam<bool> {
       ASSERT_EQ(expected_values[i], c_iter_->value().ToString()) << info;
       c_iter_->Next();
     }
+    ASSERT_OK(c_iter_->status());
     ASSERT_FALSE(c_iter_->Valid());
   }
 
@@ -318,6 +319,7 @@ TEST_P(CompactionIteratorTest, EmptyResult) {
                  test::KeyStr("a", 3, kTypeValue)},
                 {"", "val"}, {}, {}, 5);
   c_iter_->SeekToFirst();
+  ASSERT_OK(c_iter_->status());
   ASSERT_FALSE(c_iter_->Valid());
 }
 
@@ -339,6 +341,7 @@ TEST_P(CompactionIteratorTest, CorruptionAfterSingleDeletion) {
   ASSERT_TRUE(c_iter_->Valid());
   ASSERT_EQ(test::KeyStr("b", 10, kTypeValue), c_iter_->key().ToString());
   c_iter_->Next();
+  ASSERT_OK(c_iter_->status());
   ASSERT_FALSE(c_iter_->Valid());
 }
 
@@ -355,6 +358,7 @@ TEST_P(CompactionIteratorTest, SimpleRangeDeletion) {
   ASSERT_TRUE(c_iter_->Valid());
   ASSERT_EQ(test::KeyStr("night", 3, kTypeValue), c_iter_->key().ToString());
   c_iter_->Next();
+  ASSERT_OK(c_iter_->status());
   ASSERT_FALSE(c_iter_->Valid());
 }
 
@@ -376,6 +380,7 @@ TEST_P(CompactionIteratorTest, RangeDeletionWithSnapshots) {
   ASSERT_TRUE(c_iter_->Valid());
   ASSERT_EQ(test::KeyStr("night", 40, kTypeValue), c_iter_->key().ToString());
   c_iter_->Next();
+  ASSERT_OK(c_iter_->status());
   ASSERT_FALSE(c_iter_->Valid());
 }
 
@@ -469,6 +474,7 @@ TEST_P(CompactionIteratorTest, CompactionFilterSkipUntil) {
   ASSERT_EQ(test::KeyStr("h", 91, kTypeValue), c_iter_->key().ToString());
   ASSERT_EQ("hv91", c_iter_->value().ToString());
   c_iter_->Next();
+  ASSERT_OK(c_iter_->status());
   ASSERT_FALSE(c_iter_->Valid());
 
   // Check that the compaction iterator did the correct sequence of calls on
@@ -662,6 +668,7 @@ TEST_P(CompactionIteratorTest, SingleMergeOperand) {
   ASSERT_TRUE(c_iter_->Valid());
   ASSERT_EQ("bv1bv2", c_iter_->value().ToString());
   c_iter_->Next();
+  ASSERT_OK(c_iter_->status());
   ASSERT_EQ("cv1cv2", c_iter_->value().ToString());
 }
 

db/compaction/compaction_job.cc

@@ -1545,6 +1545,9 @@ Status CompactionJob::OpenCompactionOutputFile(
   s = io_s;
   if (sub_compact->io_status.ok()) {
     sub_compact->io_status = io_s;
+    // Since this error is really a copy of the io_s that is checked below as s,
+    // it does not also need to be checked.
+    sub_compact->io_status.PermitUncheckedError();
   }
   if (!s.ok()) {
     ROCKS_LOG_ERROR(
@@ -1640,6 +1643,9 @@ void CompactionJob::CleanupCompaction() {
         TableCache::Evict(table_cache_.get(), out.meta.fd.GetNumber());
       }
     }
+    // TODO: sub_compact.io_status is not checked like status. Not sure if thats
+    // intentional. So ignoring the io_status as of now.
+    sub_compact.io_status.PermitUncheckedError();
   }
   delete compact_;
   compact_ = nullptr;

db/compaction/compaction_job_stats_test.cc

@@ -796,7 +796,7 @@ TEST_P(CompactionJobStatsTest, CompactionJobStatsTest) {
     }
 
     ASSERT_OK(Flush(1));
-    static_cast_with_check<DBImpl>(db_)->TEST_WaitForCompact();
+    ASSERT_OK(static_cast_with_check<DBImpl>(db_)->TEST_WaitForCompact());
 
     stats_checker->set_verify_next_comp_io_stats(true);
     std::atomic<bool> first_prepare_write(true);
@@ -894,7 +894,7 @@ TEST_P(CompactionJobStatsTest, DeletionStatsTest) {
   CompactRangeOptions cr_options;
   cr_options.change_level = true;
   cr_options.target_level = 2;
-  db_->CompactRange(cr_options, handles_[1], nullptr, nullptr);
+  ASSERT_OK(db_->CompactRange(cr_options, handles_[1], nullptr, nullptr));
   ASSERT_GT(NumTableFilesAtLevel(2, 1), 0);
 
   // Stage 2: Generate files including keys from the entire key range
@@ -1000,7 +1000,7 @@ TEST_P(CompactionJobStatsTest, UniversalCompactionTest) {
             num_input_units,
             num_keys_per_table * num_input_units,
             1.0, 0, false));
-    dbfull()->TEST_WaitForCompact();
+    ASSERT_OK(dbfull()->TEST_WaitForCompact());
   }
   ASSERT_EQ(stats_checker->NumberOfUnverifiedStats(), 3U);
 
@@ -1011,7 +1011,7 @@ TEST_P(CompactionJobStatsTest, UniversalCompactionTest) {
         &rnd, start_key, start_key + key_base - 1,
         kKeySize, kValueSize, key_interval,
         compression_ratio, 1);
-    static_cast_with_check<DBImpl>(db_)->TEST_WaitForCompact();
+    ASSERT_OK(static_cast_with_check<DBImpl>(db_)->TEST_WaitForCompact());
   }
   ASSERT_EQ(stats_checker->NumberOfUnverifiedStats(), 0U);
 }

db/compaction/compaction_job_test.cc

@@ -192,8 +192,9 @@ class CompactionJobTest : public testing::Test {
                  kUnknownFileChecksum, kUnknownFileChecksumFuncName);
 
     mutex_.Lock();
-    versions_->LogAndApply(versions_->GetColumnFamilySet()->GetDefault(),
-                           mutable_cf_options_, &edit, &mutex_);
+    EXPECT_OK(
+        versions_->LogAndApply(versions_->GetColumnFamilySet()->GetDefault(),
+                               mutable_cf_options_, &edit, &mutex_));
     mutex_.Unlock();
   }
 
@@ -284,6 +285,8 @@ class CompactionJobTest : public testing::Test {
     // Make "CURRENT" file that points to the new manifest file.
     s = SetCurrentFile(fs_.get(), dbname_, 1, nullptr);
 
+    ASSERT_OK(s);
+
     std::vector<ColumnFamilyDescriptor> column_families;
     cf_options_.table_factory = mock_table_factory_;
     cf_options_.merge_operator = merge_op_;
@@ -342,8 +345,10 @@ class CompactionJobTest : public testing::Test {
     Status s;
     s = compaction_job.Run();
     ASSERT_OK(s);
+    ASSERT_OK(compaction_job.io_status());
     mutex_.Lock();
     ASSERT_OK(compaction_job.Install(*cfd->GetLatestMutableCFOptions()));
+    ASSERT_OK(compaction_job.io_status());
     mutex_.Unlock();
 
     if (verify) {

db/db_impl/db_impl_compaction_flush.cc

@@ -916,7 +916,8 @@ Status DBImpl::CompactRange(const CompactRangeOptions& options,
       TEST_SYNC_POINT("DBImpl::CompactRange:PreRefitLevel");
       s = ReFitLevel(cfd, final_output_level, options.target_level);
       TEST_SYNC_POINT("DBImpl::CompactRange:PostRefitLevel");
-      ContinueBackgroundWork();
+      // ContinueBackgroundWork always return Status::OK().
+      assert(ContinueBackgroundWork().ok());
     }
     EnableManualCompaction();
   }
@@ -1260,6 +1261,7 @@ void DBImpl::NotifyOnCompactionBegin(ColumnFamilyData* cfd, Compaction* c,
     for (auto listener : immutable_db_options_.listeners) {
       listener->OnCompactionBegin(this, info);
     }
+    info.status.PermitUncheckedError();
   }
   mutex_.Lock();
   current->Unref();
@@ -2920,7 +2922,6 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
 
     NotifyOnCompactionBegin(c->column_family_data(), c.get(), status,
                             compaction_job_stats, job_context->job_id);
-
     mutex_.Unlock();
     TEST_SYNC_POINT_CALLBACK(
         "DBImpl::BackgroundCompaction:NonTrivial:BeforeRun", nullptr);

db/db_options_test.cc

@@ -900,7 +900,7 @@ TEST_F(DBOptionsTest, ChangeCompression) {
   ASSERT_OK(Put("foo", "foofoofoo"));
   ASSERT_OK(Put("bar", "foofoofoo"));
   ASSERT_OK(Flush());
-  dbfull()->TEST_WaitForCompact();
+  ASSERT_OK(dbfull()->TEST_WaitForCompact());
   ASSERT_TRUE(compacted);
   ASSERT_EQ(CompressionType::kNoCompression, compression_used);
   ASSERT_EQ(options.compression_opts.level, compression_opt_used.level);
@@ -918,7 +918,7 @@ TEST_F(DBOptionsTest, ChangeCompression) {
   ASSERT_OK(Put("foo", "foofoofoo"));
   ASSERT_OK(Put("bar", "foofoofoo"));
   ASSERT_OK(Flush());
-  dbfull()->TEST_WaitForCompact();
+  ASSERT_OK(dbfull()->TEST_WaitForCompact());
   ASSERT_TRUE(compacted);
   ASSERT_EQ(CompressionType::kSnappyCompression, compression_used);
   ASSERT_EQ(6, compression_opt_used.level);

db/memtable_list_test.cc

@@ -65,12 +65,14 @@ class MemTableListTest : public testing::Test {
   ~MemTableListTest() override {
     if (db) {
       std::vector<ColumnFamilyDescriptor> cf_descs(handles.size());
+#ifndef ROCKSDB_LITE
       for (int i = 0; i != static_cast<int>(handles.size()); ++i) {
-        handles[i]->GetDescriptor(&cf_descs[i]);
+        EXPECT_OK(handles[i]->GetDescriptor(&cf_descs[i]));
       }
+#endif  // !ROCKSDB_LITE
       for (auto h : handles) {
         if (h) {
-          db->DestroyColumnFamilyHandle(h);
+          EXPECT_OK(db->DestroyColumnFamilyHandle(h));
         }
       }
       handles.clear();
@@ -123,6 +125,7 @@ class MemTableListTest : public testing::Test {
     Status s = list->TryInstallMemtableFlushResults(
         cfd, mutable_cf_options, m, &dummy_prep_tracker, &versions, &mutex,
         file_num, to_delete, nullptr, &log_buffer, &flush_jobs_info, &io_s);
+    EXPECT_OK(io_s);
     return s;
   }
 

db/merge_helper.cc

@@ -140,15 +140,17 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
   ParsedInternalKey orig_ikey;
   bool succ = ParseInternalKey(original_key, &orig_ikey);
   assert(succ);
+  Status s;
   if (!succ) {
-    return Status::Corruption("Cannot parse key in MergeUntil");
+    s = Status::Corruption("Cannot parse key in MergeUntil");
+    return s;
   }
 
-  Status s;
   bool hit_the_next_user_key = false;
   for (; iter->Valid(); iter->Next(), original_key_is_iter = false) {
     if (IsShuttingDown()) {
-      return Status::ShutdownInProgress();
+      s = Status::ShutdownInProgress();
+      return s;
     }
 
     ParsedInternalKey ikey;
@@ -158,7 +160,8 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
       // stop at corrupted key
       if (assert_valid_internal_key_) {
         assert(!"Corrupted internal key not expected.");
-        return Status::Corruption("Corrupted internal key not expected.");
+        s = Status::Corruption("Corrupted internal key not expected.");
+        return s;
       }
       break;
     } else if (first_key) {
@@ -193,7 +196,7 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
       // the compaction iterator to write out the key we're currently at, which
       // is the put/delete we just encountered.
       if (keys_.empty()) {
-        return Status::OK();
+        return s;
       }
 
       // TODO(noetzli) If the merge operator returns false, we are currently
@@ -284,14 +287,14 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
         keys_.clear();
         merge_context_.Clear();
         has_compaction_filter_skip_until_ = true;
-        return Status::OK();
+        return s;
       }
     }
   }
 
   if (merge_context_.GetNumOperands() == 0) {
     // we filtered out all the merge operands
-    return Status::OK();
+    return s;
   }
 
   // We are sure we have seen this key's entire history if:

include/rocksdb/listener.h

@@ -250,6 +250,7 @@ struct CompactionFileInfo {
 };
 
 struct CompactionJobInfo {
+  ~CompactionJobInfo() { status.PermitUncheckedError(); }
   // the id of the column family where the compaction happened.
   uint32_t cf_id;
   // the name of the column family where the compaction happened.