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

Implement tcp_reconnect_backoff minion option #59432

Merged
merged 1 commit into from Mar 4, 2021

Conversation

lukasraska
Copy link
Contributor

What does this PR do?

Implements new tcp_reconnect_backoff minion config option for dynamic reconnect interval on TCP transport

What issues does this PR fix or reference?

Fixes: #59431

Previous Behavior

Minion would try to reconnect exactly after 1 second from previous disconnect

New Behavior

Reconnect backoff follows tcp_reconnect_backoff option (with 1 second default)

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

@lukasraska lukasraska requested a review from a team as a code owner February 4, 2021 21:41
@lukasraska lukasraska requested review from dwoz and removed request for a team February 4, 2021 21:41
Ch3LL
Ch3LL previously approved these changes Feb 11, 2021
@sagetherage sagetherage added Aluminium Release Post Mg and Pre Si Feature new functionality including changes to functionality and code refactors, etc. labels Feb 16, 2021
Copy link
Contributor

@dwoz dwoz left a comment

Choose a reason for hiding this comment

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

Please change tcp_authentication_retries toauthentication_retries so we can parody this feature in other transports.

@lukasraska
Copy link
Contributor Author

@dwoz This PR implements tcp_reconnect_backoff option (the tcp_authentication_retries had just misnamed anchor in docs, so I've changed it since it's in the same section). I can rename the tcp_reconnect_backoff option to reconnect_backoff so it can be reused for ZeroMQ (for ZMQ_RECONNECT_IVL or similar) as well. I can also change the tcp_authentication_retries to authentication_retries If you want, but since it's existing opt, I would do that in separate PR.

@dwoz
Copy link
Contributor

dwoz commented Mar 3, 2021

@dwoz This PR implements tcp_reconnect_backoff option (the tcp_authentication_retries had just misnamed anchor in docs, so I've changed it since it's in the same section). I can rename the tcp_reconnect_backoff option to reconnect_backoff so it can be reused for ZeroMQ (for ZMQ_RECONNECT_IVL or similar) as well. I can also change the tcp_authentication_retries to authentication_retries If you want, but since it's existing opt, I would do that in separate PR.

Okay, that makes sense. I just prefer we consolidate them in some manner in a future PR. Thanks!

@Ch3LL Ch3LL merged commit 051dbe4 into saltstack:master Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aluminium Release Post Mg and Pre Si Feature new functionality including changes to functionality and code refactors, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] Configurable reconnect backoff interval for TCP transport
5 participants