-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Retry backoff to follow changes made to gRFC A6 #7514
Comments
Because the Go and C core clients seem to have different implementations of the retryPolicy exponential backoff, I have asked for clarification on StackOverflow: https://stackoverflow.com/questions/78877444/grpc-client-core-c-retrypolicy-logic-vs-go-client |
I've also just discovered that my use of |
Gitter chat for the same issue: https://matrix.to/#/!LWDFtYAcIVyULYUnXs:gitter.im/$NTr_B4whf71VHcXiBnNB78hzqF46RrYSTTFIGXnaIXU?via=gitter.im Based on the reply from @ejona86
Checking with other language maintainers about the way forward. |
@justinfx, thanks for bringing up this issue. After discussions between the maintainers, it is decided that the gRFC should be updated to specify the behaviour similar to c-core. Java, Go and Node will update their behaviour to match the updated gRFC. For backward compatibility, language implementations can provide an environment variable or other knob to get back the old behaviour, for a release or two. I will keep this issue open to track the changes in Go. |
@arjan-bal this is a fantastic result! I'm so happy to see that the C-core approach was deemed more appropriate. This will really improve the usefulness of the retry policy in the other clients. Thanks! |
@arjan-bal : Is there an issue to update the gRFC? Can we link it here and mark this as blocked? Thanks. |
The gRFC was updated: grpc/proposal#452 |
@easwars since the gRFC is updated in grpc/proposal#452, I believe this issue isn't blocked now. |
@arjan-bal : Do we need code changes for this? |
Yes. We implemented the gRFC as documented before, and now the behavior is changed. |
What version of gRPC are you using?
google.golang.org/grpc v1.65.0
What version of Go are you using (
go version
)?1.23.0
What operating system (Linux, Windows, …) and version?
Linux
What did you do?
I have been trying to debug the effective behaviour of the client retryPolicy values in response to an "UNAVAILABLE" server. After using many different combinations of values for "InitialBackoff", "MaxBackoff", and "BackoffMultiplier", I have found the retry backoff times to be wildly unpredictable.
Example configs:
What did you expect to see?
As I increase the InitialBackoff and BackoffMultiplier I would expect to see a progressively longer backoff time between each retry. The goal is to choose an overall timeout, such as "20s", and try to get the retry process to attempt for something close to that window.
What did you see instead?
I observe wildly variable outcomes, where sometimes the total elapsed time for all retries could be 5s, or 10s, or 20s.
Here is an example of a pretty strange set of 5 attempts:
Policy:
Connection logs:
It retries with less than a second between attempts, two different times, and then a big 8 second one for the last one. But it is all entirely random.
Further investigations
MaxAttempts is capped to 5, so it is not possible to try and squeeze more retry time out of the entire process, when the retry backoff times are extremely variable in comparison to the configuration.
The Go implementation uses a randomization of the calculated backoff duration, which makes it land anywhere between 0-backoff:
https://github.com/grpc/grpc-go/blob/v1.65.0/stream.go#L702
The C++ implementation uses the actual calculated backoff duration, which makes it actually increase the backoff with each attempt. The randomization is only applied to the jitter:
https://github.com/grpc/grpc/blob/v1.65.4/src/core/lib/backoff/backoff.cc#L34
It seems entirely non-sensical to randomize the entire backoff value between 0-backoff. Am I reading the logic wrong? Does it not defeat the purpose of an exponential backoff if the next value could be smaller? Why is the Go client not doing a similar approach where it applies random jitter?
The end result is that the numbers I plug into the Go retryPolicy feel really arbitrary and magical. And I have to keep twiddling them and running a test to see what kind of range of retries I might get out of the results.
The text was updated successfully, but these errors were encountered: