activator: non-blocking #1833
activator: non-blocking #1833
Conversation
bgm-malbeclabs
left a comment
bgm-malbeclabs
left a comment
There was a problem hiding this comment.
Feels like we need to show more evidence that this fixes the issue since the tests don't seem to have materially changed. Also why move to non-blocking? I have a pretty good idea but it's not clear why the changes from the description.
The blocking tasks in the existing activator prevent it from shutting down, so we have to use SIGKILL. This has the consequence of when things fail catastrophically, it winds up blocking on shutdown instead of closing out and being restarted by systemd. |
|
Updated the summary of the PR and also included the GH issue |
| let pubkey = keyed_account.pubkey.clone(); | ||
| match rpckeyedaccount_decode(keyed_account) { | ||
| Ok(Some((pubkey, account))) => { | ||
| tx.send((pubkey, account)).await.unwrap(); |
There was a problem hiding this comment.
You could do something like:
if let Err(e) = tx.send(...).await {
print_some_error_or_whatever
break;
}The unwrap would crash the entire activator process otherwise
There was a problem hiding this comment.
Good catch. I never intended to leave this in, I was just trying to get to a compilation milestone.
| error!("Activator thread exited unexpectedly with reason: {err:?}"); | ||
| } | ||
| _ = websocket_task(Arc::clone(&client), tx.clone(), shutdown.clone()) => { | ||
| info!("Websocket task finished, stopping activator..."); |
There was a problem hiding this comment.
Do we not have to shutdown.store(true, Ordering::Relaxed) here as well?
Summary of Changes
Closes #1215
Testing Verification