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_watch is inefficient and doesn't scale well #803

Closed
garlick opened this issue Sep 10, 2016 · 2 comments
Closed

kvs_watch is inefficient and doesn't scale well #803

garlick opened this issue Sep 10, 2016 · 2 comments

Comments

@garlick
Copy link
Member

garlick commented Sep 10, 2016

The current kvs.watch implementation is inefficient. On receipt of every kvs.setroot message, the kvs module must do an internal "get" on all watcheds keys, compare the old value to the new, then if it has changed, issue a response. It was noted in #802 our current stdio scheme requires flux-wreckrun to register two watchers per task, which creates a lot of work for the rank 0 KVS when a large job is launched.

A possible enhancement is to add a list of changed keys to the kvs.setroot event. The main purpose of the setroot event is to announce a new root reference after a commit. Since the commit logic starts with a list of keys, it would be fairly simple to include that same list in this event. Each kvs module instance would then compare its list of watched keys against the list that changed (and their parent directories which implicitly have changed), and only perform gets on the keys that were included in the commit.

@garlick
Copy link
Member Author

garlick commented Sep 10, 2016

Alternatively, the kvs.watch code could be eliminated from the KVS module, and the client watch implementation could subscribe to the kvs.setroot event internally. To be transactional, there would need to be a way to look up a key against the specific root blobref from the kvs.setroot event, which has been discussed before, e.g. #64.

garlick added a commit to garlick/flux-core that referenced this issue Sep 14, 2016
Include an array of (de-duplicated) keys that were modified
by the commit in the kvs.setroot event.  This prepares the
way for more efficient kvs_watch() handling as described in flux-framework#803.

Update test for kvs.setroot codec.
garlick added a commit to garlick/flux-core that referenced this issue Sep 15, 2016
Include an array of (de-duplicated) keys that were modified
by the commit in the kvs.setroot event.  This prepares the
way for more efficient kvs_watch() handling as described in flux-framework#803.

Update test for kvs.setroot codec.
garlick added a commit to garlick/flux-core that referenced this issue Sep 19, 2016
Include an array of (de-duplicated) keys that were modified
by the commit in the kvs.setroot event.  This prepares the
way for more efficient kvs_watch() handling as described in flux-framework#803.

Update test for kvs.setroot codec.
@chu11
Copy link
Member

chu11 commented Jan 31, 2019

closing this, this was dealt with via the kvs-watch module, initially merged in #1643

@chu11 chu11 closed this as completed Jan 31, 2019
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