Updates to KV Watcher. #900
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. If the underlying stream was very busy pending might not go to zero, so snapshot and mark initial state done when received >= init pending or delta == 0 2. When a watcher was cancelled with a context we would not signal to allow and range w.Updates() calls to exit. 3. General flow control checks were timing sensitive after the initial checks, needed to be 2x HB interval always, not just beginning. Signed-off-by: Derek Collison <derek@nats.io>
kozlovic
reviewed
Feb 7, 2022
Signed-off-by: Derek Collison <derek@nats.io>
derekcollison
force-pushed
the
watcher_updates
branch
from
3987a74
to
e6ee279
Compare
kozlovic
requested changes
Feb 8, 2022
nats.go
Outdated
| @@ -3944,6 +3946,13 @@ func (nc *Conn) removeSub(s *Subscription) { | |||
| if s.pCond != nil { | |||
| s.pCond.Broadcast() | |||
| } | |||
|
|
|||
| // Check for watchers. | |||
| if jsi != nil && jsi.w != nil { | |||
There was a problem hiding this comment.
This is not enough to guarantee that waitForMsgs() is not invoking the message callback at this time, or about to do it.
There was a problem hiding this comment.
Yea I am reworking this as we speak since the large KINE test failed on send to closed chan.
There was a problem hiding this comment.
ok I reworked it, please take another look.
Signed-off-by: Derek Collison <derek@nats.io>
11 tasks
kozlovic
added a commit
that referenced
this pull request
Feb 19, 2022
This is related to #900 that introduced the change of discard policy if connecting to a server v2.7.2+. The CreateKeyValue() API calls AddStream(). If the stream already exists and the configuration is identical, the call succeeds (idempotent). However, since we now try to set the discard policy to "new" when connecting to a server v2.7.2+, the call will fail if the KV store already existed (CreateKeyValue() was called with a v2.7.1 server and stream was created with DiscardOld). The approach here is that if AddStream() fails with an "already in use" error, then we will lookup the stream info, and if we detect that the configuration is same (except for the discard policy), then we call UpdateStream() with the new discard policy. The problematic part is that the client side configuration does not set some of the fields (or their value is 0), but then the server sets either some defaults (like Duplicates set to 2min) or replaces 0 to -1. So the info we get back and the config we have need to be tweaked before being compared. This is really hacky and prone to break if server were to change some defaults. Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
derekcollison
pushed a commit
that referenced
this pull request
Mar 16, 2022
This is related to #900 that introduced the change of discard policy if connecting to a server v2.7.2+. The CreateKeyValue() API calls AddStream(). If the stream already exists and the configuration is identical, the call succeeds (idempotent). However, since we now try to set the discard policy to "new" when connecting to a server v2.7.2+, the call will fail if the KV store already existed (CreateKeyValue() was called with a v2.7.1 server and stream was created with DiscardOld). The approach here is that if AddStream() fails with an "already in use" error, then we will lookup the stream info, and if we detect that the configuration is same (except for the discard policy), then we call UpdateStream() with the new discard policy. The problematic part is that the client side configuration does not set some of the fields (or their value is 0), but then the server sets either some defaults (like Duplicates set to 2min) or replaces 0 to -1. So the info we get back and the config we have need to be tweaked before being compared. This is really hacky and prone to break if server were to change some defaults. Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Derek Collison derek@nats.io