don't automatically dispatch to a busy session #80
Conversation
| stderr = "|" | ||
| ) | ||
| withr::defer(busy$kill()) | ||
| Sys.sleep(1) |
There was a problem hiding this comment.
I'm not into these two Sys.sleeps, but it's not clear to me that we can infer these processes' status by calling methods on them. We need to give the sessions long enough to be ready to respond in first_responsive_session().
| { | ||
| # the dial in list_r_sessions itself doesn’t prove the session is | ||
| # responsive—require the session to actually reply and then postprocess | ||
| sessions <- list_r_sessions() |
There was a problem hiding this comment.
My sense is that we need to run basically all of list_r_sessions() in order to actually retrieve this information we want. In theory, we're waiting on all pipes in sort(as.character(nanonext::collect_aio_(res))) to respond and we could return earlier in the first_responsive_session() case—returning when the lowest-indexed session responds regardless of the status of the others—but I don't have a strong opinion on whether this is worth the additional complexity.
shikokuchuo
left a comment
shikokuchuo
left a comment
There was a problem hiding this comment.
I left a comment on the issue, but my sense is that if there's more than one session, we don't dispatch to a (random) one.
In that case, we'd have to try dialling the sockets (first part of list_r_sessions()) to see if more than one succeeds (dial(fail = "none") returns 0L on success and a positive integer on failure), but we wouldn't need to request/wait for any responses).
|
Got it, thanks for your thoughts! I think that's reasonable—not worth the change if it's mostly geared to rare uses. |
Closes #63.