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

Fix not getting expected injected read error #12793

Closed hx235 closed 1 week ago

hx235 commented 1 week ago

Context/Summary:

https://github.com/facebook/rocksdb/pull/12713 accidentally removed the mechanism of ignoring injected read error on non-critical read path such as in read from filter. IO failure in read from filter should not fail the read as we can always read from the actual file. Therefore error injection in filter read path does not need to lead to a failure in Get() and crash test should allow that. Otherwise, we will get crash test error "Didn't get expected error from..."

Test: CI

facebook-github-bot commented 1 week ago

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

hx235 commented 1 week ago

Pre-existing CI errors

hx235 commented 1 week ago

Pre-existing CI errors and irrelevant db crash test error

hx235 commented 1 week ago

Would we inject error multiple times in the same Get()?

Yes

If so, if we inject an error for both non-critical read and critical read path, I think for now we just don't check error is returned in this case?

Correct

facebook-github-bot commented 1 week ago

@hx235 merged this pull request in facebook/rocksdb@981fd432fa2441fc10a59a462bd14906ccb1c0e0.