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

[FIXED] LeafNode: connection may fail on slow link #5424

Merged
merged 1 commit into from
May 16, 2024
Merged

Conversation

kozlovic
Copy link
Member

@kozlovic kozlovic commented May 15, 2024

Added the leafnode remote configuration parameter first_info_timeout
which is the amount of time that a server creating a leafnode
connection will wait for the initial INFO from the remote server.

Resolves #5417

Signed-off-by: Ivan Kozlovic [email protected]

@kozlovic kozlovic requested a review from a team as a code owner May 15, 2024 21:02
@kozlovic kozlovic requested a review from derekcollison May 15, 2024 21:12
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.

If no one sets anything, how long can a leafnode exists before being declared stalled?

@kozlovic
Copy link
Member Author

If no one sets anything, how long can a leafnode exists before being declared stalled?

pingInterval*time.Duration(pingMax+1), which since default ping interval is 2min, and max pings 2, it would be 2min*3=6min. Now for route and gateways, we cap the ping interval to 15 seconds I believe, so it is much smaller.

If you think this is too much, I could pass different values, but then it means adding a new configuration setting? (which was what the user wanted: make the wait configurable).

@derekcollison
Copy link
Member

I think we want to make the wait configurable for leafnodes vs using the ping mechanism.

@kozlovic
Copy link
Member Author

I think we want to make the wait configurable for leafnodes vs using the ping mechanism.

Ok, so a new value in a leaf node's remote configuration block and I assume that if not set, default to that constant that we were using.

@derekcollison
Copy link
Member

Yes I believe so..

Added the leafnode remote configuration parameter `first_info_timeout`
which is the amount of time that a server creating a leafnode
connection will wait for the initial INFO from the remote server.

Resolves #5417

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

@derekcollison Sorry, did a force push with the amend because I wanted the commit log to be in phase with the use of a new config param instead of the original approach.

@derekcollison
Copy link
Member

Just ping when ready for review.

@kozlovic
Copy link
Member Author

It was/is ready for review. I was just saying that I did a push force (which I usually don't do once a code review has started).

@derekcollison derekcollison self-requested a review May 16, 2024 00:57
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

@derekcollison derekcollison merged commit deb6350 into main May 16, 2024
4 checks passed
@derekcollison derekcollison deleted the issue_5417 branch May 16, 2024 00:59
neilalexander added a commit that referenced this pull request Feb 21, 2025
Includes the following:

- #6524
- #6525
- #6526
- #5424
- #6565
- #6532

Signed-off-by: Neil Twigg <[email protected]>
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.

Connection between Leafnode and Core NATS over satellite link fails to get established
2 participants