rocksdb/utilities
Hui Xiao cff7819dff Fix BackupEngine's internal callers of GenericRateLimiter::Request() not honoring bytes <= GetSingleBurstBytes() (#9063)
Summary:
**Context:**
Some existing internal calls of `GenericRateLimiter::Request()` in backupable_db.cc and newly added internal calls in https://github.com/facebook/rocksdb/pull/8722/ do not make sure `bytes <= GetSingleBurstBytes()` as required by rate_limiter https://github.com/facebook/rocksdb/blob/master/include/rocksdb/rate_limiter.h#L47.

**Impacts of this bug include:**
(1) In debug build, when `GenericRateLimiter::Request()` requests bytes greater than `GenericRateLimiter:: kMinRefillBytesPerPeriod = 100` byte, process will crash due to assertion failure. See https://github.com/facebook/rocksdb/pull/9063#discussion_r737034133 and for possible scenario
(2) In production build, although there will not be the above crash due to disabled assertion, the bug can lead to a request of small bytes being blocked for a long time by a request of same priority with insanely large bytes from a different thread. See updated https://github.com/facebook/rocksdb/wiki/Rate-Limiter ("Notice that although....the maximum bytes that can be granted in a single request have to be bounded...") for more info.

There is an on-going effort to move rate-limiting to file wrapper level so rate limiting in `BackupEngine` and this PR might be made obsolete in the future.

**Summary:**
- Implemented loop-calling `GenericRateLimiter::Request()` with `bytes <= GetSingleBurstBytes()` as a static private helper function `BackupEngineImpl::LoopRateLimitRequestHelper`
   -- Considering make this a util function in `RateLimiter` later or do something with `RateLimiter::RequestToken()`
- Replaced buggy internal callers with this helper function wherever requested byte is not pre-limited by `GetSingleBurstBytes()`
- Removed the minimum refill bytes per period enforced by `GenericRateLimiter` since it is useless and prevents testing `GenericRateLimiter` for extreme case with small refill bytes per period.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9063

Test Plan:
- Added a new test that failed the assertion before this change and now passes
  - It exposed bugs in [the write during creation in `CopyOrCreateFile()`](df7cc66e17/utilities/backupable/backupable_db.cc (L2034-L2043)), [the read of table properties in `GetFileDbIdentities()`](df7cc66e17/utilities/backupable/backupable_db.cc (L2372-L2378)), [some read of metadata in `BackupMeta::LoadFromFile()`](df7cc66e17/utilities/backupable/backupable_db.cc (L2726))
- Passing Existing tests

Reviewed By: ajkr

Differential Revision: D31824535

Pulled By: hx235

