config: Make gRPC xDS retry backoff configurable #24701

jaysonjdmello · 2022-12-29T00:17:39Z

Signed-off-by: Jayson Dmello jdmello@confluent.io

Commit Message:
The gRPC xDS implementation uses hardcoded retry durations for re-establishing the gRPC channel. This change makes the backoff parameters configurable.

Additional Description:
Risk Level: Low
Testing: unit test
Docs Changes:
Release Notes: Added
Platform Specific Features:
[Optional Runtime guard:]
Fixes #24236
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

repokitteh-read-only · 2022-12-29T00:17:43Z

Hi @jaysonjdmello, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #24701 was opened by jaysonjdmello.

see: more, trace.

repokitteh-read-only · 2022-12-29T00:17:50Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @lizan
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #24701 was opened by jaysonjdmello.

see: more, trace.

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

KBaichoo · 2023-01-09T16:23:37Z

/assign @yanjunxiang-google

Can you take a look?

yanjunxiang-google · 2023-01-09T17:02:33Z

source/common/config/utility.h

@@ -41,6 +41,9 @@ namespace Config {

 constexpr absl::string_view Wildcard = "*";

+static constexpr uint64_t RetryBaseIntervalMs = 500;


Are these global constants GRPC retry mechanism specific? Need some comments here. Also is here the best place to define them?

These are not intended to be specific to GRPC retry timers but rather the the default time for prepareBackoffStrategy method in this file.

Can you suggest an alternate location for these global constants please.

test/common/config/utility_test.cc

yanjunxiang-google · 2023-01-09T17:19:56Z

LGTM in general. Just a few nit comments.

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

api/envoy/config/core/v3/grpc_service.proto

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

KBaichoo · 2023-01-10T16:41:38Z

/assign @adisuissa

For xDS code review maintainer pass

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

/lgtm api

lizan · 2023-01-11T02:19:37Z

/lgtm api

adisuissa

Thanks for your contribution.
Before going into the details, can you explain the benefit of using this over the gRPC retries mechanism?

/wait-any

repokitteh-read-only · 2023-04-06T00:03:23Z

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24701 (comment) was created by @jaysonjdmello.

see: more, trace.

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

jaysonjdmello · 2023-04-08T02:56:54Z

/retest

repokitteh-read-only · 2023-04-08T02:57:01Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24701 (comment) was created by @jaysonjdmello.

see: more, trace.

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

jaysonjdmello · 2023-04-11T20:34:00Z

/retest

repokitteh-read-only · 2023-04-11T20:34:06Z

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24701 (comment) was created by @jaysonjdmello.

see: more, trace.

jaysonjdmello · 2023-04-11T21:01:32Z

/retest

repokitteh-read-only · 2023-04-11T21:01:38Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24701 (comment) was created by @jaysonjdmello.

see: more, trace.

jaysonjdmello · 2023-04-11T22:15:27Z

@htuch, would highly appreciate if you could provide feedback on the new added unit tests in grpc_stream_test , thanks!

source/common/config/utility.cc

htuch

LGTM modulo tiniest nits, thanks for the tests!

test/common/config/utility_test.cc

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

jaysonjdmello · 2023-04-14T15:51:09Z

LGTM modulo tiniest nits, thanks for the tests!

Thank you for reviewing! I have addressed the issues

jaysonjdmello · 2023-04-14T19:58:26Z

/retest

repokitteh-read-only · 2023-04-14T19:58:31Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24701 (comment) was created by @jaysonjdmello.

see: more, trace.

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

htuch

LGTM, thanks!

Relates: envoyproxy/envoy#24701 Signed-off-by: Tam Mach <tam.mach@cilium.com>

Relates: envoyproxy/envoy#24701 Signed-off-by: Tam Mach <tam.mach@cilium.io>

Make xDS retry backoff configurable

cdcf3cf

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

repokitteh-read-only bot added the api label Dec 29, 2022

repokitteh-read-only bot assigned lizan Dec 29, 2022

jaysonjdmello changed the title ~~Make gRPC xDS retry backoff configurable~~ config: Make gRPC xDS retry backoff configurable Dec 29, 2022

jaysonjdmello added 3 commits December 28, 2022 18:04

Fix Changelog ref

8e80c9d

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

Fix EDS Unit Test

2b9e70e

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

Add size checkg for GRPC config

5706fb9

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

repokitteh-read-only bot assigned yanjunxiang-google Jan 9, 2023

yanjunxiang-google reviewed Jan 9, 2023

View reviewed changes

test/common/config/utility_test.cc Outdated Show resolved Hide resolved

Address Code Review Comments

2495a26

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

jaysonjdmello force-pushed the grpc_configurable branch from 77d31a0 to 2495a26 Compare January 9, 2023 21:31

jaysonjdmello and others added 2 commits January 9, 2023 14:36

Fix comment block

4858b7e

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

Merge branch 'main' into grpc_configurable

4bd2992

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

lizan previously requested changes Jan 10, 2023

View reviewed changes

api/envoy/config/core/v3/grpc_service.proto Outdated Show resolved Hide resolved

Revert proto numbering

f2e01ce

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

repokitteh-read-only bot assigned adisuissa Jan 10, 2023

jaysonjdmello requested a review from lizan January 10, 2023 16:45

jaysonjdmello and others added 2 commits January 10, 2023 11:06

Fix formatting

185c54f

Signed-off-by: Jayson Dmello <jdmello@confluent.io>

Merge branch 'main' into grpc_configurable

bb7d941

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

repokitteh-read-only bot removed the api label Jan 11, 2023

adisuissa reviewed Jan 11, 2023

View reviewed changes

repokitteh-read-only bot added the waiting:any label Jan 11, 2023

jaysonjdmello added 3 commits April 6, 2023 11:30

Sync with upstream

c43bea7

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

Fix clang-tidy format issues

80aa513

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

fix the rename error

560d128

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

fixing unit test

52e0587

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

htuch reviewed Apr 14, 2023

View reviewed changes

source/common/config/utility.cc Outdated Show resolved Hide resolved

htuch reviewed Apr 14, 2023

View reviewed changes

test/common/config/utility_test.cc Outdated Show resolved Hide resolved

test/common/config/utility_test.cc Outdated Show resolved Hide resolved

Fix review comments

74fb9d1

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

Sync with upstream

7242843

Signed-off-by: Jayson Dmello <110573042+jaysonjdmello@users.noreply.github.com>

htuch approved these changes Apr 16, 2023

View reviewed changes

htuch enabled auto-merge (squash) April 16, 2023 22:38

htuch merged commit 672b415 into envoyproxy:main Apr 16, 2023

jaysonjdmello deleted the grpc_configurable branch April 17, 2023 16:34

yanjunxiang-google mentioned this pull request Apr 19, 2023

envoy:filter_fuzz_test: ASSERT: base_interval_ > 0 #26840

Closed

sayboras pushed a commit to sayboras/proxy that referenced this pull request Jul 14, 2023

cilium: Add jitter argument for GrpcImpl

943691c

Relates: envoyproxy/envoy#24701 Signed-off-by: Tam Mach <tam.mach@cilium.com>

sayboras pushed a commit to sayboras/proxy that referenced this pull request Jul 14, 2023

cilium: Add jitter argument for GrpcImpl

7d56862

Relates: envoyproxy/envoy#24701 Signed-off-by: Tam Mach <tam.mach@cilium.com>

sayboras pushed a commit to sayboras/proxy that referenced this pull request Jul 14, 2023

cilium: Add jitter argument for GrpcImpl

c5fd1ca

Relates: envoyproxy/envoy#24701 Signed-off-by: Tam Mach <tam.mach@cilium.io>

sayboras pushed a commit to cilium/proxy that referenced this pull request Jul 17, 2023

cilium: Add jitter argument for GrpcImpl

d3cca76

Relates: envoyproxy/envoy#24701 Signed-off-by: Tam Mach <tam.mach@cilium.io>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: Make gRPC xDS retry backoff configurable #24701

config: Make gRPC xDS retry backoff configurable #24701

jaysonjdmello commented Dec 29, 2022

repokitteh-read-only bot commented Dec 29, 2022

repokitteh-read-only bot commented Dec 29, 2022

KBaichoo commented Jan 9, 2023

yanjunxiang-google Jan 9, 2023

jaysonjdmello Jan 9, 2023 •

edited

Loading

yanjunxiang-google commented Jan 9, 2023

KBaichoo commented Jan 10, 2023

lizan commented Jan 11, 2023

adisuissa left a comment

repokitteh-read-only bot commented Apr 6, 2023

jaysonjdmello commented Apr 8, 2023

repokitteh-read-only bot commented Apr 8, 2023

jaysonjdmello commented Apr 11, 2023

repokitteh-read-only bot commented Apr 11, 2023

jaysonjdmello commented Apr 11, 2023

repokitteh-read-only bot commented Apr 11, 2023

jaysonjdmello commented Apr 11, 2023

htuch left a comment

jaysonjdmello commented Apr 14, 2023

jaysonjdmello commented Apr 14, 2023

repokitteh-read-only bot commented Apr 14, 2023

htuch left a comment

		@@ -41,6 +41,9 @@ namespace Config {

		constexpr absl::string_view Wildcard = "*";

		static constexpr uint64_t RetryBaseIntervalMs = 500;

config: Make gRPC xDS retry backoff configurable #24701

config: Make gRPC xDS retry backoff configurable #24701

Conversation

jaysonjdmello commented Dec 29, 2022

repokitteh-read-only bot commented Dec 29, 2022

repokitteh-read-only bot commented Dec 29, 2022

KBaichoo commented Jan 9, 2023

yanjunxiang-google Jan 9, 2023

Choose a reason for hiding this comment

jaysonjdmello Jan 9, 2023 • edited Loading

Choose a reason for hiding this comment

yanjunxiang-google commented Jan 9, 2023

KBaichoo commented Jan 10, 2023

lizan commented Jan 11, 2023

adisuissa left a comment

Choose a reason for hiding this comment

repokitteh-read-only bot commented Apr 6, 2023

jaysonjdmello commented Apr 8, 2023

repokitteh-read-only bot commented Apr 8, 2023

jaysonjdmello commented Apr 11, 2023

repokitteh-read-only bot commented Apr 11, 2023

jaysonjdmello commented Apr 11, 2023

repokitteh-read-only bot commented Apr 11, 2023

jaysonjdmello commented Apr 11, 2023

htuch left a comment

Choose a reason for hiding this comment

jaysonjdmello commented Apr 14, 2023

jaysonjdmello commented Apr 14, 2023

repokitteh-read-only bot commented Apr 14, 2023

htuch left a comment

Choose a reason for hiding this comment

jaysonjdmello Jan 9, 2023 •

edited

Loading