API: support max_retries in circuit breakers API#2773
Merged
zhaohuabing merged 5 commits intoenvoyproxy:mainfrom Mar 5, 2024
Merged
API: support max_retries in circuit breakers API#2773zhaohuabing merged 5 commits intoenvoyproxy:mainfrom
zhaohuabing merged 5 commits intoenvoyproxy:mainfrom
Conversation
…ults Signed-off-by: Guy Daich <guy.daich@sap.com>
Signed-off-by: Guy Daich <guy.daich@sap.com>
Signed-off-by: Guy Daich <guy.daich@sap.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2773 +/- ##
==========================================
- Coverage 63.44% 63.43% -0.02%
==========================================
Files 125 125
Lines 20532 20547 +15
==========================================
+ Hits 13027 13033 +6
- Misses 6668 6676 +8
- Partials 837 838 +1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Guy Daich <guy.daich@sap.com>
arkodg
reviewed
Mar 4, 2024
| if circuitBreaker.MaxParallelRetries != nil { | ||
| cbt.MaxRetries = &wrapperspb.UInt32Value{ | ||
| Value: *circuitBreaker.MaxParallelRetries, | ||
| } |
Contributor
There was a problem hiding this comment.
lets be explicit with 1024 in case circuitBreaker.MaxParallelRetries == nil, in case we decide to run in non k8s env that doesnt set that default
Contributor
There was a problem hiding this comment.
maybe we even need to set it if circuitBreaker == nil
Contributor
Author
There was a problem hiding this comment.
Sure, will do. For the other settings, I think that there's no reason to do it, as they anyway default to 1024 on envoy level.
Signed-off-by: Guy Daich <guy.daich@sap.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Allows configuration of
max_retriesin BTP's circuit breakers. After some internal discussion, it seems that circuit breakers are a better location for this feature:We should enhance documentation and let users know that these retry-related settings exist in the circuit breakers section.
Removes retry_budget for now, following the discussion here: #2769 (comment)
Which issue(s) this PR fixes:
Fixes #2322