Open hx235 opened 2 days ago
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@hx235 has updated the pull request. You must reimport the pull request before landing.
@hx235 has updated the pull request. You must reimport the pull request before landing.
@hx235 has updated the pull request. You must reimport the pull request before landing.
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@hx235 has updated the pull request. You must reimport the pull request before landing.
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
Context/Summary: When fault injection is enabled and db open fails because of that, crash test will retry open without injected error in order to proceed with a clean open: https://github.com/facebook/rocksdb/blob/9eebaf11cbd875435b572f05f0378ecdb761cc74/db_stress_tool/db_stress_test_base.cc#L3559 https://github.com/facebook/rocksdb/blob/9eebaf11cbd875435b572f05f0378ecdb761cc74/db_stress_tool/db_stress_test_base.cc#L3586-L3639
However, between openings under the same FaultInjectionTestFS object, we didn't reset FaultInjectionTestFS internal state. So internal state from previous open with injected error can affect the next open.
For example, considering the following: a.
FaultInjectionTestFS::dir_to_new_files_since_last_sync
records files that are created but not yet synced. b. When we create CURRENT, we will first create a temp file and rename it as "CURRENT". As part of the renaming, we will assertFaultInjectionTestFS::dir_to_new_files_since_last_sync
doesn't already have a file namedCURRENT
.(1) 1st open, with metadata write error
FaultInjectionTestFS::dir_to_new_files_since_last_sync_
https://github.com/facebook/rocksdb/blob/9eebaf11cbd875435b572f05f0378ecdb761cc74/utilities/fault_injection_fs.cc#L735SyncDir()
here https://github.com/facebook/rocksdb/blob/9eebaf11cbd875435b572f05f0378ecdb761cc74/file/filename.cc#L412 failed with injected metadata write error. Therefore, "CURRENT" file didn't get removed fromFaultInjectionTestFS::dir_to_new_files_since_last_sync_
as it would ifSyncDir()
succeeded https://github.com/facebook/rocksdb/blob/9eebaf11cbd875435b572f05f0378ecdb761cc74/utilities/fault_injection_fs.h#L344(2) 2st open
FaultInjectionTestFS::dir_to_new_files_since_last_sync_
already had a file called CURRENT. So https://github.com/facebook/rocksdb/blame/9eebaf11cbd875435b572f05f0378ecdb761cc74/utilities/fault_injection_fs.cc#L1003 will failThis PR fixed this by resetting FaultInjectionTestFS's internal state between such opening Bonus: a better name for the function to reset file related state in FaultInjectionTestFS (FaultInjectionTestFS::ResetFileRelatedState()); include a previously missing file state in the function
Test: Command constantly failed before the fix but passed after the PR running for 10 minutes