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

Skip reconnect delay on first reconnect attempt #1257

Closed
wants to merge 1 commit into from

Conversation

timbussmann
Copy link
Contributor

An idea to improve the behavior when using message throughput throttling in which case the implementation always triggers a reconnect. The default implementation then by default waits 10 seconds causing an unexpected delay in immediate retry processing attempts.

At first thought, the first reconnect attempt can be attempted without any delay, while bringing in the delay in case the first reconnect attempt fails.

Not quite sure if that would cause issues with other calls to Reconnect, e.g. in Channel_ModelShutdown, Connection_ConnectionShutdown, and Consumer_ConsumerCancelled though.

Any thoughts on this?

@timbussmann timbussmann requested a review from bording June 14, 2023 13:27
@timbussmann
Copy link
Contributor Author

on playing with this setting a bit further, a problem that needs to be taken into account is that closing the connection will make all the inflight messages immediately reappear on the new connection. Without the delay, this will increase the chance of multiple active processing pipelines that are processing the same message. While the 10 second delay does not fully prevent that, it's likely that it will at least reduce such scenarios.

@timbussmann
Copy link
Contributor Author

closing this as the 10 second delay is currently not necessarily bad to prevent duplicate messages as described on this comment. While the approach proposed in #1258 can gracefully handle the scenario by draining the in-flight messages before reconnecting.

@timbussmann timbussmann deleted the reconnect-delay branch June 16, 2023 14:18
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.

1 participant