Skip to content

Commit c52cd52

Browse files
[CP-to-6.29.tikv] rate-limiter: fix clock skew if enabling auto-tuned. (#401) (#402)
close tikv/tikv#17995 Signed-off-by: lucasliang <[email protected]>
1 parent ee26b5c commit c52cd52

File tree

2 files changed

+43
-10
lines changed

2 files changed

+43
-10
lines changed

utilities/rate_limiters/write_amp_based_rate_limiter.cc

+29-10
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,11 @@ void WriteAmpBasedRateLimiter::Request(int64_t bytes, const Env::IOPriority pri,
189189
(!queue_[Env::IO_LOW].empty() && &r == queue_[Env::IO_LOW].front()))) {
190190
leader_ = &r;
191191
int64_t delta = next_refill_us_ - NowMicrosMonotonic(env_);
192-
delta = delta > 0 ? delta : 0;
192+
// Clamp delta between 0 and refill_period_us_:
193+
// (1) set negative values to 0
194+
// (2) cap maximum wait time to refill_period_us_ to prevent excessive
195+
// delays that could occur due to clock skew.
196+
delta = delta > 0 ? std::min(delta, refill_period_us_) : 0;
193197
if (delta == 0) {
194198
timedout = true;
195199
} else {
@@ -325,20 +329,35 @@ Status WriteAmpBasedRateLimiter::Tune() {
325329
// lower bound for write amplification estimation
326330
const int kRatioLower = 10;
327331
const int kPercentDeltaMax = 6;
332+
const auto millis_per_tune = 1000 * secs_per_tune_;
333+
// Define the max limit of tick duration limits to handle clock skew.
334+
const auto max_tune_tick_duration_limit =
335+
std::chrono::microseconds(secs_per_tune_ * 1000 * 1000) * 7 /
336+
4; // 1.75x multiplier
328337

329-
std::chrono::microseconds prev_tuned_time = tuned_time_;
338+
const std::chrono::microseconds prev_tuned_time = tuned_time_;
330339
tuned_time_ = std::chrono::microseconds(NowMicrosMonotonic(env_));
331-
auto duration = tuned_time_ - prev_tuned_time;
332-
auto duration_ms =
333-
std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();
340+
// Validate tuning interval to detect system anomalies:
341+
// (1) tuned_time_ < prev_tuned_time: Clock moved backwards (clock skew)
342+
// (2) Interval > max_tune_tick_duration_limit: System stall or severe clock
343+
// skew
344+
if (tuned_time_ <= prev_tuned_time ||
345+
tuned_time_ >= prev_tuned_time + max_tune_tick_duration_limit) {
346+
// Fall back to max rate limiter for safety if duration is invalid or
347+
// exceeds max limit.
348+
SetActualBytesPerSecond(max_bytes_per_sec_.load(std::memory_order_relaxed));
349+
return Status::Aborted();
350+
}
351+
auto duration_ms = std::chrono::duration_cast<std::chrono::milliseconds>(
352+
tuned_time_ - prev_tuned_time)
353+
.count();
334354

335355
int64_t prev_bytes_per_sec = GetBytesPerSecond();
336-
337356
// This function can be called less frequent than we anticipate when
338357
// compaction rate is low. Loop through the actual time slice to correct
339358
// the estimation.
340-
auto millis_per_tune = 1000 * secs_per_tune_;
341-
for (uint32_t i = 0; i < duration_ms / millis_per_tune; i++) {
359+
auto sampling_count = duration_ms / millis_per_tune;
360+
for (uint32_t i = 0; i < sampling_count; i++) {
342361
bytes_sampler_.AddSample(duration_bytes_through_ * 1000 / duration_ms);
343362
highpri_bytes_sampler_.AddSample(duration_highpri_bytes_through_ * 1000 /
344363
duration_ms);
@@ -407,8 +426,8 @@ void WriteAmpBasedRateLimiter::PaceUp(bool critical) {
407426
}
408427

409428
RateLimiter* NewWriteAmpBasedRateLimiter(
410-
int64_t rate_bytes_per_sec, int64_t refill_period_us /* = 100 * 1000 */,
411-
int32_t fairness /* = 10 */,
429+
int64_t rate_bytes_per_sec /* = 10GiB */,
430+
int64_t refill_period_us /* = 100 * 1000 */, int32_t fairness /* = 10 */,
412431
RateLimiter::Mode mode /* = RateLimiter::Mode::kWritesOnly */,
413432
bool auto_tuned /* = false */, int tune_per_sec /* = 1 */,
414433
size_t smooth_window_size /* = 300 */,

utilities/rate_limiters/write_amp_based_rate_limiter_test.cc

+14
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,20 @@ TEST_F(WriteAmpBasedRateLimiterTest, AutoTune) {
7878
limiter.Request(1000 /* bytes */, Env::IO_LOW, nullptr /* stats */,
7979
RateLimiter::OpType::kWrite);
8080
ASSERT_EQ(10485760, limiter.GetBytesPerSecond());
81+
// If there exists clock skew issues, the next Tune()
82+
// should be skipped and use the max_rate_bytes_per_sec
83+
// as the next limit.
84+
thread_env->SleepForMicroseconds(2000 * 1000);
85+
limiter.Request(1000 /* bytes */, Env::IO_LOW, nullptr /* stats */,
86+
RateLimiter::OpType::kWrite);
87+
ASSERT_EQ(10000, limiter.GetBytesPerSecond());
88+
// After recovering, the auto-tune works normally.
89+
for (auto i = 1; i <= 3; i++) {
90+
thread_env->SleepForMicroseconds(1000 * 1000);
91+
limiter.Request(1000 /* bytes */, Env::IO_LOW, nullptr /* stats */,
92+
RateLimiter::OpType::kWrite);
93+
ASSERT_EQ(10485760, limiter.GetBytesPerSecond());
94+
}
8195
// TODO: add more logic for auto-tune
8296
}
8397

0 commit comments

Comments
 (0)