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.

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

kvs: fold watch into flux_kvs_lookup() interface with new flag #1556

Closed
garlick opened this issue Jun 28, 2018 · 3 comments
Closed

kvs: fold watch into flux_kvs_lookup() interface with new flag #1556

garlick opened this issue Jun 28, 2018 · 3 comments

Comments

@garlick
Copy link
Member

garlick commented Jun 28, 2018

The KVS watch API has still not been reworked to fit with the rest of the KVS API.

Here is one proposal. Since the bindings and a lot of tests depend on the current semantics of kvs_watch, develop a new interface based on adding a watch flag to flux_kvs_lookup(), then move bindings and tests to it gradually.

Since we now have flux_future_reset() (#1503) and have defined semantics for "streaming" responses to RPCs in RFC 6, it seems appropriate to have a lookup request that can receive multiple responses.

As suggested in #803, we could dramatically reduce the cost of a KVS watch by listing the modified keys (from txn ops) in the kvs.setroot event. If a watched key appears on a setroot event, this can trigger a lookup relative to the root blobref in the event, and a lookup response for its value.

Doing things this way would have the following effects on watch semantics:

  • switch to futures (no purpose-built callbacks)
  • a lookup on a nonexistent key fails (might need to add a another flag to block lookup until key exists)
  • a lookup with FLUX_KVS_READDIR fails on a non-directory
  • a lookup without FLUX_KVS_READDIR fails on a directory
  • a lookup response is generated if identical value written to key (maybe needs a workaround?)
  • a lookup stream terminates when key is removed
  • some changes that don't mention the key might not be captured, for example directory removal
@garlick
Copy link
Member Author

garlick commented Jun 29, 2018

Thinking about this, we could maybe implement this in a new kvs-watch module to avoid adding complexity (and more stuff to do in the single thread) to the kvs module.

So if flux_kvs_lookup() includes FLUX_KVS_WATCH flag, it would redirect to kvs-watcher.lookup.

The new module would monitor the kvs.setroot event and respond to lookup requests with multiple responses. It could implement a hash by KVS key of watchers. When KVS key is mentioned in kvs.setroot message, it issues a kvs.lookup to fetch the new value. It could do that once for multiple watchers of the same key. It must ensure that responses are kept in commit sequence order, since kvs.lookup can respond out of order.

Eventually we would remove the old KVS watch code from the kvs module.

@grondo grondo added this to the release 0.10.0 milestone Jul 10, 2018
@garlick
Copy link
Member Author

garlick commented Aug 24, 2018

Just an update here: The kvs-watch module was implemented in part in PR #1622, although not with a fully functional "key watch". Instead, it provides support for a multi-response flux_kvs_getroot() call, with FLUX_KVS_WATCH flag. It also added an array of changed keys to the kvs.setroot event.

What's left to do is to implement the ability a kvs-watch.lookup method and modify the lookup API to allow the FLUX_KVS_WATCH flag to redirect a lookup from kvs to kvs-watch.

@garlick
Copy link
Member Author

garlick commented Dec 12, 2018

We're almost there. Closing this and will track remaining work in individual issues.

@garlick garlick closed this as completed Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants