shikokuchuo / mirai

mirai - Minimalist Async Evaluation Framework for R
https://shikokuchuo.net/mirai/
GNU General Public License v3.0
193 stars 10 forks source link

Persistence across R sessions, persistent dispatcher #81

Closed krlmlr closed 1 year ago

krlmlr commented 1 year ago

Currently, running daemons(4) starts four R processes. These are terminated when the host R process finishes execution.

I would like those processes to survive the end of the host process, and to be able to "reconnect" from a fresh R session. Is this something that mirai could support eventually?

Use case: repeated execution of short-lived tasks from different R processes, e.g., runs of testthat::test_file() or styler::style_file() .

shikokuchuo commented 1 year ago

Persistence is the default out of the box really so I'm sure this is possible.

In fact we devoted considerable time to ensuring that everything does exit cleanly under all circumstances! In early versions we had to contend with dispatcher hanging when daemons crashed etc.

Currently the behaviour is very safe, as long as the parent connection drops, both dispatcher and daemons will exit. Removing these safeguards then requires creative way to kill them - likely different in each case as nothing actually gets evaluated on dispatcher (it is straight pass-through).

Leave this one with me and I'll have a think about the best way to implement.

wlandau commented 1 year ago

Currently the behaviour is very safe, as long as the parent connection drops, both dispatcher and daemons will exit.

Yes, and crew will always rely on these automatic exits. I really appreciate how robust you have made this, and it would be great if it continued to be the default behavior.

shikokuchuo commented 1 year ago

@wlandau yes, rest assured this will continue to be the default. My comment was just that if persistence were to become an option, then there would have to be a robust way of terminating daemons/dispatcher in that particular case.

krlmlr commented 1 year ago

I fully agree that the persistent dispatcher is something that one would have to opt in to.

shikokuchuo commented 1 year ago

Persistence is now implemented for daemons.

I've re-purposed the existing 'asyncdial' argument into 'autoexit', as one implies the other.

As these now survive socket disconnections, I've brought back a bit of magic that was used previously to send exit signals. This is done by a daemons(NULL) command rather than daemons(0).

Sample code below show how easy it is to disconnect / reconnect, and how daemons(NULL) terminates the daemons. This works the same way with or without dispatcher.

library(mirai)
daemons(4, autoexit = FALSE)
Sys.sleep(1)
status()
url <- nextget("urls")
daemons(0)

daemons(url = url)
Sys.sleep(1)
status()
daemons(NULL)

daemons(url = url)
Sys.sleep(1)
status()
daemons(0)

If this is sufficient for your use cases, I would like to keep the persistence settings at this level rather than at dispatcher. The code changes here are minimal and clean. Whereas initially dispatcher was created as a 'module', I am increasingly thinking of it as an extension of 'host'. Indeed the current roadmap is for there to be a threaded version down the line, rather than requiring a full background process.

In terms of interface, this is potentially fluid until the next CRAN release, and I'm open to suggestions.

wlandau commented 1 year ago

Will any changes need to happen in crew? crew needs its workers to exit immediately if there is no host running on the other end, and I have been using asyncdial = FALSE for this. Does autoexit = TRUE cover this? Also, should I start replacing daemons(n = 0L) with daemons(n = NULL)?

shikokuchuo commented 1 year ago

The default is autoexit = TRUE so it won't affect any behaviour in crew (it implies asyncdial = FALSE). You can take any 'asyncdial' references out though. It also shouldn't be exposed as a user option in crew as I don't think it would ever be appropriate there.

You don't need to change the daemons command as daemons(NULL) just sends an exit signal in addition. It won't do any harm, but is unnecessary.

shikokuchuo commented 1 year ago

Incorporated in mirai 0.11.1 release. Thanks again for the feature suggestion. Any further comments please feel free to raise another issue.

krlmlr commented 1 year ago

Thanks, great! I'll get back to you when I next spend time in this space.