Skip to content

Commit

Permalink
Handle overflow case of rate limiter's paramters
Browse files Browse the repository at this point in the history
Summary: When rate_bytes_per_sec * refill_period_us_ overflows, the actual limited rate is very low. Handle this case so the rate will be large.

Test Plan: Add a unit test for it.

Reviewers: IslamAbdelRahman, andrewkr

Reviewed By: andrewkr

Subscribers: yiwu, lightmark, leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D58929
  • Loading branch information
siying committed May 27, 2016
1 parent 57461fb commit f62fbd2
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 4 deletions.
1 change: 1 addition & 0 deletions port/port_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ namespace port {
// For use at db/file_indexer.h kLevelMaxIndex
const int kMaxInt32 = std::numeric_limits<int32_t>::max();
const uint64_t kMaxUint64 = std::numeric_limits<uint64_t>::max();
const int64_t kMaxInt64 = std::numeric_limits<int64_t>::max();
const size_t kMaxSizet = std::numeric_limits<size_t>::max();

static const bool kLittleEndian = PLATFORM_IS_LITTLE_ENDIAN;
Expand Down
2 changes: 2 additions & 0 deletions port/win/port_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ namespace port {
// For use at db/file_indexer.h kLevelMaxIndex
const int kMaxInt32 = std::numeric_limits<int>::max();
const uint64_t kMaxUint64 = std::numeric_limits<uint64_t>::max();
const int64_t kMaxInt64 = std::numeric_limits<int64_t>::max();

const size_t kMaxSizet = std::numeric_limits<size_t>::max();

Expand All @@ -94,6 +95,7 @@ const size_t kMaxSizet = std::numeric_limits<size_t>::max();

// For use at db/file_indexer.h kLevelMaxIndex
const int kMaxInt32 = INT32_MAX;
const int64_t kMaxInt64 = INT64_MAX;
const uint64_t kMaxUint64 = UINT64_MAX;

#ifdef _WIN64
Expand Down
13 changes: 13 additions & 0 deletions util/rate_limiter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// found in the LICENSE file. See the AUTHORS file for names of contributors.

#include "util/rate_limiter.h"
#include "port/port.h"
#include "rocksdb/env.h"

namespace rocksdb {
Expand Down Expand Up @@ -204,6 +205,18 @@ void GenericRateLimiter::Refill() {
}
}

int64_t GenericRateLimiter::CalculateRefillBytesPerPeriod(
int64_t rate_bytes_per_sec) {
if (port::kMaxInt64 / rate_bytes_per_sec < refill_period_us_) {
// Avoid unexpected result in the overflow case. The result now is still
// inaccurate but is a number that is large enough.
return port::kMaxInt64 / 1000000;
} else {
return std::max(kMinRefillBytesPerPeriod,
rate_bytes_per_sec * refill_period_us_ / 1000000);
}
}

RateLimiter* NewGenericRateLimiter(
int64_t rate_bytes_per_sec, int64_t refill_period_us, int32_t fairness) {
assert(rate_bytes_per_sec > 0);
Expand Down
5 changes: 1 addition & 4 deletions util/rate_limiter.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ class GenericRateLimiter : public RateLimiter {

private:
void Refill();
int64_t CalculateRefillBytesPerPeriod(int64_t rate_bytes_per_sec) {
return std::max(kMinRefillBytesPerPeriod,
rate_bytes_per_sec * refill_period_us_ / 1000000);
}
int64_t CalculateRefillBytesPerPeriod(int64_t rate_bytes_per_sec);

// This mutex guard all internal states
mutable port::Mutex request_mutex_;
Expand Down
5 changes: 5 additions & 0 deletions util/rate_limiter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ namespace rocksdb {

class RateLimiterTest : public testing::Test {};

TEST_F(RateLimiterTest, OverflowRate) {
GenericRateLimiter limiter(port::kMaxInt64, 1000, 10);
ASSERT_GT(limiter.GetSingleBurstBytes(), 1000000000ll);
}

TEST_F(RateLimiterTest, StartStop) {
std::unique_ptr<RateLimiter> limiter(new GenericRateLimiter(100, 100, 10));
}
Expand Down

0 comments on commit f62fbd2

Please sign in to comment.