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

This commit allows new servers in a supercluster to be informed of accounts with leafnodes. #1327

Merged
merged 2 commits into from
Apr 8, 2020

Conversation

derekcollison
Copy link
Member

This is needed to put those accounts into interest only mode for inbound gateway connections. Also added code to make sure we were doing proper account tracking and would track the global account as well, which used to be excluded.

Fixes #977

Signed-off-by: Derek Collison derek@nats.io

/cc @nats-io/core

…ounts with active leafnode connections.

This is needed to put those accounts into interest only mode for inbound gateway connections. Also added code
to make sure we were doing proper account tracking and would track the global account as well, which used to
be excluded.

Fixes #977

Signed-off-by: Derek Collison <derek@nats.io>
@derekcollison derekcollison requested a review from kozlovic April 7, 2020 23:37
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

Since we are regenerating the leafnode connect event, which I consider brute-force, I would make sure that this is also done when GW connections are established. See comment for more details.

server/events.go Outdated
// connect update to make sure they switch this account to interest only mode.
// TODO(dlc) - this will cause this account to be loaded on all servers. Need a better
// way with GW2.
if s.gateway.enabled && len(s.leafs) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

With this approach, we are at the mercy of the system event $SYS.SERVER.ACCOUNT.%s.CONNS to be sent before switching to interest-only mode in the situation where B is restarted in the setup: A <-(GW)- B <- (LN) - C.
This is because C will likely recreate the LN connection to B before B has setup the GW connection to A, so the LN connect event is "lost". Once the GW connection is established, we have to wait for the event hb interval to have A sends the account's conn update for the switch to be made.

I would suggest moving this code to a separate function (and invoke that function from here) as such:

// If GW is enabled on this server and there are any leaf node connections,
// this function will send a LeafNode connect system event to the super cluster
// to ensure that the GWs are in interest-only mode for this account.
// Lock should be held upon entry.
func (s *Server) ensureGWsInterestOnlyForLeafNodes() {
	if !s.gateway.enabled || len(s.leafs) == 0 {
		return
	}
	sent := make(map[*Account]bool, len(s.leafs))
	for _, c := range s.leafs {
		if !sent[c.acc] {
			fmt.Printf("@@IK: HERE!\n")
			s.sendLeafNodeConnectMsg(c.acc.Name)
			sent[c.acc] = true
		}
	}
}

and call this function in gateway.go:1080, when processing inbound first INFO protocol:

    s.mu.Lock()
    s.ensureGWsInterestOnlyForLeafNodes()
    s.mu.Unlock()

This will make the switch occur when the GW connection are made, which should also solve the case when there is a disconnect/reconnect of GW connection due to a slow consumer for instance.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's actually not the conns event that triggers it but seeing new servers that will eventually fix it, but most likely they miss the original and need to wait on HB intervals to see the new servers and respond. That is the new code I added.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with your suggestion above. It is a blunt instrument for now until we have something like GW2.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok take another look, posted updates.

Signed-off-by: Derek Collison <derek@nats.io>
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM!

@derekcollison derekcollison merged commit 743c6ec into master Apr 8, 2020
@derekcollison derekcollison deleted the leaf_staggered branch April 8, 2020 18:02
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.

LeafNode: need to remember to switch GW to interest-mode only
2 participants