fbshipit-source-id: d2b3dea7a64e2a4b1e6a59fca322f0800a4fcbcc
2021-11-16 09:52:16 -08:00
..
backupable Fix BackupEngine's internal callers of GenericRateLimiter::Request() not honoring bytes <= GetSingleBurstBytes() (#9063) 2021-11-16 09:52:16 -08:00
blob_db Skip directory fsync for filesystem btrfs (#8903) 2021-11-03 12:21:27 -07:00
cassandra Fix and detect headers with missing dependencies (#8893) 2021-09-10 10:00:26 -07:00
checkpoint Skip directory fsync for filesystem btrfs (#8903) 2021-11-03 12:21:27 -07:00
compaction_filters Make MergeOperator+CompactionFilter/Factory into Customizable Classes (#8481) 2021-08-06 08:27:25 -07:00
convenience Add a SystemClock class to capture the time functions of an Env (#7858) 2021-01-25 22:09:11 -08:00
leveldb_options Replace namespace name "rocksdb" with ROCKSDB_NAMESPACE (#6433) 2020-02-20 12:09:57 -08:00
memory Make types of Immutable/Mutable Options fields match that of the underlying Option (#8176) 2021-04-22 20:43:54 -07:00
merge_operators Replace most typedef with using= (#8751) 2021-09-07 11:31:59 -07:00
option_change_migration Fix a minor issue with initializing the test path (#8555) 2021-07-23 08:38:45 -07:00
options Allow WAL dir to change with db dir (#8582) 2021-07-30 12:16:44 -07:00
persistent_cache Improve support for using regexes (#8740) 2021-09-07 13:05:23 -07:00
simulator_cache Make TraceRecord and Replayer public (#8611) 2021-08-11 19:32:46 -07:00
table_properties_collectors Make WalFilter, SstPartitionerFactory, FileChecksumGenFactory, and TableProperties Customizable (#8638) 2021-09-28 05:32:02 -07:00
trace Fix and detect headers with missing dependencies (#8893) 2021-09-10 10:00:26 -07:00
transactions Update TransactionUtil::CheckKeyForConflict to also use timestamps (#9162) 2021-11-15 12:52:18 -08:00
ttl Cleanup includes in dbformat.h (#8930) 2021-09-29 04:04:40 -07:00
write_batch_with_index Fix small issues (#5896) 2021-11-08 12:32:38 -08:00
cache_dump_load_impl.cc Introduce a mechanism to dump out blocks from block cache and re-insert to secondary cache (#8912) 2021-10-07 11:42:31 -07:00
cache_dump_load_impl.h Initialize cache dumper DumpUnit in constructor (#9014) 2021-10-11 13:05:35 -07:00
cache_dump_load.cc Introduce a mechanism to dump out blocks from block cache and re-insert to secondary cache (#8912) 2021-10-07 11:42:31 -07:00
compaction_filters.cc Make MergeOperator+CompactionFilter/Factory into Customizable Classes (#8481) 2021-08-06 08:27:25 -07:00
debug.cc In ParseInternalKey(), include corrupt key info in Status (#7515) 2020-10-28 10:12:58 -07:00
env_librados_test.cc Fix EnvLibrados and add to CI (#9088) 2021-10-29 08:19:03 -07:00
env_librados.cc Fix EnvLibrados and add to CI (#9088) 2021-10-29 08:19:03 -07:00
env_librados.md Update branch name to main in env_librados.md (#8738) 2021-09-01 14:28:58 -07:00
env_mirror_test.cc Replace namespace name "rocksdb" with ROCKSDB_NAMESPACE (#6433) 2020-02-20 12:09:57 -08:00
env_mirror.cc Add new Append API with DataVerificationInfo to Env WritableFile (#8071) 2021-03-19 11:44:13 -07:00
env_timed_test.cc Make env*_test work with ASSERT_STATUS_CHECKED (#7176) 2020-07-28 22:59:48 -07:00
env_timed.cc Make FileSystem a Customizable Class (#8649) 2021-11-02 09:07:11 -07:00
env_timed.h Make FileSystem a Customizable Class (#8649) 2021-11-02 09:07:11 -07:00
fault_injection_env.cc Protect existing files in FaultInjectionTest{Env,FS}::ReopenWritableFile() (#8995) 2021-10-11 16:23:18 -07:00
fault_injection_env.h Protect existing files in FaultInjectionTest{Env,FS}::ReopenWritableFile() (#8995) 2021-10-11 16:23:18 -07:00
fault_injection_fs.cc Skip directory fsync for filesystem btrfs (#8903) 2021-11-03 12:21:27 -07:00
fault_injection_fs.h Skip directory fsync for filesystem btrfs (#8903) 2021-11-03 12:21:27 -07:00
fault_injection_secondary_cache.cc Secondary cache error injection (#9002) 2021-11-08 10:27:27 -08:00
fault_injection_secondary_cache.h Secondary cache error injection (#9002) 2021-11-08 10:27:27 -08:00
merge_operators.cc Make MergeOperator+CompactionFilter/Factory into Customizable Classes (#8481) 2021-08-06 08:27:25 -07:00
merge_operators.h Make MergeOperator+CompactionFilter/Factory into Customizable Classes (#8481) 2021-08-06 08:27:25 -07:00
object_registry_test.cc Add support to the ObjectRegistry for ManagedObjects (#8658) 2021-09-10 05:21:04 -07:00
object_registry.cc Add support to the ObjectRegistry for ManagedObjects (#8658) 2021-09-10 05:21:04 -07:00
util_merge_operators_test.cc Replace namespace name "rocksdb" with ROCKSDB_NAMESPACE (#6433) 2020-02-20 12:09:57 -08:00
wal_filter.cc Make WalFilter, SstPartitionerFactory, FileChecksumGenFactory, and TableProperties Customizable (#8638) 2021-09-28 05:32:02 -07:00