-
Notifications
You must be signed in to change notification settings - Fork 326
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
Exit when worker detects expired client, remove event decoding trace #1023
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left one question/suggestion, which is not critical. But otherwise looks good.
relayer/src/worker/client.rs
Outdated
@@ -56,7 +56,7 @@ impl ClientWorker { | |||
// Run client refresh, exit only if expired or frozen | |||
if let Err(e @ ForeignClientError::ExpiredOrFrozen(..)) = client.refresh() { | |||
error!("failed to refresh client '{}': {}", client, e); | |||
continue; | |||
return Err(Box::new(e)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a thought: Would it make more sense to return with an Ok(())
here? I think it might be less confusing for the caller if we distinguish between returning with an Error
(in which case the worker could retry for specific error), and returning with Ok
(in this case the worker finished its job).
return Err(Box::new(e)); | |
return Ok(()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think return Ok
is fine, as we already print the error before exiting (though perhaps it should be a warning then?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…nformalsystems#1023) * Exit when worker detects expired client, remove event decoding trace * Provide worker's name when worker aborts * Exit client worker without error when client is expired or frozen * Remove unused import * Changelog Co-authored-by: Romain Ruetschi <romain@informal.systems> Co-authored-by: Adi Seredinschi <adi@informal.systems>
Closes: #1022
Description
While debugging an issue on akashnet, noticed that client workers don't exit when states are expired.
Also the trace for undecoded events is still super annoying so let's remove it.
For contributor use:
docs/
) and code comments.Files changed
in the Github PR explorer.