Skip to content
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

fix: Enhance gorouter retry logic (#437) #443

Merged
merged 3 commits into from
Nov 4, 2024

Conversation

b1tamara
Copy link
Contributor

@b1tamara b1tamara commented Oct 4, 2024

Summary

The fix for the issue described in #437.

  • We propose to use the pool index to "seed" each iterator and next calls on that iterator will then loop over the entire pool exactly once (without to affect the global pool iterator).
  • Implement additional tests for two parallel iterator
  • Adapt the test cases in round_robin_test.go which were bound to pool nextIndex strongly (e.g. "RoundRobin Next when locally-optimistic mode on the first attempt when the pool has multiple in the same AZ as the router when all AZ-local endpoints have errors it resets the errors and returns one of the endpoints regardless of AZ" to check not a specific CanonicalAddr but to validate that one endpoint from other AZ has been returned)

Backward Compatibility

Breaking Change? No

@b1tamara b1tamara marked this pull request as ready for review October 17, 2024 08:54
@b1tamara b1tamara requested a review from a team as a code owner October 17, 2024 08:54
@ameowlia
Copy link
Member

@b1tamara can you add some comments to your tests about exactly what the different loops are doing? It took me a bit of looking to grok them 😅.

Other than that, it LGTM.

@b1tamara
Copy link
Contributor Author

@ameowlia
Could you please take a look again? I tryed to add more text context with By() to the new test to make it more readable/clear.

@ameowlia
Copy link
Member

ameowlia commented Nov 4, 2024

👍 Thanks for the updates. It LGTM. Waiting to merge until the pipeline is green. I will keep an eye on it.

@ameowlia ameowlia merged commit 9ee77f4 into cloudfoundry:main Nov 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants