From 48d5aa9bab7ba84e796719b9ecfdc14ef0c30708 Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Fri, 2 Oct 2020 11:22:20 -0700 Subject: [PATCH] Enable status check for db_secondary_test (#7487) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7487 Test Plan: ASSERT_STATUS_CHECKED=1 make db_secondary_test ./db_secondary_test Reviewed By: zhichao-cao Differential Revision: D24071038 Pulled By: riversand963 fbshipit-source-id: e6600c0aecab71c1326b22af263e92bddee5f7ac --- Makefile | 1 + db/db_impl/db_impl_secondary.cc | 12 ++++++++++-- db/db_impl/db_secondary_test.cc | 19 +++++++++++-------- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index fdf8ce46e..2f5141aeb 100644 --- a/Makefile +++ b/Makefile @@ -591,6 +591,7 @@ ifdef ASSERT_STATUS_CHECKED db_with_timestamp_compaction_test \ db_options_test \ db_properties_test \ + db_secondary_test \ options_file_test \ defer_test \ filename_test \ diff --git a/db/db_impl/db_impl_secondary.cc b/db/db_impl/db_impl_secondary.cc index ce04f0663..c0572948e 100644 --- a/db/db_impl/db_impl_secondary.cc +++ b/db/db_impl/db_impl_secondary.cc @@ -192,6 +192,8 @@ Status DBImplSecondary::RecoverLogFiles( auto it = log_readers_.find(log_number); assert(it != log_readers_.end()); log::FragmentBufferedReader* reader = it->second->reader_; + Status* wal_read_status = it->second->status_; + assert(wal_read_status); // Manually update the file number allocation counter in VersionSet. versions_->MarkFileNumberUsed(log_number); @@ -203,13 +205,16 @@ Status DBImplSecondary::RecoverLogFiles( while (reader->ReadRecord(&record, &scratch, immutable_db_options_.wal_recovery_mode) && - status.ok()) { + wal_read_status->ok() && status.ok()) { if (record.size() < WriteBatchInternal::kHeader) { reader->GetReporter()->Corruption( record.size(), Status::Corruption("log record too small")); continue; } - WriteBatchInternal::SetContents(&batch, record); + status = WriteBatchInternal::SetContents(&batch, record); + if (!status.ok()) { + break; + } SequenceNumber seq_of_batch = WriteBatchInternal::Sequence(&batch); std::vector column_family_ids; status = CollectColumnFamilyIdsFromWriteBatch(batch, &column_family_ids); @@ -295,6 +300,9 @@ Status DBImplSecondary::RecoverLogFiles( reader->GetReporter()->Corruption(record.size(), status); } } + if (status.ok() && !wal_read_status->ok()) { + status = *wal_read_status; + } if (!status.ok()) { return status; } diff --git a/db/db_impl/db_secondary_test.cc b/db/db_impl/db_secondary_test.cc index 0d00e3de9..23dc63aca 100644 --- a/db/db_impl/db_secondary_test.cc +++ b/db/db_impl/db_secondary_test.cc @@ -52,7 +52,7 @@ class DBSecondaryTest : public DBTestBase { void CloseSecondary() { for (auto h : handles_secondary_) { - db_secondary_->DestroyColumnFamilyHandle(h); + ASSERT_OK(db_secondary_->DestroyColumnFamilyHandle(h)); } handles_secondary_.clear(); delete db_secondary_; @@ -97,7 +97,7 @@ void DBSecondaryTest::CheckFileTypeCounts(const std::string& dir, int expected_log, int expected_sst, int expected_manifest) const { std::vector filenames; - env_->GetChildren(dir, &filenames); + ASSERT_OK(env_->GetChildren(dir, &filenames)); int log_cnt = 0, sst_cnt = 0, manifest_cnt = 0; for (auto file : filenames) { @@ -777,8 +777,8 @@ TEST_F(DBSecondaryTest, CatchUpAfterFlush) { WriteOptions write_opts; WriteBatch wb; - wb.Put("key0", "value0"); - wb.Put("key1", "value1"); + ASSERT_OK(wb.Put("key0", "value0")); + ASSERT_OK(wb.Put("key1", "value1")); ASSERT_OK(dbfull()->Write(write_opts, &wb)); ReadOptions read_opts; std::unique_ptr iter1(db_secondary_->NewIterator(read_opts)); @@ -791,25 +791,27 @@ TEST_F(DBSecondaryTest, CatchUpAfterFlush) { ASSERT_FALSE(iter1->Valid()); iter1->Seek("key1"); ASSERT_FALSE(iter1->Valid()); + ASSERT_OK(iter1->status()); std::unique_ptr iter2(db_secondary_->NewIterator(read_opts)); iter2->Seek("key0"); ASSERT_TRUE(iter2->Valid()); ASSERT_EQ("value0", iter2->value()); iter2->Seek("key1"); ASSERT_TRUE(iter2->Valid()); + ASSERT_OK(iter2->status()); ASSERT_EQ("value1", iter2->value()); { WriteBatch wb1; - wb1.Put("key0", "value01"); - wb1.Put("key1", "value11"); + ASSERT_OK(wb1.Put("key0", "value01")); + ASSERT_OK(wb1.Put("key1", "value11")); ASSERT_OK(dbfull()->Write(write_opts, &wb1)); } { WriteBatch wb2; - wb2.Put("key0", "new_value0"); - wb2.Delete("key1"); + ASSERT_OK(wb2.Put("key0", "new_value0")); + ASSERT_OK(wb2.Delete("key1")); ASSERT_OK(dbfull()->Write(write_opts, &wb2)); } @@ -823,6 +825,7 @@ TEST_F(DBSecondaryTest, CatchUpAfterFlush) { ASSERT_EQ("new_value0", iter3->value()); iter3->Seek("key1"); ASSERT_FALSE(iter3->Valid()); + ASSERT_OK(iter3->status()); } TEST_F(DBSecondaryTest, CheckConsistencyWhenOpen) {