From d1b70b05a6bf1d1fcc1f74865e37f98980c3d3d1 Mon Sep 17 00:00:00 2001 From: anand76 Date: Sun, 11 Jul 2021 22:37:01 -0700 Subject: [PATCH] Avoid passing existing BG error to WriteStatusCheck (#8511) Summary: In ```DBImpl::WriteImpl()```, we call ```PreprocessWrite()``` which, among other things, checks the BG error and returns it set. This return status is later on passed to ```WriteStatusCheck()```, which calls ```SetBGError()```. This results in a spurious call, and info logs, on every user write request. We should avoid passing the ```PreprocessWrite()``` return status to ```WriteStatusCheck()```, as the former would have called ```SetBGError()``` already if it encountered any new errors, such as error when creating a new WAL file. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8511 Test Plan: Run existing tests Reviewed By: zhichao-cao Differential Revision: D29639917 Pulled By: anand1976 fbshipit-source-id: 19234163969e1645dbeb273712aaf5cd9ea2b182 --- HISTORY.md | 1 + db/db_impl/db_impl_write.cc | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index e6304896a..07d7425b9 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -7,6 +7,7 @@ * Blob file checksums are now printed in hexadecimal format when using the `manifest_dump` `ldb` command. * `GetLiveFilesMetaData()` now populates the `temperature`, `oldest_ancester_time`, and `file_creation_time` fields of its `LiveFileMetaData` results when the information is available. Previously these fields always contained zero indicating unknown. * Fix mismatches of OnCompaction{Begin,Completed} in case of DisableManualCompaction(). +* Fix continuous logging of an existing background error on every user write ### New Features * ldb has a new feature, `list_live_files_metadata`, that shows the live SST files, as well as their LSM storage level and the column family they belong to. diff --git a/db/db_impl/db_impl_write.cc b/db/db_impl/db_impl_write.cc index 6dcef6208..c2e8c0dc6 100644 --- a/db/db_impl/db_impl_write.cc +++ b/db/db_impl/db_impl_write.cc @@ -251,6 +251,7 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options, write_thread_.EnterAsBatchGroupLeader(&w, &write_group); IOStatus io_s; + Status pre_release_cb_status; if (status.ok()) { // Rules for when we can update the memtable concurrently // 1. supported by memtable @@ -361,7 +362,7 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options, writer->sequence, disable_memtable, writer->log_used, index++, pre_release_callback_cnt); if (!ws.ok()) { - status = ws; + status = pre_release_cb_status = ws; break; } } @@ -414,10 +415,13 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options, if (!w.CallbackFailed()) { if (!io_s.ok()) { + assert(pre_release_cb_status.ok()); IOStatusCheck(io_s); } else { - WriteStatusCheck(status); + WriteStatusCheck(pre_release_cb_status); } + } else { + assert(io_s.ok() && pre_release_cb_status.ok()); } if (need_log_sync) {