Skip to content

Add ResumeAtRevision support to KV Watcher#491

Merged
mtmk merged 14 commits into
nats-io:mainfrom
niklasfp:478-watch-resume-at-revision
Jun 6, 2024
Merged

Add ResumeAtRevision support to KV Watcher#491
mtmk merged 14 commits into
nats-io:mainfrom
niklasfp:478-watch-resume-at-revision

Conversation

@niklasfp

@niklasfp niklasfp commented Apr 24, 2024

Copy link
Copy Markdown
Contributor

Resolves #478

@niklasfp

niklasfp commented Apr 24, 2024

Copy link
Copy Markdown
Contributor Author

@mtmk I'm seeing some test flappers in Watcher_reconnect_with_history and Watcher_timeout_reconnect locally (also without my changes)

Machine: Mac M2 Pro arm64

@mtmk mtmk self-assigned this Apr 24, 2024
@mtmk

mtmk commented Apr 24, 2024

Copy link
Copy Markdown
Member

@mtmk I'm seeing some test flappers in Watcher_reconnect_with_history and Watcher_timeout_reconnect locally (also without my changes)

Machine: Mac M2 Pro arm64

I'm not surprised. they're most stable on Linux at the moment. We also have #464 to deal with Windows flappers. we might open one for Mac too. I'm guessing it's mostly around managing the server process and sockets, but I might be wrong.

mtmk
mtmk reviewed Apr 24, 2024
View reviewed changes
Comment thread src/NATS.Client.KeyValueStore/Internal/NatsKVWatcher.cs Outdated
Comment thread src/NATS.Client.KeyValueStore/NatsKVOpts.cs Outdated
mtmk
mtmk approved these changes Apr 25, 2024
View reviewed changes

@mtmk mtmk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM thank you @niklasfp 👍

@mtmk
mtmk requested a review from caleblloyd April 25, 2024 15:39
caleblloyd
caleblloyd reviewed Apr 25, 2024
View reviewed changes
Comment thread src/NATS.Client.KeyValueStore/NatsKVOpts.cs
@niklasfp

niklasfp commented May 2, 2024

Copy link
Copy Markdown
Contributor Author

@mtmk I have not forgotten about this one, I'll get to it this weekend

@mtmk

mtmk commented Jun 4, 2024

Copy link
Copy Markdown
Member

@niklasfp, @caleblloyd just made the last bit of validation changes. Please feel free to make suggestion, especially for docs. Let me know if y'all happy with it then I can merge it ;)

@niklasfp

niklasfp commented Jun 5, 2024

Copy link
Copy Markdown
Contributor Author

@niklasfp, @caleblloyd just made the last bit of validation changes. Please feel free to make suggestion, especially for docs. Let me know if y'all happy with it then I can merge it ;)

Omg thank u, and so sorry for not following up on this, life happened 🥲 I'll take a look today.

@mtmk

mtmk commented Jun 5, 2024

Copy link
Copy Markdown
Member

@niklasfp, @caleblloyd just made the last bit of validation changes. Please feel free to make suggestion, especially for docs. Let me know if y'all happy with it then I can merge it ;)

Omg thank u, and so sorry for not following up on this, life happened 🥲 I'll take a look today.

thanks @niklasfp very kind of you 🙏 honestly, we really appreciate your contribution, you've done the hard work and I'm always happy to tie the loose ends when you don't have time 😅

niklasfp
niklasfp commented Jun 5, 2024
View reviewed changes
Comment thread src/NATS.Client.KeyValueStore/NatsKVOpts.cs Outdated
caleblloyd
caleblloyd reviewed Jun 5, 2024
View reviewed changes
Comment thread src/NATS.Client.KeyValueStore/INatsKVStore.cs Outdated
Comment thread src/NATS.Client.KeyValueStore/INatsKVStore.cs Outdated
Comment thread src/NATS.Client.KeyValueStore/INatsKVStore.cs Outdated
Comment thread src/NATS.Client.KeyValueStore/INatsKVStore.cs Outdated
mtmk and others added 6 commits June 6, 2024 01:00
@mtmk @caleblloyd
Update src/NATS.Client.KeyValueStore/INatsKVStore.cs
6a48971 
Co-authored-by: Caleb Lloyd <2414837+caleblloyd@users.noreply.github.com>
@mtmk @caleblloyd
Update src/NATS.Client.KeyValueStore/INatsKVStore.cs
c8303a0 
Co-authored-by: Caleb Lloyd <2414837+caleblloyd@users.noreply.github.com>
@mtmk @caleblloyd
Update src/NATS.Client.KeyValueStore/INatsKVStore.cs
b1e04a9 
Co-authored-by: Caleb Lloyd <2414837+caleblloyd@users.noreply.github.com>
@mtmk @niklasfp
Update src/NATS.Client.KeyValueStore/NatsKVOpts.cs
7ed9e88 
Co-authored-by: Niklas Petersen <7766483+niklasfp@users.noreply.github.com>
@mtmk
Remove exception comment from PurgeDeletesAsync method
a424ace
@mtmk
Typo
2a72374
caleblloyd
caleblloyd approved these changes Jun 6, 2024
View reviewed changes

@caleblloyd caleblloyd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@mtmk
mtmk merged commit fc9e2ca into nats-io:main Jun 6, 2024
mtmk added a commit that referenced this pull request Jun 11, 2024
@mtmk
Release 2.2.3
4432609 
* Add ResumeAtRevision support to KV Watcher (#491)
* prefer sub channel ReadAllAsync and rm SubAnchor (#507)
* Keep sub alive when reading channel (#506)
* Add overload for empty requests (#502)
@mtmk mtmk mentioned this pull request Jun 11, 2024
Merged
mtmk added a commit that referenced this pull request Jun 11, 2024
@mtmk
Release 2.2.3 (#515)
c7229c9 
* Add ResumeAtRevision support to KV Watcher (#491)
* prefer sub channel ReadAllAsync and rm SubAnchor (#507)
* Keep sub alive when reading channel (#506)
* Add overload for empty requests (#502)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mtmk mtmk mtmk approved these changes
@caleblloyd caleblloyd caleblloyd approved these changes

Assignees

@mtmk mtmk

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

KV ResumeAtRevision option for watches

3 participants

@niklasfp @mtmk @caleblloyd