-
Notifications
You must be signed in to change notification settings - Fork 5
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
Working with a load balancer #135
Merged
Merged
Conversation
This file contains 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
Closed
tarzacodes
changed the title
WIP - Working with a load balancer
Working with a load balancer
Jan 5, 2024
icappello
previously approved these changes
Jan 5, 2024
tarzacodes
force-pushed
the
103-load-balance
branch
2 times, most recently
from
January 8, 2024 10:24
868f91a
to
e5d63cf
Compare
* feat: added client side filtering of messages if offset is of type numeric, and a bunch of tests for offsets * chore: removed useless timeout from test --------- Co-authored-by: Luca <lmenghini@coders51.com>
tarzacodes
force-pushed
the
103-load-balance
branch
from
January 8, 2024 10:36
e5d63cf
to
e1dc2ca
Compare
l4mby
approved these changes
Jan 8, 2024
icappello
approved these changes
Jan 8, 2024
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
albertobarrila
approved these changes
Jan 8, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See https://blog.rabbitmq.com/posts/2021/07/connecting-to-streams/ - part "Client Workaround With a Load Balancer"
Basically, if we are dealing with a load balancer we cannot simply get the node and use the
advertised_host
andadvertised_port
: they might be unreachable for anyone BUT the load balancer. We need to connect to the load balancer and connect to a node through it. Then check if the node we got from the LB is what we wanted (a leader for a Producer, a replica for a Consumer, anything goes for a Locator) and if it is not, close the connection and retry.ADDENDUM: All the clients I've looked thru (Java, Go, dotnet) do the matching with the advertised_host / advertised_port and I've unable to find a way to extract the actual hosts and port, therefore at least for now we shall match with those.
THEREFORE THIS NO LONGER APPLIES:
We'll use the host to match - if a LB is used whoever set up the Rabbit cluster should not have configured
advertised_host
andadvertised_port
and the match should be done on the host name and port.