[ADDED] StartRevision option to KV watcher

[shadow3x3x3]

fix #1488

[derekcollison]

There is a DeliverNew delivery policy, which combined with a filter subject might do the trick.

[ripienaar]

@Jarema has some summary of our internal discussions here that we need to capture when evaluating this PR.

[Jarema]

Hey!

Thanks for the proposal and effort to make a PR!

As this feature may look like a no brainer, it can introduce a potential footgun to the users:
If you start Watcher from some sequence, you will entirely skip keys that were not updated since that sequence.
We were discussing if this should go in at all, but there is a valid use case:
To resume watching the keys after some errors/crash, as user might now want to start the whole process from the start.
This might also apply for some long running process even if it's scheduled downtime. Right now there is no way to start from where the work on watcher was left off.

So, to avoid making a footgun for people who will see it and use it without proper consideration, let's rename StartRevision to ResumeFromSeq clearly stating the intent of this option.

[shadow3x3x3]

So, to avoid making a footgun for people who will see it and use it without proper consideration, let's rename StartRevision to ResumeFromSeq clearly stating the intent of this option.

Yes, I completely agree.

This might also apply for some long running process even if it's scheduled downtime. Right now there is no way to start from where the work on watcher was left off.

At the moment, we are encountering a challenge where, in an unreliable network, multiple clients reconnecting can only commence watching from the start. This leads to some resource wastage, particularly when the bucket has a lengthy history.

Appreciate your attention to this feature. I've updated the relevant names. Kindly verify when you have a moment. Thanks!

[ripienaar]

At the moment, we are encountering a challenge where, in an unreliable network, multiple clients reconnecting can only commence watching from the start. This leads to some resource wastage, particularly when the bucket has a lengthy history.

A watch from a reconnecting client should continue where it left off, is that not the case?

If that's what you are trying to solve then there's a bug to fix because they should resume.

[shadow3x3x3]

A watch from a reconnecting client should continue where it left off, is that not the case?

Thank you for your reminder.

It seems I'm closing the existing watch every time I reconnect and then watching again upon reconnection.

However, it seems unnecessary; I just need to keep watching to retrieve the data again. 😅

Then, should we proceed with this PR?

[ripienaar]

I think we need the resume functionality, but this is good it means we can strongly state this as resume and your main problem is solved. Good outcome all round I think.

@Jarema wdyt?

[Jarema]

Yes, I agree. This is good!

Will do a proper review of the code soon.

[Jarema]

This looks good, just one change request.

Sorry for getting back and forth with naming, but your argument named for new option revision gave us the hint that the whole method should be around reviosion, not sequence. After all this is KV abstraction over stream :).

[Jarema]

As this is KV abstraction over Stream, we should probably use KV naming convetions.

This should become ResumeFromRevision.

[shadow3x3x3]

Related Sequence names have been updated to Revision.

Please take a look, thanks!

[Jarema]

LGTM!

Thanks for your contribution!

[piotrpio]

LGTM!