From 68ad6d57663068fa92e634d3c1cd7d6b5ce24d65 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 10 Jun 2020 12:30:57 +0200 Subject: [PATCH] ScheduledStartingRateLimiter: do not except, but log instead Avoid excepting in scenarios with many workers: - Log an error instead of throwing - Tweaks kMinimalWorkerDelay so that the problem may in practice resolve: allow more headroom as configured concurrency increases. Intends to fix #339 Signed-off-by: Otto van der Schaaf --- source/client/process_impl.cc | 2 +- source/common/rate_limiter_impl.cc | 2 +- test/rate_limiter_test.cc | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index 135e40706..1baada4aa 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -137,7 +137,7 @@ void ProcessImpl::shutdown() { const std::vector& ProcessImpl::createWorkers(const uint32_t concurrency) { // TODO(oschaaf): Expose kMinimalDelay in configuration. - const std::chrono::milliseconds kMinimalWorkerDelay = 500ms; + const std::chrono::milliseconds kMinimalWorkerDelay = 500ms + (concurrency * 50ms); ASSERT(workers_.empty()); // We try to offset the start of each thread so that workers will execute tasks evenly spaced in diff --git a/source/common/rate_limiter_impl.cc b/source/common/rate_limiter_impl.cc index 3977d306e..ea0844a5d 100644 --- a/source/common/rate_limiter_impl.cc +++ b/source/common/rate_limiter_impl.cc @@ -57,7 +57,7 @@ ScheduledStartingRateLimiter::ScheduledStartingRateLimiter( : ForwardingRateLimiterImpl(std::move(rate_limiter)), scheduled_starting_time_(scheduled_starting_time) { if (timeSource().monotonicTime() >= scheduled_starting_time_) { - throw NighthawkException("Scheduled starting time needs to be in the future"); + ENVOY_LOG(error, "Scheduled starting time exceeded. This may cause unintended bursty traffic."); } } diff --git a/test/rate_limiter_test.cc b/test/rate_limiter_test.cc index 9da8424cc..8dee7ffb8 100644 --- a/test/rate_limiter_test.cc +++ b/test/rate_limiter_test.cc @@ -115,8 +115,9 @@ TEST_F(RateLimiterTest, ScheduledStartingRateLimiterTestBadArgs) { EXPECT_CALL(unsafe_mock_rate_limiter, timeSource) .Times(AtLeast(1)) .WillRepeatedly(ReturnRef(time_system)); - EXPECT_THROW(ScheduledStartingRateLimiter(std::move(mock_rate_limiter), timing); - , NighthawkException); + EXPECT_NO_THROW(ScheduledStartingRateLimiter(std::move(mock_rate_limiter), timing)); + // TODO(XXX): once we can, verify a warning gets logged while running the line + // above. } }