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

KV ResumeAtRevision option for watches #478

Open
mtmk opened this issue Apr 16, 2024 · 3 comments · May be fixed by #491
Open

KV ResumeAtRevision option for watches #478

mtmk opened this issue Apr 16, 2024 · 3 comments · May be fixed by #491
Assignees
@niklasfp
Labels
good first issue Good for newcomers help wanted Extra attention is needed new-feature

Comments

@mtmk
Copy link
Collaborator

mtmk commented Apr 16, 2024 •
edited

@mtmk mtmk added good first issue Good for newcomers help wanted Extra attention is needed new-feature labels Apr 16, 2024
@niklasfp
Copy link
Contributor

@mtmk as per slack conversation, I'll like to give this one a shot.

Initial idea:

NatsKvOpts.cs
Would add ResumeAtRevision int prop to NatsKVWatchOpts (document it to be "The revision to start from, if set to 0 (default) this will be ignored"

nats.net.v2/src/NATS.Client.KeyValueStore/NatsKVOpts.cs

Line 43 in dcfaac1

public Func<CancellationToken, ValueTask<bool>>? OnNoData { get; init; }

NatsKVWatcher.cs
It's using the _sequenceStream here

nats.net.v2/src/NATS.Client.KeyValueStore/Internal/NatsKVWatcher.cs

Line 353 in dcfaac1

var sequence = Volatile.Read(ref _sequenceStream);

And I was thinking about setting _sequenceStream = opts.ResumeAtRevision here https://github.com/nats-io/nats.net.v2/blob/main/src/NATS.Client.KeyValueStore/Internal/NatsKVWatcher.cs#L78

@mtmk
Copy link
Collaborator Author

mtmk commented Apr 24, 2024

@niklasfp looking at this a bit closer I think it's good. Setting the _sequenceStream = opts.ResumeAtRevision in NatsKVWatcher.ctor should work since it will treacle down to the consumer create request down below and also should be updated as expected on reconnects. feel free to open a PR when you're ready and have time. thank you 💯

@niklasfp niklasfp linked a pull request Apr 24, 2024 that will close this issue
Open
@niklasfp
Copy link
Contributor

@niklasfp looking at this a bit closer I think it's good. Setting the _sequenceStream = opts.ResumeAtRevision in NatsKVWatcher.ctor should work since it will treacle down to the consumer create request down below and also should be updated as expected on reconnects. feel free to open a PR when you're ready and have time. thank you 💯

For future reference and history, setting the sequence in the ctor was not an option because of the way watchers gets re-created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@niklasfp niklasfp

Labels
good first issue Good for newcomers help wanted Extra attention is needed new-feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add ResumeAtRevision support to KV Watcher niklasfp/nats.net.v2
2 participants
@mtmk @niklasfp