Skip to content

JetStream additions #2432

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

Merged
merged 3 commits into from
Aug 11, 2021
Merged

JetStream additions #2432

merged 3 commits into from
Aug 11, 2021

Conversation

derekcollison
Copy link
Member

This adds two items.

  1. Domain information for PubAcks.
  2. Active state for push consumers that details active subject and optionally queue group.

Signed-off-by: Derek Collison [email protected]

/cc @nats-io/core

Signed-off-by: Derek Collison <[email protected]>
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 (at least the PushActive part)

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

Signed-off-by: Derek Collison <[email protected]>
@@ -69,6 +70,11 @@ type ConsumerConfig struct {
Direct bool `json:"direct,omitempty"`
}

type PushActive struct {
Subject string `json:"subject"`
Queue string `json:"queue,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

but there could be multiple queue groups on the same subject - and some unqueued?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, once a consumer is bound to a queue group, meaning we know you want to do scale out on this consumer, its bound to that queue group.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, 2 nats clients can have queue groups on this subject, 2 different queue groups. And there could also be subscribes on it that has no queue group - while others does.

You specifically take only the first, but thats just not deterministic at all because if there are 2 clients each with its own queue group one server will report them one way and another another way depending on how and when splits happened.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just like two apps having different delivery subjects (e.g. _INBOX.>) only one is bound to the server side consumer. This is similar in that only one queue group should be bound to the delivery subject for scale out binding to a single consumer.

If someone binds that shouldn't, the app that should be bound will fail to bind with a queue mismatch error.

mset.pubAck = []byte(fmt.Sprintf("{%q:%q, %q:%q, %q:", "stream", cfg.Name, "domain", domain, "seq"))
} else {
mset.pubAck = []byte(fmt.Sprintf("{%q:%q, %q:", "stream", cfg.Name, "seq"))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

After this pubacks are broken.

I am seeing acks like:

{"stream":"GOVERNOR_TEST", "seq":10

no closing }

Copy link
Member Author

Choose a reason for hiding this comment

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

I am looking at a race at the moment in that section, will have PR shortly.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

3 participants