search

facebook / rocksdb

A library that provides an embeddable, persistent key-value store for fast storage.
http://rocksdb.org
GNU General Public License v2.0
27.83k stars 6.2k forks source link

Remove the return value of `SetBGError()` #12792

Closed cbi42 closed 1 week ago

cbi42 commented 1 week ago

Summary: the return value for ErrorHandler::SetBGError(error) seems to be not well-defined, it can be bg_error_ (no matter if the bg_error_ is set to the input error), ok status or recovery_error_ from StartRecoverFromRetryableBGIOError(). The recovery_error_ returned may be an OK status.

We have only a few places that use the return value of SetBGError() and they don't need to do so. Using the return value may even be wrong for example in https://github.com/facebook/rocksdb/blob/3ee4d5a11a882056b341a9a1694a71371a39f664/db/db_impl/db_impl_write.cc#L2365 where a non-ok s could be overwritten to OK. This PR changes SetBGError() to return void and clean up relevant code.

Test plan: existing unit tests and go over all places where return value of SetBGError() is used.

facebook-github-bot commented 1 week ago

@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 1 week ago

@cbi42 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 week ago

@cbi42 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 1 week ago

@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 1 week ago

@cbi42 merged this pull request in facebook/rocksdb@a31fe521732c6150003ea43f1e30f27f13be597c.