Stop all actors at once instead of stopping/joining threads sequentially #85
Conversation
Change-Id: Iaf8c28a18cf041f162f98e769f6b42f344285811
| // Stopping actors in the reverse order in which they were spawned. | ||
| // We send the Stop control message to all actors first so they can | ||
| // all shut down in parallel, so actors will be in the process of | ||
| // stopping when we join the threads below. |
There was a problem hiding this comment.
tiny nit present simple, imperative form of comments
| // Stopping actors in the reverse order in which they were spawned. | |
| // We send the Stop control message to all actors first so they can | |
| // all shut down in parallel, so actors will be in the process of | |
| // stopping when we join the threads below. | |
| // Stop actors in the reverse order in which they were spawned. | |
| // Send the Stop control message to all actors first so they can | |
| // all shut down in parallel, so actors will be in the process of | |
| // stopping when we join the threads below. |
| let actor_name = entry.name(); | ||
|
|
||
| if let Err(e) = entry.control_addr().send(Control::Stop) { | ||
| warn!("control channel is closed: {} ({})", actor_name, e); |
There was a problem hiding this comment.
Preexisting, but It'd be nice to mention a context where this is happening and the consequence (to please @strohel). For instance:
| warn!("control channel is closed: {} ({})", actor_name, e); | |
| warn!("Couldn't send Control::Stop to {} to shut it down. Proceeding to shut down remaining actors: {}", actor_name, e); |
There was a problem hiding this comment.
The new wording is confusing to me, the error message should come after the first sentence, not the second one. I'd just remove the "Proceeding" sentence.
There was a problem hiding this comment.
My take:
| warn!("control channel is closed: {} ({})", actor_name, e); | |
| warn!("Couldn't send Control::Stop to {} to shut it down: {}. Ignoring and proceeding.", actor_name, e); |
| let actor_name = entry.name(); | ||
|
|
||
| if let Err(e) = entry.control_addr().send(Control::Stop) { | ||
| warn!("control channel is closed: {} ({})", actor_name, e); |
There was a problem hiding this comment.
The new wording is confusing to me, the error message should come after the first sentence, not the second one. I'd just remove the "Proceeding" sentence.
There was a problem hiding this comment.
IIUC this is blocked until at least the panics in https://github.com/tonarino/portal/pull/3579 are fixed, so marking as request changes until then.
Closes #12