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

Cherry-picks for 2.10.22-RC.2 #5984

Merged
merged 4 commits into from
Oct 10, 2024
Merged

Cherry-picks for 2.10.22-RC.2 #5984

merged 4 commits into from
Oct 10, 2024

Conversation

neilalexander
Copy link
Member

Includes the following:

Signed-off-by: Neil Twigg [email protected]

neilalexander and others added 4 commits October 10, 2024 15:31
While writing the test, I needed to make sure that each server in
the hub has registered interest for 2 queue subscribers from the
same group. I noticed that `Sublist.NumInterest()` (that I was
invoking from `Account.Interest()` was returning 1, even after
I knew that the propagation should have happened. It turns out
that `NumInterest()` was returning the number of queue groups, not
the number of queue subs in all those queue groups.

For the leafnode queue balancing issue, the code was favoring
local/routed queue subscriptions, so in the described issue,
the message would always go from HUB1->HUB2->LEAF2->QSub instead
of HUB1->LEAF1->QSub.

Since we had another test that was a bit reversed where we had
a HUB and LEAF1<->LEAF2 connecting to HUB and a qsub on both
HUB and LEAF1 and requests originated from LEAF2, and we were
expecting all responses to come from LEAF1 (instead of the
responder on HUB), I went with the following approach:

If the message originates from a client that connects to a server
that has a connection from a remote LEAF, then we pick that LEAF the
same as if it was a local client or routed server.
However, if the client connects to a server that has a leaf
connection to another server, then we keep track of the sub
but do not sent to that one if we have local or routed qsubs.

This makes the 2 tests pass, solving the new test and maintaining
the behavior for the old test.

Signed-off-by: Ivan Kozlovic <[email protected]>
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@neilalexander neilalexander marked this pull request as ready for review October 10, 2024 16:06
@neilalexander neilalexander requested a review from a team as a code owner October 10, 2024 16:06
@neilalexander neilalexander merged commit cc35c9a into release/v2.10.22 Oct 10, 2024
5 checks passed
@neilalexander neilalexander deleted the neil/21022rc2 branch October 10, 2024 16:07
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