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

Update README.md #1475

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update README.md #1475

wants to merge 1 commit into from

Conversation

p4xx07
Copy link

@p4xx07 p4xx07 commented Nov 20, 2023

This reconnect part does not work if you don't include also the PingInterval

This reconnect part does not work if you don't include also the PingInterval
@Allan-Nava
Copy link

+1

@@ -341,6 +341,7 @@ nc.QueueSubscribe("foo", "job_workers", func(_ *Msg) {
// the connection in reconnecting state if it failed to connect right away.
nc, err := nats.Connect(nats.DefaultURL,
nats.RetryOnFailedConnect(true),
nats.PingInterval(time.Second),
Copy link
Member

Choose a reason for hiding this comment

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

1 sec for the ping interval is super low, 30s to 2 minutes would be a better duration. Though you should not have to set this so would be good to double check the root cause at what what is happening here

Copy link
Author

Choose a reason for hiding this comment

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

I can bump it up to 30 I agree with you.

@piotrpio
Copy link
Collaborator

@p4xx07 PingInterval has a default value of 2 minutes if you don't provide it, and it should not be necessary to use the option for RetryOnFailedConnect to work. Can you maybe create an issue with some more details if you think there is a bug in the library?

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.

None yet

4 participants