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

Shutdown subscription_executor on close and reconnect #155

Merged
merged 3 commits into from
Jan 10, 2025
Merged

Conversation

vankiru
Copy link
Collaborator

@vankiru vankiru commented Jan 8, 2025

This is a fix for #153.

Close connection

On close connection, we shut down subscription_executor and give subscriptions options[:close_timeout] (30 seconds by default) to finish their work with the NATS server. After that, we proceed with the rest of the logic.

Reconnect

On reconnection, we shut down subscriprion_executor by initiating an orderly termination of all in-progress tasks but do not wait for them to finish. After the threads finish their work, they will be automatically closed in the background.

@vankiru vankiru force-pushed the evl/issue-153 branch 4 times, most recently from 2b32655 to 431b6cc Compare January 10, 2025 09:19
@vankiru vankiru marked this pull request as ready for review January 10, 2025 13:41
@palkan palkan requested review from palkan and wallyqs and removed request for palkan January 10, 2025 17:45
Copy link
Collaborator

@palkan palkan left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

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

LGTM 🙏

@wallyqs wallyqs merged commit 41bc948 into main Jan 10, 2025
14 checks passed
@wallyqs wallyqs deleted the evl/issue-153 branch January 10, 2025 18:01
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.

3 participants