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

A dispatcher process for {crew} workers #107

Closed
wlandau opened this issue Aug 21, 2023 · 6 comments
Closed

A dispatcher process for {crew} workers #107

wlandau opened this issue Aug 21, 2023 · 6 comments
Assignees

Comments

@wlandau
Copy link
Owner

wlandau commented Aug 21, 2023

mirai has an active dispatcher process for tasks. I am thinking I could implement a different one for crew workers in order to free up control in the parent process. @shikokuchuo, this may be easier than even the light-touch fused dispatcher we talked about before.

Worker dispatch in crew requires knowledge of how many workers are active (using mirai::status()) and the number of unresolved tasks. I wonder, is it possible to call mirai::status() from a different process than the one that called mirai::daemons()? And what would be the best way to count unresolved tasks from a crew worker dispatcher process?

@wlandau wlandau self-assigned this Aug 21, 2023
@shikokuchuo
Copy link
Contributor

Sorry I've not had the bandwidth to respond to your previous questions yet. I will get to these.

@shikokuchuo
Copy link
Contributor

Once I have confirmation that shikokuchuo/mirai#69 is fixed, I'll put something together that will show you how easy it is (hopefully). I do want to prioritise fixing any remaining bugs before the possibility of introducing new ones with further development.

@shikokuchuo
Copy link
Contributor

Using another dispatcher process is fine, but syncing the variables does present a problem. Previously when we were just using 'bus' sockets, that may have been more plausible as they are multicast. However, we have seen that only req/rep will do.

Instead it will be much easier if it works to run your functions on dispatcher itself, to be evaluated directly in that context. If you need another socket to send information back to host then you can still prototype that using the current limited interface, and we can add that as a setup step for efficiency later if needed.

Using the same process also solves your other issue #108 about how to sync with dispatcher events.

I have pushed a 'dispatcher' branch to mirai which implements a simple interface for dispatcher extensions.

Included is the option of supplying either a function or call. I suspect a function will be easier, and avoids the additional frame through an eval() call. Unless something complicated is required, in which case additional arguments to eval will probably also need to be exposed.

I have tested very briefly by using cat() to write dispatcher variables to a file (as stdout is redirected), and it seems to work as expected.

However let me know if structurally something is missing.

Also just a note that this feature is definitely for after the next CRAN release of mirai, so you have plenty of time to explore.

@wlandau
Copy link
Owner Author

wlandau commented Aug 24, 2023

That's fantastic, @shikokuchuo! I can't wait to try this! As soon as I emerge from this latest development sprint on cloud storage with targets.

@wlandau
Copy link
Owner Author

wlandau commented Oct 2, 2023

Given shikokuchuo/mirai#78 (reply in thread), I am not so sure the extended R dispatcher would work for crew because of the strange kinds of launch failures that crew needs to worry about. I am not sure crew could get away from a polling approach to workers (but fortunately the polling is much gentler than it would have to be for tasks).

The obvious alternatives range from challenging to infeasible:

  1. If I were to create a separate dispatcher process for just the launcher, it would need saisei() and nextget("cv"), and I am not sure if mirai can support two copies of the same host at the same time. (In the past while testing moot edge cases, I have seen NNG-level errors that forking in NNG is unsafe.)
  2. Remote controllers #105 seems messy and brittle, with a lot of extra work to manage communication to the remote controller. It would also add another network hop for data, which would be inefficient.

Fortunately though, nextget("cv") will make polling-based crew a lot nicer: in particular, it will remove the need for throttling.

@wlandau
Copy link
Owner Author

wlandau commented Oct 2, 2023

If I really need to, I will come back to #105 for this. But it is a huge undertaking to implement, especially compared to how simple crew currently is, and I am not sure we strictly need it.

@wlandau wlandau closed this as completed Oct 2, 2023
@wlandau wlandau reopened this Oct 2, 2023
@wlandau wlandau closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2023
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