Skip to content

Commit

Permalink
[call-v3] Put retries under an experiment (grpc#38255)
Browse files Browse the repository at this point in the history
I've observed a few test flakes with the new retry code... put it under an experiment for now so that those flakes don't block submission of unrelated code.

Closes grpc#38255

COPYBARA_INTEGRATE_REVIEW=grpc#38255 from ctiller:retry-experiment 1ca6a18
PiperOrigin-RevId: 704596499
  • Loading branch information
ctiller authored and copybara-github committed Dec 10, 2024
1 parent cd57733 commit 25813a0
Show file tree
Hide file tree
Showing 19 changed files with 45 additions and 0 deletions.
4 changes: 4 additions & 0 deletions bazel/experiments.bzl

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

12 changes: 12 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.

8 changes: 8 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 @@ -177,6 +177,11 @@
owner: [email protected]
test_tags: []
allow_in_fuzzing_config: false # experiment currently crashes if enabled
- name: retry_in_callv3
description: Support retries with call-v3
expiry: 2025/06/06
owner: [email protected]
test_tags: [core_end2end_test]
- name: rq_fast_reject
description:
Resource quota rejects requests immediately (before allocating the request
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/end2end_test_suites.cc
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ class ChaoticGoodFixture : public CoreTestFixture {
args.Set(GRPC_ARG_CHAOTIC_GOOD_DATA_CONNECTIONS, data_connections_)
.Set(GRPC_ARG_CHAOTIC_GOOD_MAX_RECV_CHUNK_SIZE, chunk_size_)
.Set(GRPC_ARG_CHAOTIC_GOOD_MAX_SEND_CHUNK_SIZE, chunk_size_)
.Set(GRPC_ARG_ENABLE_RETRIES, IsRetryInCallv3Enabled())
.ToC()
.get(),
nullptr);
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace grpc_core {
// - first attempt returns ABORTED
// - second attempt returns OK
CORE_END2END_TEST(RetryTest, Retry) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ namespace {
// Tests that we can unref a call after the first attempt starts but
// before any ops complete. This should not cause a memory leak.
CORE_END2END_TEST(RetryTest, RetryCancelAfterFirstAttemptStarts) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
// This is a workaround for the flakiness that if the server ever enters
// GracefulShutdown for whatever reason while the client has already been
// shutdown, the test would not timeout and fail.
Expand Down
2 changes: 2 additions & 0 deletions test/core/end2end/tests/retry_cancellation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,12 @@ void TestRetryCancellation(CoreEnd2endTest& test,
}

CORE_END2END_TEST(RetryTest, RetryCancellation) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
TestRetryCancellation(*this, std::make_unique<CancelCancellationMode>());
}

CORE_END2END_TEST(RetryTest, RetryDeadline) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
TestRetryCancellation(*this, std::make_unique<DeadlineCancellationMode>());
}

Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry_non_retriable_status.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace {
// - 1 retry allowed for ABORTED status
// - first attempt gets INVALID_ARGUMENT, so no retry is done
CORE_END2END_TEST(RetryTest, RetryNonRetriableStatus) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ namespace {
// - first attempt gets INVALID_ARGUMENT, so no retry is done
CORE_END2END_TEST(RetryTest,
RetryNonRetriableStatusBeforeRecvTrailingMetadataStarted) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry_recv_initial_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace {
// - first attempt receives initial metadata before trailing metadata,
// so no retry is done even though status was ABORTED
CORE_END2END_TEST(RetryTest, RetryRecvInitialMetadata) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry_recv_message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace {
// - first attempt receives a message and therefore does not retry even
// though the final status is ABORTED
CORE_END2END_TEST(RetryTest, RetryRecvMessage) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ namespace {
// - first attempt returns ABORTED
// - second attempt returns OK
CORE_END2END_TEST(RetryTest, RetrySendInitialMetadataRefs) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry_server_pushback_delay.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace {
// - first attempt gets ABORTED with a long delay
// - second attempt succeeds
CORE_END2END_TEST(RetryTest, RetryServerPushbackDelay) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry_server_pushback_disabled.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace {
// - first attempt gets ABORTED
// - second attempt gets ABORTED but server push back disables retrying
CORE_END2END_TEST(RetryTest, RetryServerPushbackDisabled) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry_throttled.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace {
// - 1 retry allowed for ABORTED status
// - first attempt gets ABORTED but is over limit, so no retry is done
CORE_END2END_TEST(RetryTest, RetryThrottled) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
"{\n"
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry_too_many_attempts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace {
// - first attempt gets ABORTED
// - second attempt gets ABORTED but does not retry
CORE_END2END_TEST(RetryTest, RetryTooManyAttempts) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry_unref_before_finish.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace {
// Tests that we can unref a call whose status is cached but not yet
// requested by the application. This should not cause a memory leak.
CORE_END2END_TEST(RetryTest, RetryUnrefBeforeFinish) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down
1 change: 1 addition & 0 deletions test/core/end2end/tests/retry_unref_before_recv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ namespace {
// they complete. This ensures that we don't drop callbacks or cause a
// memory leak.
CORE_END2END_TEST(RetryTest, UnrefBeforeRecv) {
if (!IsRetryInCallv3Enabled()) SKIP_IF_V3();
InitServer(ChannelArgs());
InitClient(ChannelArgs().Set(
GRPC_ARG_SERVICE_CONFIG,
Expand Down

0 comments on commit 25813a0

Please sign in to comment.