Skip to content

#2080 Add new FailureRatio and SamplingDuration V8 parameters to fine-tune Polly's circuit-breaker strategy via route-level and global QoS options #2081

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

Conversation

RaynaldM
Copy link
Collaborator

@RaynaldM RaynaldM commented May 29, 2024

Closes #2080

This PR adds 2 new parameters in QoSOptions to fine-tune circuit-breaker behavior 👉

  • FailureRatio: The failure-success ratio that will cause the circuit to break/open.
  • SamplingDuration: The time period over which the failure-success ratio is calculated (in seconds).

Predecessor

@RaynaldM RaynaldM self-assigned this May 29, 2024
@RaynaldM RaynaldM added feature A new feature small effort Likely less than a day of development effort. QoS Ocelot feature: Quality of Service aka Polly Configuration Ocelot feature: Configuration labels May 29, 2024
@raman-m raman-m changed the title #2080 Adds parameters (in this case those of Polly V8) to fine-tune c… #2080 New V8 parameters to fine-tune Polly's circuit-breaker behavior May 29, 2024
@raman-m raman-m changed the title #2080 New V8 parameters to fine-tune Polly's circuit-breaker behavior #2080 Add new V8 params to fine-tune Polly's circuit-breaker behavior May 29, 2024
@raman-m
Copy link
Member

raman-m commented May 29, 2024

Great! Which version are you planning to release it in?

Please note that the PR depends on #2073, which needs to be merged before this PR. Therefore, this feature branch should be rebased after the merging of #2073, and ideally, after #2079 is merged as well.

It appears that it won't be included in version 23.3, but rather in version 24.0, which is the next release following v23.3.
Is this delivery acceptable to you?

@RaynaldM
Copy link
Collaborator Author

Great! Which version are you planning to release it in?

Please note that the PR depends on #2073, which needs to be merged before this PR. Therefore, this feature branch should be rebased after the merging of #2073, and ideally, after #2079 is merged as well.

It appears that it won't be included in version 23.3, but rather in version 24.0, which is the next release following v23.3. Is this delivery acceptable to you?

Yes, @raman-m, you can add this when you want

@raman-m raman-m added the 2023 Annual 2023 release label Jun 11, 2024
@raman-m raman-m added this to the Annual 2023 milestone Jun 11, 2024
@raman-m
Copy link
Member

raman-m commented Jun 11, 2024

@RaynaldM
Please resolve conflicts!
PR has been added to v24.0 milestone, current release

@raman-m
Copy link
Member

raman-m commented Jun 13, 2024

@RaynaldM Conflicts!

@raman-m raman-m force-pushed the 2080-adds-parameters-in-this-case-those-of-polly-v8-to-fine-tune-circuit-breaker-behavior branch from 1de22fd to f69831a Compare June 14, 2024 17:52
@raman-m raman-m removed the small effort Likely less than a day of development effort. label Jun 14, 2024
@raman-m raman-m requested review from raman-m and ggnaegi and removed request for raman-m June 19, 2024 16:04
@raman-m raman-m added Oct'24 October 2024 release and removed 2023 Annual 2023 release labels Aug 12, 2024
@raman-m raman-m modified the milestones: Annual 2023, September'24 Aug 12, 2024
@raman-m
Copy link
Member

raman-m commented Aug 12, 2024

Let's assign this to the Sep'24 milestone since the Annual 2023 milestone is already quite overloaded, and we need to reduce the scope of work for 2023.

@raman-m raman-m added Spring'25 Spring 2025 release and removed Oct'24 October 2024 release labels Oct 26, 2024
@raman-m raman-m modified the milestones: October'24, Autumn'24 Oct 26, 2024
@raman-m
Copy link
Member

raman-m commented Jul 11, 2025

Tasks

  • Create acceptance tests to ensure global QoS configuration is accessible in the PollyQoSResiliencePipelineProvider class → Done in commit 51be1a3 ✔️
  • Refactor the provider class and clean up all code related to global QoS injection → Done in commit 3c386c1 ✔️
  • Verify unit test coverage thoroughly → Done in commits 5b92dce and 0c29c00 ✔️
  • Update documentation → Done in commit dfe4ba6 ✔️

…of merging both route & global options by QoSOptionsCreator into DownstreamRoute
…f-polly-v8-to-fine-tune-circuit-breaker-behavior
@raman-m
Copy link
Member

raman-m commented Jul 18, 2025

@ggnaegi
Gui, this commit 7d73299 was specially made for you. Here is my explanation to you...

TODO Override the Equals method of the DownstreamRoute class

image

The image above illustrates runtime debugging of the comparer being used to compare two DownstreamRoute class instances, which are reference types. In the test from the commit, I demonstrated that the standard object.Equals(object) method is called for comparison. However, this default implementation relies on comparing memory references. Therefore, we need to implement the entire stack as per .NET documentation to properly compare DownstreamRoute class instances. Unfortunately, the DownstreamRoute class currently lacks such logic.
The ultimate goal is to eliminate the MessageInvokerCacheKey structure since the DownstreamRoute class should have its own overridden version of the Equals method. Once DownstreamRoute includes all overridden versions for Equals, GetHashCode, and (in)equality operators, we can replace the MessageInvokerCacheKey class with DownstreamRoute as the key type for the relevant concurrent dictionary.
We need to run tests to ensure that the caching in the pool is functional and uses minimal RAM, allowing it to be effectively reused.
This task is not straightforward and might require significant effort due to potential cascading changes. However, we could avoid these issues by adding a simple route ID or number property as an integer, which is the most efficient type for hashing and caching. To my knowledge, the well-known Bla-bla gateway already implements an ID property for route objects. This should be considered for future development.

@raman-m raman-m changed the title #2080 Add new V8 params to fine-tune Polly's circuit-breaker behavior #2080 Add new V8 parameters to fine-tune Polly's circuit-breaker strategy via route-level and global QoS options Jul 19, 2025
@raman-m raman-m changed the title #2080 Add new V8 parameters to fine-tune Polly's circuit-breaker strategy via route-level and global QoS options #2080 Add new FailureRatio and SamplingDuration V8 parameters to fine-tune Polly's circuit-breaker strategy via route-level and global QoS options Jul 19, 2025
Copy link
Member

@raman-m raman-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready for delivery ✅

  • Code review ✔️
  • Unit testing ✔️✔️
  • Acceptance testing ✔️
  • Updated docs ✔️

@raman-m raman-m merged commit c721273 into develop Jul 19, 2025
6 of 9 checks passed
@raman-m raman-m deleted the 2080-adds-parameters-in-this-case-those-of-polly-v8-to-fine-tune-circuit-breaker-behavior branch July 19, 2025 16:22
@raman-m raman-m removed the in progress Someone is working on the issue. Could be someone on the team or off. label Jul 19, 2025
@raman-m
Copy link
Member

raman-m commented Jul 19, 2025

@RaynaldM Hi Ray!
Thanks for your contribution!
There are more resilience strategies waiting for you to add to Ocelot's Polly provider. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Configuration Ocelot feature: Configuration feature A new feature QoS Ocelot feature: Quality of Service aka Polly Spring'25 Spring 2025 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FailureRatio and SamplingDuration parameters of Polly V8 circuit-breaker
3 participants