-
Notifications
You must be signed in to change notification settings - Fork 35
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
Eternal watch should be robust when blocks are slow #64
base: master
Are you sure you want to change the base?
Conversation
This is dictated by the existing implementation of `watch`.
Before this method can become more sophisticated, it should be tested a little. In these two specs, we exercise the basic behaviour (watching for multiple changes to a key, instead of returning after one) and that recursive keys can be watched. The `eternal_watch` method uses `loop`, which would normally be hard to test, but we can control it in the specs by breaking out of the loop using an exception once we know we've exercised everything we care about. I've added some code to help manage that in a simple way.
It's good practice for etcd watchers to track what the latest update they received was, and then resume watching immediately after that update, to ensure that no changes might've been missed between the return of the first watch and the start of the second; i.e. when actual work on the changed key/value is being done. At the moment, `eternal_watch` doesn't do that, so if the yielded block doesn't return before the next key update, that update will be lost. Subsequent commits should fix this test and make the behaviour of `eternal_watch` more robust.
If the watched key changes while the given block executes, we will now still pick up the change.
@lazyatom this library is not actively maintained. Let me know if you would be interested in taking over maintainership |
It would probably be more pragmatic to work with @davissp14 to merge in his v3-compatible codebase here, and then use that as the basis for future development. I'd be happy to help facilitate that (including handing off ownership from you, if you like), if it's something you are both interested in? |
My goal is to actually get these projects under the CoreOS account. I think this will be a better long term solution for both Etcd and for dealing with the inactive maintainer issue.
I personally don't like the idea of munging the v2 and v3 codebases as they are and should be thought of as completely different databases. I think/hope that the duel support going on in the v3 binaries is a short-term solution to account for the immature v3 client driver ecosystem. Relevant Issue: |
My thinking was to branch this code as What I'd most like to see is that the "official" gem (either eventually under CoreOS' account, or semi-official if not) be able to use the If @ranjib is happy to transfer/share gem ownership then we could achieve this without the codebases living in the same repository, but it might be confusing for users reporting issues if the codebases don't live together (albeit separated via git history and branches). |
In order to ensure that we don't miss any key changes while we are doing work, the
eternal_watch
method should track themodifiedIndex
attribute of the previous response and watch again using the next index value.This pull request also adds specs for the other basic behaviours of
eternal_watch
, and fixes a minor but when it is called with only one argument.