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
main
anand76 3 years ago committed by Facebook GitHub Bot
parent 837705ad80
commit d1b70b05a6
  1. 1
      HISTORY.md
  2. 8
      db/db_impl/db_impl_write.cc

@ -7,6 +7,7 @@
* Blob file checksums are now printed in hexadecimal format when using the `manifest_dump` `ldb` command. * 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. * `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 mismatches of OnCompaction{Begin,Completed} in case of DisableManualCompaction().
* Fix continuous logging of an existing background error on every user write
### New Features ### 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. * 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.

@ -251,6 +251,7 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
write_thread_.EnterAsBatchGroupLeader(&w, &write_group); write_thread_.EnterAsBatchGroupLeader(&w, &write_group);
IOStatus io_s; IOStatus io_s;
Status pre_release_cb_status;
if (status.ok()) { if (status.ok()) {
// Rules for when we can update the memtable concurrently // Rules for when we can update the memtable concurrently
// 1. supported by memtable // 1. supported by memtable
@ -361,7 +362,7 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
writer->sequence, disable_memtable, writer->log_used, index++, writer->sequence, disable_memtable, writer->log_used, index++,
pre_release_callback_cnt); pre_release_callback_cnt);
if (!ws.ok()) { if (!ws.ok()) {
status = ws; status = pre_release_cb_status = ws;
break; break;
} }
} }
@ -414,10 +415,13 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
if (!w.CallbackFailed()) { if (!w.CallbackFailed()) {
if (!io_s.ok()) { if (!io_s.ok()) {
assert(pre_release_cb_status.ok());
IOStatusCheck(io_s); IOStatusCheck(io_s);
} else { } else {
WriteStatusCheck(status); WriteStatusCheck(pre_release_cb_status);
} }
} else {
assert(io_s.ok() && pre_release_cb_status.ok());
} }
if (need_log_sync) { if (need_log_sync) {

Loading…
Cancel
Save