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

JetStreamManager.AddConsumer ambiguous behaviour with existing consumer #1542

Closed
bbanelli opened this issue Feb 1, 2024 · 5 comments
Closed
Assignees
Labels
defect Suspected defect such as a bug or regression

Comments

@bbanelli
Copy link

bbanelli commented Feb 1, 2024

Observed behavior

AddConsumer returns no error if it "creates" already existing consumer with exactly the same configuration options.

Expected behavior

Return JSErrCodeConsumerAlreadyExists for the same configuration of existing consumer on the server.

Server and client version

nats-server

Version: 2.10.9
Git Commit: a5298e2

Driver

github.com/nats-io/nats.go v1.32.0

Host environment

Host

go version go1.20.13 windows/amd64

Server

x64/aarch64 Linux NATS cluster with versions 2.10.9

Steps to reproduce

Invoke AddConsumer with a nats.ConsumerConfig{} configuration and repeat it again with the same configuration.

I would either expect an error or more verbose documentation about this behavior.

@bbanelli bbanelli added the defect Suspected defect such as a bug or regression label Feb 1, 2024
@Jarema
Copy link
Member

Jarema commented Feb 1, 2024

This behaviour is on purpose, as in distributed systems like NATS, retrying the same operation in case of failure might be necessary in case of transient unavailability and by making Create idempotent, we ensure that no suprisings errors will happen if first attempt timed out, while second sends error already created. This can happen if first request actually worked, but failed to send confirmation. If Create would send error, from user perspective, none of operations were successful.

The distinction between create and update is to:

  1. Protect Update from accidental cration of non-existing resources
  2. Protect Create from accidental update of existing resource

If config is exactly the same, it's a noop for the server/meta leader.

@bbanelli
Copy link
Author

bbanelli commented Feb 1, 2024

@Jarema thank you for the quick update. However, as I said, than this should be explicitly documented, correct? I cannot find any reference about this behavior, yet your verbose explanation is very precise and helpful, so perhaps it could be incorporated in the docs right away? :)

@Jarema
Copy link
Member

Jarema commented Feb 1, 2024

Yes, we can definately add it into the docs, especially as our next release will contain a big docs improvements (see #1537 and #1532).

@bbanelli
Copy link
Author

bbanelli commented Feb 1, 2024

This is fantastic info! Thank you so much for your quick support; actually, this is desired behavior for my use case, I just wasn't sure if it is designed exactly that way!

@Jarema Jarema self-assigned this Feb 1, 2024
@piotrpio
Copy link
Collaborator

The documentation for AddConsumer was improved in a recent PR: #1555

Thanks for bringing this to our attention!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Development

No branches or pull requests

3 participants