-
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
client: implement maxAttempts for retryPolicy #7229
Conversation
Hi @aranjans! Is there something you are waiting for from me? |
@imoore76 no, i think this PR LGTM! Approved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for the PR. Just one thing that needs to change: the godoc comment, and one thing that would be nice-to-have (combining test cases) if it's possible, and one small optional thing that is a judgment call that I'll leave up to you.
Thank you for the PR! |
gRGC A6 - gRPC Retry Design (a.k.a. built in backoff retry) https://github.com/grpc/proposal/blob/master/A6-client-retries.md was implemented by grpc/grpc-go#2111 but unusable for a long time since maxAttempts was limited to hardcoded 5 (grpc/grpc-go#4615), recent PR fixed that grpc/grpc-go#7229. It's transparent to the user, to see it in action, make sure the gRPC server is unreachable (do not start tetragon for example), run tetra with: GRPC_GO_LOG_SEVERITY_LEVEL=warning <tetra cmd> Note that logs don't always have the time to be pushed before exit so output might be a bit off but the number of retries is respected (you can debug or synchronously print in the grpc/stream.c:shouldRetry or :withRetry to verify). Also note that the final backoff duration is completely random and chosen between 0 and the final duration that was computed via to the params: https://github.com/grpc/grpc-go/blob/v1.65.0/stream.go#L702 Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
gRGC A6 - gRPC Retry Design (a.k.a. built in backoff retry) https://github.com/grpc/proposal/blob/master/A6-client-retries.md was implemented by grpc/grpc-go#2111 but unusable for a long time since maxAttempts was limited to hardcoded 5 (grpc/grpc-go#4615), recent PR fixed that grpc/grpc-go#7229. It's transparent to the user, to see it in action, make sure the gRPC server is unreachable (do not start tetragon for example), run tetra with: GRPC_GO_LOG_SEVERITY_LEVEL=warning <tetra cmd> Note that logs don't always have the time to be pushed before exit so output might be a bit off but the number of retries is respected (you can debug or synchronously print in the grpc/stream.c:shouldRetry or :withRetry to verify). Also note that the final backoff duration is completely random and chosen between 0 and the final duration that was computed via to the params: https://github.com/grpc/grpc-go/blob/v1.65.0/stream.go#L702 Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
gRGC A6 - gRPC Retry Design (a.k.a. built in backoff retry) https://github.com/grpc/proposal/blob/master/A6-client-retries.md was implemented by grpc/grpc-go#2111 but unusable for a long time since maxAttempts was limited to hardcoded 5 (grpc/grpc-go#4615), recent PR fixed that grpc/grpc-go#7229. It's transparent to the user, to see it in action, make sure the gRPC server is unreachable (do not start tetragon for example), run tetra with: GRPC_GO_LOG_SEVERITY_LEVEL=warning <tetra cmd> Note that logs don't always have the time to be pushed before exit so output might be a bit off but the number of retries is respected (you can debug or synchronously print in the grpc/stream.c:shouldRetry or :withRetry to verify). Also note that the final backoff duration is completely random and chosen between 0 and the final duration that was computed via to the params: https://github.com/grpc/grpc-go/blob/v1.65.0/stream.go#L702 Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
gRGC A6 - gRPC Retry Design (a.k.a. built in backoff retry) https://github.com/grpc/proposal/blob/master/A6-client-retries.md was implemented by grpc/grpc-go#2111 but unusable for a long time since maxAttempts was limited to hardcoded 5 (grpc/grpc-go#4615), recent PR fixed that grpc/grpc-go#7229. It's transparent to the user, to see it in action, make sure the gRPC server is unreachable (do not start tetragon for example), run tetra with: GRPC_GO_LOG_SEVERITY_LEVEL=warning <tetra cmd> Note that logs don't always have the time to be pushed before exit so output might be a bit off but the number of retries is respected (you can debug or synchronously print in the grpc/stream.c:shouldRetry or :withRetry to verify). Also note that the final backoff duration is completely random and chosen between 0 and the final duration that was computed via to the params: https://github.com/grpc/grpc-go/blob/v1.65.0/stream.go#L702 Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
Fixes #4615
RELEASE NOTES:
WithMaxCallAttempts
to configure gRPC's retry behavior per-channel.