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] - Message cleanup for interest stream and no-ack consumers in clustered mode. #2499

Merged
merged 1 commit into from
Sep 8, 2021

Conversation

derekcollison
Copy link
Member

Fixed a bug when an interest retention stream with noack consumers is in clustered mode.
We were not properly propagating the ack state and proper cleanup of the stream messages.

Signed-off-by: Derek Collison derek@nats.io

Resolves #2488 (Related)

/cc @nats-io/core

… in clustered mode.

We were not properly propagating the ack state and proper cleanup of the stream messages.

Signed-off-by: Derek Collison <derek@nats.io>
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM, but small comment about mset.amch

// If we are ack none and mset is interest only we should make sure stream removes interest.
if ap == AckNone && mset.cfg.Retention != LimitsPolicy {
if o.node == nil || o.cfg.Direct {
mset.amch <- seq
Copy link
Member

Choose a reason for hiding this comment

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

You used to check for amch != nil, not sure if this was necessary or not, but you don't do that anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

In stream.go we always create when cfg.Retention != LimitsPolicy, and since we capture that condition above in line 2359 it has to be present.

Copy link
Member

Choose a reason for hiding this comment

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

As long as it is not set to nil (which I just checked it is not), then indeed it is ok.

@derekcollison derekcollison merged commit 309856d into main Sep 8, 2021
@derekcollison derekcollison deleted the interest-noack branch September 8, 2021 22:30
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.

Consumers stops receiving messages
2 participants