[db_stress] Add env_fault_injection argument to db_stress #6687
Conversation
|
oh I just remembered there's also ongoing work to add fault injection on the read side of things - #6538. Do you mind checking this change is consistent with what's going on over there? |
|
@anand1976 This PR may related to the one you are working on |
|
@ajkr : Sorry for the late reply. Thanks for the info! I will take a look. |
There was a problem hiding this comment.
@yhchiang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
db_stress_tool/db_stress_gflags.cc
Outdated
| @@ -674,4 +674,8 @@ DEFINE_int32(approximate_size_one_in, 64, | |||
|
|
|||
| DEFINE_int32(read_fault_one_in, 1000, | |||
| "On non-zero, enables fault injection on read"); | |||
|
|
|||
| DEFINE_bool(env_fault_injection, false, | |||
There was a problem hiding this comment.
Can we leave env out of the name? Maybe name it write_fault_injection or sync_fault_injection. The reason is we're moving to the FileSystem class for file related APIs, and this might become a bit of a misnomer.
There was a problem hiding this comment.
Sure. Let me update to apply only FaultInjectionFS.
db_stress_tool/db_stress_tool.cc
Outdated
| @@ -84,6 +85,16 @@ int db_stress_tool(int argc, char** argv) { | |||
| raw_env = fault_env_guard.get(); | |||
| } | |||
| #endif | |||
|
|
|||
| if (FLAGS_env_fault_injection) { | |||
There was a problem hiding this comment.
Can we combine this block with the one above and use FaultInjectionTestFS, which offers the same functionality?
|
@yhchiang has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@yhchiang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@yhchiang has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@yhchiang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@yhchiang has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@yhchiang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@yhchiang has updated the pull request. Re-import the pull request |
|
@yhchiang has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@yhchiang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@yhchiang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@yhchiang has updated the pull request. Re-import the pull request |
|
@yhchiang has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@yhchiang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@yhchiang has updated the pull request. Re-import the pull request |
Summary: Add sync_fault_injection argument to db_stress. When enabled, FaultInjectionTestFS will be used with SetFilesystemDirectWritable(true). As db_stress will not be able to successfully finish its initial open / recovery with SetFilesystemDirectWritable(true), SetFilesystemDirectWritable(false) is used at the beginning of db_stress, and switch to SetFilesystemDirectWritable(true) after the initial open / recovery is done. Test Plan: make db_stress -j32 Run db_stress with both sync_fault_injection on and off. Observed db_stress finishes successfully, and DirectWritable is disabled when sync_fault_injection is on. ./db_stress --ops_per_thread=100 --reopen=10 --compression_type=zlib --sync_fault_injection=1 ./db_stress --ops_per_thread=100 --reopen=10 --compression_type=zlib --sync_fault_injection=0 Reviewers: ajkr Subscribers: rocksdb Tasks: T37386000 Tags: rocksdb
|
@yhchiang has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@yhchiang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary:
Add env_fault_injection argument to db_stress. When enabled,
FaultInjectionTestEnv will be used instead. Currently this
option does not support running with other env setting.
This will allow
us to later manually produce error when running db_crashtest.
Test Plan:
make db_stress -j32
./db_stress --env_fault_injection
./db_stress --env_fault_injection --hdfs // expect error message
Reviewers: ajkr
Subscribers: rocksdb
Tasks: T37386000
Tags: rocksdb