Secondary cache error injection #9002
Conversation
| @@ -0,0 +1,110 @@ | |||
| // Copyright (c) 2011-present, Facebook, Inc. All rights reserved. | |||
There was a problem hiding this comment.
Maybe just use "secondary" instead of "sec" will be easier to remember and search
| const Cache::CacheItemHelper* helper) { | ||
| ErrorContext* ctx = GetErrorContext(); | ||
| if (ctx->rand.OneIn(prob_)) { | ||
| return Status::IOError(); |
There was a problem hiding this comment.
Should we return Corrupted? Or IOError is better. I'm not sure what we expect from the implementation layer of secondary cache. We may also need to consider to unify the error code returned in secondary cache since it uses the storage and IOError might make sense in some cases.
There was a problem hiding this comment.
All insert errors are currently treated the same by RocksDB, so it doesn't matter much what the error is. We can always add more here if the implementation changes.
| SecondaryCache* cache_; | ||
| }; | ||
|
|
||
| class FaultInjectionSecondaryCache : public SecondaryCache { |
There was a problem hiding this comment.
Maybe some comments about the general behavior of the fault injection and how it will be used for testing.
| } | ||
|
|
||
| void FaultInjectionSecondaryCache::ResultHandle::WaitAll( | ||
| FaultInjectionSecondaryCache* cache, |
There was a problem hiding this comment.
We do not inject error in WaitAll?
There was a problem hiding this comment.
We do inject error in WaitAll. Its done in FaultInjectionSecondaryCache::ResultHandle::UpdateHandleValue.
anand1976
force-pushed
the
sec_cache_error
branch
from
81c49b4
to
fd84169
Compare
|
@anand1976 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
| @@ -841,6 +841,9 @@ DEFINE_int32(open_metadata_write_fault_one_in, 0, | |||
| #ifndef ROCKSDB_LITE | |||
| DEFINE_string(secondary_cache_uri, "", | |||
| "Full URI for creating a customized secondary cache object"); | |||
| DEFINE_int32(secondary_cache_fault_one_in, 0, | |||
There was a problem hiding this comment.
why don't use DEFINE_bool ?
There was a problem hiding this comment.
Its the probability, not a flag.
anand1976
force-pushed
the
sec_cache_error
branch
from
fd84169
to
a5c3d78
Compare
|
@anand1976 has updated the pull request. You must reimport the pull request before landing. |
|
@anand1976 has updated the pull request. You must reimport the pull request before landing. |
|
@anand1976 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@anand1976 has updated the pull request. You must reimport the pull request before landing. |
|
@anand1976 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@anand1976 merged this pull request in 78556c1. |
Implement secondary cache error injection in db_stress.