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] JetStream: natsSubscription_Destroy should not delete JS consumer #582

Merged
merged 1 commit into from
Sep 16, 2022

Conversation

kozlovic
Copy link
Member

Added comments in the js_Subscribe() calls that explains the behavior of auto created/deleted JS consumers.
It was the intent that auto-created JS consumers would be deleted only when explicitly calling unsubscribe/drain, similar to Go client.

However, in C, the user needs to call natsSubscription_Destroy to free memory. This call is calling internally natsSubscription_Unsubscribe (if the call was not made explicitly), but then we need to disable the deletion of the JS consumer in that case.

Signed-off-by: Ivan Kozlovic ivan@synadia.com

…umer

Added comments in the js_Subscribe() calls that explains the
behavior of auto created/deleted JS consumers.
It was the intent that auto-created JS consumers would be deleted
only when explicitly calling unsubscribe/drain, similar to Go client.

However, in C, the user needs to call natsSubscription_Destroy to
free memory. This call is calling internally natsSubscription_Unsubscribe
(if the call was not made explicitly), but then we need to disable
the deletion of the JS consumer in that case.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Copy link
Member

@aricart aricart left a comment

Choose a reason for hiding this comment

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

LGTM

@kozlovic kozlovic merged commit 75166b4 into main Sep 16, 2022
@kozlovic kozlovic deleted the js_fix_sub_destroy branch September 16, 2022 20:48
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.

2 participants