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

js: Prevent sending duplicated Acks #637

Merged
merged 1 commit into from Jan 22, 2021
Merged

Conversation

wallyqs
Copy link
Member

@wallyqs wallyqs commented Jan 20, 2021

Add suppression so that a msg is only attempted to be acked a single time (other than InProgress acks).

Fixes #634

Signed-off-by: Waldemar Quevedo wally@synadia.com

js.go Outdated Show resolved Hide resolved
@wallyqs wallyqs force-pushed the js-ack-once branch 3 times, most recently from 1db4e05 to bfd077e Compare January 21, 2021 08:56
func (m *Msg) Term() error {
return m.ackReply(AckTerm, false)
}

// Indicate that this message is being worked on and reset redelkivery timer in the server.
// InProgress indicates that this message is being worked on
// and reset the redelivery timer in the server.
func (m *Msg) InProgress() error {
return m.ackReply(AckProgress, false)
Copy link
Member

Choose a reason for hiding this comment

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

We could issue lots of these, the suppression code would suppress so we may need to look at ack type and the .ackd var.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to skip suppressing acks in case they are to progress report.

@wallyqs wallyqs force-pushed the js-ack-once branch 3 times, most recently from 5dfda1a to b01f159 Compare January 21, 2021 23:06
@coveralls
Copy link

coveralls commented Jan 21, 2021

Coverage Status

Coverage increased (+0.3%) to 87.303% when pulling bddf81b on wallyqs:js-ack-once into 7fb8bac on nats-io:master.

js.go Show resolved Hide resolved
Signed-off-by: Waldemar Quevedo <wally@synadia.com>
Copy link
Member

@derekcollison derekcollison 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 fccc82d into nats-io:master Jan 22, 2021
@wallyqs wallyqs deleted the js-ack-once branch January 22, 2021 03:11
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.

m.Ack() sends duplicate when auto-ack enabled
3 participants