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

unwatch kvs values and gracefully return from a flux_reactor based application #75

Closed
grondo opened this issue Oct 19, 2014 · 4 comments

Comments

@grondo
Copy link
Contributor

grondo commented Oct 19, 2014

kvs_watch() is cumbersome to use with the flux reactor because there is no way to
deregister the watch function, and no way to gracefully interrupt the reactor itself
(kvs_watch callbacks have no return value to pass up to reactor core)

If an application is using the flux_reactor interface, and wants to watch even one kvs
value or directory, it cannot use the nice idiom of deregistering all reactor handlers
indicating there is nothing left to do, because the kvs handlers do not support this
functionality.

Also since kvs handlers don't support passing error values up to flux reactor, we
can't even force an exit from the reactor by returning -1. The only solution is to
interrupt the reactor with a signal somehow, or set up a message to ourselves
that forces exit from the reactor by returning -1. (And I'm not sure the second approach
is currently possible either)

One solution is to support kvs_unwatch(), another solution is support the use of
kvs_watch_once() style single-action trigger. The user would be required to reset
the watch before returning to the reactor. The second approach sounds more
easily implemented, but probably has a race condition.

@garlick
Copy link
Member

garlick commented Oct 19, 2014

Yeah the watch callbacks should return -1 to stop the reactor like other reactor callbacks. A workaround for now is to call flux_reactor_stop() from within the callback.

Matching kvs_watch_add() / kvs_watch_remove() calls also make sense, and I don't know why I didn't do it that way to begin with.

@grondo
Copy link
Contributor Author

grondo commented Oct 19, 2014

Ah, thanks -- I forgot about flux_reactor_stop() -- that will work for my use case

garlick added a commit to garlick/flux-core that referenced this issue Dec 19, 2014
Add 'matchtag' argument to flux_response_recvmsg() and flux_json_request().
Change flux_json_rpc() to internally allocate/free a unique matchtag.

Legacy request functions set 'matchtag' to zero in
flux_response_recvmsg() and flux_json_request() to get the legacy
behavior.

Special KVS note:  kvs_watch() was changed to allocate a matchtag and hold
it for the life of the handle since a kvs_watch() request has multiple
replies and persists until the handle is destroyed.  The design of
kvs_watch() will be revisited - see issue flux-framework#75.
grondo pushed a commit to grondo/flux-core that referenced this issue Jan 21, 2015
Add 'matchtag' argument to flux_response_recvmsg() and flux_json_request().
Change flux_json_rpc() to internally allocate/free a unique matchtag.

Legacy request functions set 'matchtag' to zero in
flux_response_recvmsg() and flux_json_request() to get the legacy
behavior.

Special KVS note:  kvs_watch() was changed to allocate a matchtag and hold
it for the life of the handle since a kvs_watch() request has multiple
replies and persists until the handle is destroyed.  The design of
kvs_watch() will be revisited - see issue flux-framework#75.
@garlick
Copy link
Member

garlick commented Feb 26, 2015

Revisiting this as I'd like to revamp some of the reactor interfaces.

First I should note that the reactor will not return when there are no callbacks registered. There are two problems that need to be solved here:

  • internal handle sockets should get registered with the underlying reactor "on demand" rather than by default. Currently they are registered at reactor creation and never unregistered, so the reactor never terminates due to lack of callbacks.
  • Although we now have kvs_unwatch(), the KVS code unconditionally registers an internal callback for kvs.watch in its initialization and never unregisters it. That also should be on demand.

Second, what do you think about a void type for reactor callbacks like libev's and then relying exclusively on flux_reactor_stop() or lack of handlers registered to terminate the reactor, which would return 0 except when it encounters an internal error? I think it's somewhat unintuitive that reactor callbacks returning -1 will terminate the reactor...

Looking back I really made some "noob" decisions in the reactor code!

@garlick
Copy link
Member

garlick commented Mar 18, 2015

Since PR #140 we have kvs_unwatch().

Also in #147 and related changes, reactor will now exit once all handlers are unregistered.

We will want to change the kvs watch callback prototype when we overhaul the reactor interface. This is being discussed in #124

I think we can close this one.

@garlick garlick closed this as completed Mar 18, 2015
grondo added a commit to grondo/flux-core that referenced this issue Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants