From 5a61e7864d04e7e72b62749ca07bdb94af85ca87 Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Mon, 4 May 2020 17:43:33 -0700 Subject: [PATCH] Fix db_stress when GetLiveFiles() flushes dropped CF (#6805) Summary: Current impl. of db_stress will abort verification and report failure if GetLiveFiles() causes a dropped column family to be flushed. This is not desired. To fix, this PR makes the following change: In GetLiveFiles, if flush is triggered and returns Status::IsColumnFamilyDropped(), then set status to Status::OK(). This is OK because dropped column families will be skipped during the rest of this function, and valid column families will have their live files returned to caller. Test plan (dev server): make check ./db_stress -ops_per_thread=1000 -get_live_files_one_in=100 -clear_column_family_one_in=100 ./db_stress -disable_wal=1 -reopen=0 -ops_per_thread=1000 -get_live_files_one_in=100 -clear_column_family_one_in=100 Pull Request resolved: https://github.com/facebook/rocksdb/pull/6805 Reviewed By: ltamasi Differential Revision: D21390044 Pulled By: riversand963 fbshipit-source-id: de67846b95a4f1b88aa0a30c3d70c43cc68625b9 --- HISTORY.md | 1 + db/db_filesnapshot.cc | 7 ++++++- db/db_impl/db_impl_compaction_flush.cc | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 17e13f3c8..cd8370b92 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -15,6 +15,7 @@ * Add a ConfigOptions argument to the APIs dealing with converting options to and from strings and files. The ConfigOptions is meant to replace some of the options (such as input_strings_escaped and ignore_unknown_options) and allow for more parameters to be passed in the future without changing the function signature. * Add NewFileChecksumGenCrc32cFactory to the file checksum public API, such that the builtin Crc32c based file checksum generator factory can be used by applications. * Add IsDirectory to Env and FS to indicate if a path is a directory. +* Flush(..., column_family) may return Status::ColumnFamilyDropped() instead of Status::InvalidArgument() if column_family is dropped while processing the flush request. ### New Features * Added support for pipelined & parallel compression optimization for `BlockBasedTableBuilder`. This optimization makes block building, block compression and block appending a pipeline, and uses multiple threads to accelerate block compression. Users can set `CompressionOptions::parallel_threads` greater than 1 to enable compression parallelism. This feature is experimental for now. diff --git a/db/db_filesnapshot.cc b/db/db_filesnapshot.cc index bb6199827..8af226c31 100644 --- a/db/db_filesnapshot.cc +++ b/db/db_filesnapshot.cc @@ -90,6 +90,9 @@ Status DBImpl::GetLiveFiles(std::vector& ret, mutex_.Unlock(); status = AtomicFlushMemTables(cfds, FlushOptions(), FlushReason::kGetLiveFiles); + if (status.IsColumnFamilyDropped()) { + status = Status::OK(); + } mutex_.Lock(); } else { for (auto cfd : *versions_->GetColumnFamilySet()) { @@ -103,8 +106,10 @@ Status DBImpl::GetLiveFiles(std::vector& ret, TEST_SYNC_POINT("DBImpl::GetLiveFiles:2"); mutex_.Lock(); cfd->UnrefAndTryDelete(); - if (!status.ok()) { + if (!status.ok() && !status.IsColumnFamilyDropped()) { break; + } else if (status.IsColumnFamilyDropped()) { + status = Status::OK(); } } } diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc index b01143f45..016776d50 100644 --- a/db/db_impl/db_impl_compaction_flush.cc +++ b/db/db_impl/db_impl_compaction_flush.cc @@ -1906,7 +1906,7 @@ Status DBImpl::WaitForFlushMemTables( } } if (1 == num_dropped && 1 == num) { - return Status::InvalidArgument("Cannot flush a dropped CF"); + return Status::ColumnFamilyDropped(); } // Column families involved in this flush request have either been dropped // or finished flush. Then it's time to finish waiting.