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

feat: add pause and resume jetstream consumer #1571

Merged
merged 1 commit into from Mar 15, 2024

Conversation

yordis
Copy link
Contributor

@yordis yordis commented Feb 27, 2024

closes #1569

@yordis yordis force-pushed the issues/1569 branch 5 times, most recently from 181cd77 to 481ab36 Compare February 27, 2024 18:42
go.mod Outdated Show resolved Hide resolved
@yordis yordis force-pushed the issues/1569 branch 2 times, most recently from 808189f to 12b1e06 Compare February 27, 2024 21:44
go_test.mod Outdated Show resolved Hide resolved
@yordis yordis force-pushed the issues/1569 branch 2 times, most recently from 12fa780 to e17e2a9 Compare February 27, 2024 22:26
@yordis
Copy link
Contributor Author

yordis commented Feb 27, 2024

I need help with:

  1. What should be the final public API (should I remove resume and expect people to call Pause with the time.Now())
  2. Verify that the payload is correct by not allowing nil time

CONTRIBUTING.md Outdated Show resolved Hide resolved
@ripienaar
Copy link
Contributor

In jsm.go - that is the underlying library for the CLI I added:

  • PauseUntil(deadline time.Time) as a create option to create the consumer paused
  • Pause(deadline time.Time) to pause a already created consumer
  • Resume() to resume a paused consumer
  • IsPaused() on a consumer - which I now notice I forgot to add

CLI got:

  consumer pause    Pause a consumer until a later time
  consumer resume   Resume a paused consumer

So seems good to me to have pause and resume behaviors in APIs, the original ask from the server was to have a Resume API also but we decided to take a bit of a shortcut with the nil time behavior. I think client facing be good to have Pause/Resume both.

jetstream/jetstream.go Outdated Show resolved Hide resolved
go_test.mod Outdated Show resolved Hide resolved
jetstream/consumer.go Show resolved Hide resolved
jetstream/jetstream.go Outdated Show resolved Hide resolved
jetstream/jetstream.go Outdated Show resolved Hide resolved
jetstream/stream.go Outdated Show resolved Hide resolved
@yordis yordis force-pushed the issues/1569 branch 5 times, most recently from 5e31e66 to fb007dd Compare February 28, 2024 20:24
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
Copy link
Contributor Author

yordis commented Feb 29, 2024

Hey guys, any update over here?

@piotrpio piotrpio changed the base branch from main to v2.11-dev February 29, 2024 20:23
Copy link
Collaborator

@piotrpio piotrpio left a comment

Choose a reason for hiding this comment

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

Great work, LGTM!

I changed the target branch to v2.11-dev since we don't want to merge to main until the feature is released on the server.

@yordis
Copy link
Contributor Author

yordis commented Feb 29, 2024

Waiting to be merge so I move to the Elixir version

@piotrpio piotrpio merged commit cb9fabe into nats-io:v2.11-dev Mar 15, 2024
1 check passed
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.

Implement Consumers Pause
4 participants