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] Recreating ordered consumers on server restart #1425

Merged
merged 4 commits into from Oct 15, 2023

Conversation

piotrpio
Copy link
Collaborator

No description provided.

Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
@ColinSullivan1
Copy link
Member

ColinSullivan1 commented Oct 13, 2023

Hi @piotrpio and @Jarema - thank you for resolving this issue! Would you have an ETA for the next release with this fix? Unfortunately this issue is impacting a deployment of NATS in production.

Copy link
Member

@Jarema Jarema left a comment

Choose a reason for hiding this comment

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

Overall, a solind improvement.

Just few comments to consider/answer.

if c.currentConsumer.subscriptions[""] != nil {
c.currentConsumer.subscriptions[""].Stop()
}
consName := c.currentConsumer.CachedInfo().Name
Copy link
Member

Choose a reason for hiding this comment

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

are we confident that consumer name is updated after recreation, so we always delete the proper one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should be good, in line 443 we do:

c.currentConsumer = cons.(*pullConsumer)

This is done right after successful create.

jetstream/pull.go Show resolved Hide resolved
jetstream/pull.go Show resolved Hide resolved
Copy link
Member

@Jarema Jarema left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
@piotrpio piotrpio merged commit 5e46542 into main Oct 15, 2023
1 of 2 checks passed
@piotrpio piotrpio deleted the reset-ordered-consumer branch October 15, 2023 18:06
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

3 participants