Skip to content

Handle infinite looping when encountering a misconfigured load balancer #121

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

Merged
merged 7 commits into from
May 17, 2022
Merged

Handle infinite looping when encountering a misconfigured load balancer #121

merged 7 commits into from
May 17, 2022

Conversation

ricsiLT
Copy link
Contributor

@ricsiLT ricsiLT commented May 17, 2022

Proposal to handle infinite looping when connecting to a load balancer where nodes are misconfigured, causing client to hang indefinitely.

Logic can be replaced by time-based loop, separate exception could be used if necessary (maybe even exposing nodes that were tried for easier diagnostics).

@Gsantomaggio Gsantomaggio self-requested a review May 17, 2022 11:18
@Gsantomaggio Gsantomaggio self-assigned this May 17, 2022
@Gsantomaggio Gsantomaggio added this to the v1.0.0-beta.6 milestone May 17, 2022
@Gsantomaggio
Copy link
Member

Thank you for your PR. I left a comment

@ricsiLT
Copy link
Contributor Author

ricsiLT commented May 17, 2022

Submitted changes. Tried running unit tests with your approach ((1 + replicas.Count) * 2) but it would spuriously fail. Tell me if Pow() is too much :D

@Gsantomaggio
Copy link
Member

Can you please execute dotnet format and then commit again? Thank you

@ricsiLT
Copy link
Contributor Author

ricsiLT commented May 17, 2022

Sorry for multiple commits, can only do this PR via github web UI.

@Gsantomaggio
Copy link
Member

@ricsiLT don't worry :)! We will Squash the commits!

@Gsantomaggio
Copy link
Member

In order to format you need to use the last version:

$ dotnet --version
6.0.300

they changed something, so previous versions won't work.

if you can't update dotnet, please give me the grant I will format the code.

Signed-off-by: Gabriele Santomaggio <[email protected]>
Gsantomaggio
Gsantomaggio previously approved these changes May 17, 2022
@ricsiLT
Copy link
Contributor Author

ricsiLT commented May 17, 2022

So you'll merge this whenever? Nothing else required from me?

@Gsantomaggio
Copy link
Member

So you'll merge this whenever?

I formatted the code. I will merge! Thank you

The same for producer and consumer. Just to be sure to try all the nodes at least tree times to avoid
temp fails

Signed-off-by: Gabriele Santomaggio <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #121 (71a802f) into main (82ab5ff) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 71a802f differs from pull request most recent head b9f7a26. Consider uploading reports for the commit b9f7a26 to get more accurate results

@@            Coverage Diff             @@
##             main     #121      +/-   ##
==========================================
- Coverage   91.58%   91.57%   -0.01%     
==========================================
  Files          72       72              
  Lines        5301     5331      +30     
  Branches      332      332              
==========================================
+ Hits         4855     4882      +27     
- Misses        365      370       +5     
+ Partials       81       79       -2     
Impacted Files Coverage Δ
RabbitMQ.Stream.Client/RoutingClient.cs 95.00% <100.00%> (-5.00%) ⬇️
Tests/UnitTests.cs 92.22% <100.00%> (-5.42%) ⬇️
RabbitMQ.Stream.Client/Consumer.cs 89.18% <0.00%> (+0.90%) ⬆️
Tests/ClientTests.cs 97.29% <0.00%> (+1.35%) ⬆️
RabbitMQ.Stream.Client/WireFormatting.cs 74.82% <0.00%> (+4.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82ab5ff...b9f7a26. Read the comment docs.

@Gsantomaggio Gsantomaggio merged commit b3a646b into rabbitmq:main May 17, 2022
@lukebakken lukebakken modified the milestones: v1.0.0-beta.6, v1.0.0-rc.1 May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants