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.

Sign up for GitHub

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

Jump to bottom

Add support for pausing consumers #5066

Merged
merged 3 commits into from
Feb 26, 2024
Merged

Add support for pausing consumers #5066

merged 3 commits into from
Feb 26, 2024

Conversation

neilalexander
Copy link
Member

@neilalexander neilalexander commented Feb 12, 2024 •
edited
Loading

This adds a new pause_until configuration option for pausing consumers.

It can either be set when the consumer is created (but not via a standard consumer update) or it can be changed later by using the new $JS.API.CONSUMER.PAUSE.*.* API endpoint by sending:

{"pause_until": "2024-02-08T19:00:00Z"}

Any time that is in the past, or a zero timestamp, is considered as "unpaused". Once the consumer reaches the pause_until time, messages will start flowing again automatically.

The consumer info will additionally include paused (type bool) and, if paused, a pause_remaining (type time.Duration) to report the pause status.

Also adds $JS.EVENT.ADVISORY.CONSUMER.PAUSE.*.* advisory messages which are sent when pausing and unpausing (i.e. reaching the pause deadline).

Idle heartbeats continue to be sent while the consumer is paused to satisfy liveness checks.

Before merge: nats-io/nats.go#1554 and go.mod updated.

Signed-off-by: Neil Twigg neil@nats.io

@neilalexander neilalexander force-pushed the neil/consumerpause branch 4 times, most recently from d773766 to 81b0221 Compare February 14, 2024 10:32
@ripienaar
Copy link
Contributor

ripienaar commented Feb 14, 2024 •
edited
Loading

Have tested:

  • Advisories are created when creating durables and ephemerals ✅
  • Advisories only sent by the consumer leader (being fixed) ✅
  • Creating paused works, and sends advisories ✅
  • Creating unpaused does not pause and does not send advisories ✅
  • Resume happens on schedule in both created paused or set paused by the api ✅
  • Paused state survives consumer leader elections ✅
  • Paused state survives full cluster restarts ✅
  • Waiting pulls will get the messages they are waiting on soon as they resume ✅
  • Calling consumer create with a different time doesnt update the paused until and doesnt change the paused state ✅
  • Config and State items works fine ✅
  • Pause API can pause and resume consumers ✅

Good stuff

@neilalexander neilalexander force-pushed the neil/consumerpause branch 5 times, most recently from e5255cc to 27e87b6 Compare February 16, 2024 11:33
@neilalexander neilalexander marked this pull request as ready for review February 16, 2024 12:44
@neilalexander neilalexander requested a review from a team as a code owner February 16, 2024 12:44
ripienaar
ripienaar approved these changes Feb 16, 2024
View reviewed changes
Copy link
Contributor

@ripienaar ripienaar left a comment

Choose a reason for hiding this comment

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

LGTM functionality wise

derekcollison
derekcollison reviewed Feb 16, 2024
View reviewed changes
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.

Looks really good, few comments.

go.mod Outdated Show resolved Hide resolved
server/consumer.go Show resolved Hide resolved
server/consumer.go Outdated Show resolved Hide resolved
server/consumer.go Show resolved Hide resolved
server/jetstream_api.go Show resolved Hide resolved
server/jetstream_api.go Outdated Show resolved Hide resolved
server/jetstream_cluster.go Outdated Show resolved Hide resolved
server/jetstream_events.go Show resolved Hide resolved
@derekcollison derekcollison self-requested a review February 16, 2024 18:55
derekcollison
derekcollison reviewed Feb 16, 2024
View reviewed changes
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 - Let's figure out client piece.

In general when I introduce new things to the server I simply bypass the client and do them all manually (See direct get batch etc). And place a note to change over once client supports it. I suggest we do that here.

@ripienaar ripienaar mentioned this pull request Feb 20, 2024
Closed
@neilalexander
Copy link
Member Author

The unit tests have been updated to use new jsTestPause_CreateOrUpdateConsumer and jsTestPause_PauseConsumer functions that understand the new ConsumerConfig and ConsumerInfo fields.

Therefore they no longer rely on upstream changes in nats.go and the replace directive has been removed.

@derekcollison derekcollison self-requested a review February 24, 2024 18:18
derekcollison
derekcollison reviewed Feb 24, 2024
View reviewed changes
server/consumer.go Outdated
@@ -1860,6 +1930,22 @@ func (o *consumer) updateConfig(cfg *ConsumerConfig) error {
o.dtmr = time.AfterFunc(o.dthresh, o.deleteNotActive)
}
}
// heck whether the pause has changed
Copy link
Member

Choose a reason for hiding this comment

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

check

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, fixed & rebased!

derekcollison
derekcollison approved these changes Feb 24, 2024
View reviewed changes
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 - One nit on spelling.

Nice work!

@neilalexander
This adds a new pause_until configuration option for pausing consum…
6f9fc13 
…ers.

It can either be set when the consumer is created (but not via a
standard consumer update) or it can be changed later by using the new
`$JS.API.CONSUMER.PAUSE.*.*` API endpoint by sending:
```
{"pause_until": "2024-02-08T19:00:00Z"}
```

Any time that is in the past, or a zero timestamp, is considered as
"unpaused". Once the consumer reaches the `pause_until` time, messages
will start flowing again automatically.

The consumer info will additionally include `paused` (type `bool`) and,
if paused, a `pause_remaining` (type `time.Duration`) to report the pause
status.

Also adds `$JS.EVENT.ADVISORY.CONSUMER.PAUSE.*.*` advisory messages which
are sent when pausing and unpausing (i.e. reaching the pause deadline).

Idle heartbeats continue to be sent while the consumer is paused to
satisfy liveness checks.

Signed-off-by: Neil Twigg <neil@nats.io>
@neilalexander
Pointerise PauseUntil in ConsumerConfig
4a44009 
Signed-off-by: Neil Twigg <neil@nats.io>
@neilalexander
Use helper functions for pause consumer tests
88b3027 
Signed-off-by: Neil Twigg <neil@nats.io>
derekcollison
derekcollison approved these changes Feb 26, 2024
View reviewed changes
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

@derekcollison derekcollison merged commit bb5a959 into main Feb 26, 2024
4 checks passed
@derekcollison derekcollison deleted the neil/consumerpause branch February 26, 2024 02:30
This was referenced Feb 26, 2024
Open
Open
Closed
Closed
Closed
Closed
Open
Open
@scottf scottf mentioned this pull request Feb 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ripienaar ripienaar ripienaar approved these changes

@derekcollison derekcollison derekcollison approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@neilalexander @ripienaar @derekcollison