Skip to content
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
90bd582
save state on ramping rate limiter
oschaaf Nov 16, 2019
bf5ae1d
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Nov 25, 2019
28011ef
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Nov 27, 2019
be93d1a
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Nov 27, 2019
76158b9
Add tests, clean up
oschaaf Nov 29, 2019
d29c95c
Refactor + LinearlyOpeningRateLimiterFilter
oschaaf Nov 29, 2019
ebd857c
save state
oschaaf Nov 30, 2019
1d62a00
save state
oschaaf Dec 2, 2019
c86cb92
Back out change to constness in Frequency
oschaaf Dec 2, 2019
7fc2992
Save state, wire in zipf / foo ZipfRateLimiter
oschaaf Dec 2, 2019
9def3cd
Some comments & tidying up
oschaaf Dec 2, 2019
abda9db
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Dec 3, 2019
4c54837
small cleanup
oschaaf Dec 3, 2019
7883d59
Update Envoy dep for access to enableHRTimer()
oschaaf Dec 3, 2019
e458ea5
Whoops, amend bad copying
oschaaf Dec 3, 2019
e7b65c1
Also, sync .bazelrc while at it
oschaaf Dec 3, 2019
a8c94d8
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Dec 6, 2019
cc4fead
Review feedback
oschaaf Dec 11, 2019
ea43285
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Dec 11, 2019
4e7eb0a
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Dec 12, 2019
a69d811
Clean up the diff
oschaaf Dec 12, 2019
b5b5fd2
Merge remote-tracking branch 'upstream/master' into update-envoy-dep-8
oschaaf Dec 12, 2019
fb85732
Update Envoy to the latest
oschaaf Dec 12, 2019
8a268fe
Move to the sha that has our target
oschaaf Dec 12, 2019
34f649b
Unbreak it
oschaaf Dec 13, 2019
54aa937
Merge remote-tracking branch 'origin/update-envoy-dep-8' into ramping…
oschaaf Dec 13, 2019
f34708f
Fix a TODO
oschaaf Dec 13, 2019
58a7566
Fix accidental comment
oschaaf Dec 13, 2019
1f90162
Fix clang-tidy issue
oschaaf Dec 13, 2019
e1fa16d
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Dec 13, 2019
af45a0a
Zipf: expose q and v arguments of the distribution
oschaaf Dec 13, 2019
5c8637c
Zipf: expose q and v arguments of the distribution
oschaaf Dec 13, 2019
89557e8
Merge branch 'ramping-linear-rate-limiter' of github.com:oschaaf/nigh…
oschaaf Dec 13, 2019
79b641c
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Dec 16, 2019
bec3b95
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Dec 19, 2019
fe7bb0c
Merge remote-tracking branch 'upstream/master' into ramping-linear-ra…
oschaaf Dec 23, 2019
324a4ee
Remove zipf stuff
oschaaf Dec 23, 2019
bbea17c
Revert "Remove zipf stuff"
oschaaf Dec 23, 2019
d24ee03
Merge remote-tracking branch 'upstream/master' into zipf-distributing…
oschaaf Jan 3, 2020
7f17056
Merge remote-tracking branch 'upstream/master' into zipf-distributing…
oschaaf Jan 3, 2020
d00feae
Merge remote-tracking branch 'upstream/master' into zipf-distributing…
oschaaf Jan 4, 2020
1815831
Merge remote-tracking branch 'upstream/master' into zipf-distributing…
oschaaf Jan 6, 2020
10dbc29
Review feedback
oschaaf Jan 7, 2020
5defc57
Merge remote-tracking branch 'upstream/master' into zipf-distributing…
oschaaf Jan 10, 2020
747c49f
Merge remote-tracking branch 'upstream/master' into zipf-distributing…
oschaaf Jan 10, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion source/common/rate_limiter_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ bool LinearRampingRateLimiterImpl::tryAcquireOne() {
acquired_count_++;
return acquireable_count_--;
}

const std::chrono::nanoseconds elapsed_time = elapsed();
double elapsed_fraction = 1.0;
if (elapsed_time < ramp_time_) {
Expand Down Expand Up @@ -191,4 +190,10 @@ GraduallyOpeningRateLimiterFilter::GraduallyOpeningRateLimiterFilter(
}
}

ZipfRateLimiterImpl::ZipfRateLimiterImpl(RateLimiterPtr&& rate_limiter, bool deterministic,
double q, double v)
: FilteringRateLimiterImpl(std::move(rate_limiter),
[this]() { return deterministic_ ? dist_(mt_) : dist_(g_); }),
dist_(1, q, v), deterministic_(deterministic) {}

} // namespace Nighthawk
27 changes: 27 additions & 0 deletions source/common/rate_limiter_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "common/frequency.h"

#include "absl/random/random.h"
#include "absl/random/zipf_distribution.h"
#include "absl/types/optional.h"

namespace Nighthawk {
Expand Down Expand Up @@ -203,4 +204,30 @@ class GraduallyOpeningRateLimiterFilter : public FilteringRateLimiterImpl {
const std::chrono::nanoseconds ramp_time_;
};

/**
* Thin wrapper around absl::zipf_distribution that will pull zeroes and ones from the distribution
* with the intent to probabilistically suppress the wrapped rate limiter.
* This may need further consideration, because it will shoot holes in the pacing, lowering the
* actual achieved frequency.
*/
class ZipfRateLimiterImpl : public FilteringRateLimiterImpl {
public:
/**
* From the absl header associated to the zipf distribution:
* Preconditions: v > 0, q > 1
* The precondidtions are validated when NDEBUG is not defined via
* a pair of assert() directives.
* If NDEBUG is defined and either or both of these parameters take invalid
* values, the behavior of the class is undefined.
*/
ZipfRateLimiterImpl(RateLimiterPtr&& rate_limiter, bool deterministic, double q = 2.0,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we had some open comments for the API and its documentation here in the original PR where this was introduced before we split it out. Would you mind resolving those comments before we proceed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, addressed the prior feedback round in 10dbc29

double v = 1.0);

private:
absl::zipf_distribution<uint64_t> dist_;
absl::InsecureBitGen g_;
std::mt19937_64 mt_;
bool deterministic_;
};

} // namespace Nighthawk
24 changes: 24 additions & 0 deletions test/rate_limiter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -340,4 +340,28 @@ TEST_F(GraduallyOpeningRateLimiterFilterTest, TimingVerificationTest) {
760, 780, 840, 860, 880, 900, 920, 940, 960, 980, 1000}));
}

class ZipfRateLimiterImplTest : public Test {};

TEST_F(ZipfRateLimiterImplTest, TimingVerificationTest) {
Envoy::Event::SimulatedTimeSystem time_system;
auto rate_limiter = std::make_unique<ZipfRateLimiterImpl>(
std::make_unique<LinearRateLimiter>(time_system, 10_Hz), true);
const std::chrono::seconds duration = 15s;
std::vector<int64_t> aquisition_timings;
auto total_ms_elapsed = 0ms;
auto clock_tick = 1ms;

do {
if (rate_limiter->tryAcquireOne()) {
aquisition_timings.push_back(total_ms_elapsed.count());
}
time_system.sleep(clock_tick);
total_ms_elapsed += clock_tick;
} while (total_ms_elapsed <= duration);
EXPECT_EQ(aquisition_timings,
std::vector<int64_t>({500, 800, 1300, 2400, 2900, 3900, 4200, 4400, 4500,
5800, 6000, 6400, 7900, 8400, 8600, 9900, 10200, 10500,
10600, 12000, 12300, 12600, 13300, 13600, 13700, 13800, 13900}));
}

} // namespace Nighthawk