Skip to content

Commit

Permalink
[backoff] cap initial backoff at max backoff (grpc#38239)
Browse files Browse the repository at this point in the history
Closes grpc#38239

COPYBARA_INTEGRATE_REVIEW=grpc#38239 from markdroth:backoff_clamp_initial_to_max bd8b099
PiperOrigin-RevId: 703370103
  • Loading branch information
markdroth authored and copybara-github committed Dec 6, 2024
1 parent 1ef140c commit b3a44f1
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 5 deletions.
1 change: 1 addition & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3386,6 +3386,7 @@ grpc_cc_library(
visibility = ["@grpc:alt_grpc_base_legacy"],
deps = [
"gpr_platform",
"//src/core:experiments",
"//src/core:time",
],
)
Expand Down
1 change: 1 addition & 0 deletions bazel/experiments.bzl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions src/core/lib/experiments/experiments.cc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions src/core/lib/experiments/experiments.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/core/lib/experiments/experiments.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@

# This file only defines the experiments. Refer to rollouts.yaml for the rollout
# state of each experiment.
- name: backoff_cap_initial_at_max
description: Backoff library applies max_backoff even on initial_backoff.
expiry: 2025/03/01
owner: [email protected]
test_tags: []
- name: call_tracer_in_transport
description: Transport directly passes byte counts to CallTracer.
expiry: 2025/02/01
Expand Down
2 changes: 2 additions & 0 deletions src/core/lib/experiments/rollouts.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
#
# Supported platforms: ios, windows, posix

- name: backoff_cap_initial_at_max
default: true
- name: call_tracer_in_transport
default: true
- name: call_v3
Expand Down
19 changes: 15 additions & 4 deletions src/core/util/backoff.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,27 @@

#include <algorithm>

#include "src/core/lib/experiments/experiments.h"

namespace grpc_core {

BackOff::BackOff(const Options& options) : options_(options) { Reset(); }

Duration BackOff::NextAttemptDelay() {
if (initial_) {
initial_ = false;
if (IsBackoffCapInitialAtMaxEnabled()) {
if (initial_) {
initial_ = false;
} else {
current_backoff_ *= options_.multiplier();
}
current_backoff_ = std::min(current_backoff_, options_.max_backoff());
} else {
current_backoff_ = std::min(current_backoff_ * options_.multiplier(),
options_.max_backoff());
if (initial_) {
initial_ = false;
} else {
current_backoff_ = std::min(current_backoff_ * options_.multiplier(),
options_.max_backoff());
}
}
const double jitter =
absl::Uniform(rand_gen_, 1 - options_.jitter(), 1 + options_.jitter());
Expand Down
2 changes: 1 addition & 1 deletion test/core/end2end/tests/retry_cancel_during_delay.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void TestRetryCancelDuringDelay(
" \"retryPolicy\": {\n"
" \"maxAttempts\": 3,\n"
" \"initialBackoff\": \"%ds\",\n"
" \"maxBackoff\": \"120s\",\n"
" \"maxBackoff\": \"1000s\",\n"
" \"backoffMultiplier\": 1.6,\n"
" \"retryableStatusCodes\": [ \"ABORTED\" ]\n"
" }\n"
Expand Down
22 changes: 22 additions & 0 deletions test/core/util/backoff_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "src/core/lib/experiments/experiments.h"
#include "src/core/util/time.h"
#include "test/core/test_util/test_config.h"

Expand All @@ -50,6 +51,27 @@ TEST(BackOffTest, ConstantBackOff) {
EXPECT_EQ(backoff.NextAttemptDelay(), kInitialBackoff);
}

TEST(BackOffTest, InitialBackoffCappedByMaxBackoff) {
if (!IsBackoffCapInitialAtMaxEnabled()) {
GTEST_SKIP() << "test requires backoff_cap_initial_at_max experiment";
}
const auto kInitialBackoff = Duration::Seconds(2);
const auto kMaxBackoff = Duration::Seconds(1);
const double kMultiplier = 1.0;
const double kJitter = 0.0;
BackOff::Options options;
options.set_initial_backoff(kInitialBackoff)
.set_multiplier(kMultiplier)
.set_jitter(kJitter)
.set_max_backoff(kMaxBackoff);
BackOff backoff(options);
EXPECT_EQ(backoff.NextAttemptDelay(), kMaxBackoff);
EXPECT_EQ(backoff.NextAttemptDelay(), kMaxBackoff);
EXPECT_EQ(backoff.NextAttemptDelay(), kMaxBackoff);
EXPECT_EQ(backoff.NextAttemptDelay(), kMaxBackoff);
EXPECT_EQ(backoff.NextAttemptDelay(), kMaxBackoff);
}

TEST(BackOffTest, NoJitterBackOff) {
const auto kInitialBackoff = Duration::Milliseconds(2);
const double kMultiplier = 2.0;
Expand Down

0 comments on commit b3a44f1

Please sign in to comment.