Skip to content
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

Shut down Worker/Processor if initialization failed #7575

Open
SanjoDeundiak opened this issue Feb 12, 2024 · 4 comments
Open

Shut down Worker/Processor if initialization failed #7575

SanjoDeundiak opened this issue Feb 12, 2024 · 4 comments

Comments

@SanjoDeundiak
Copy link
Member

Observed behavior

Currently, if Worker or Processor return an error from its initialize function, it's not shut down.

Steps to reproduce

Create a dummy Worker that always fails during initialization.

Desired behavior

Worker/Processor should be stopped

Ockam Version

0.116.0

@pradovic
Copy link
Contributor

Hi, may I take this one?

@SanjoDeundiak
Copy link
Member Author

@pradovic sure, go ahead

@pradovic
Copy link
Contributor

pradovic commented Feb 26, 2024

@SanjoDeundiak Thanks! Finally got the time to take a look.
If I understand correctly the error in processor or worker initialize function error is just logged by WorkerRelay.

#[cfg_attr(not(feature = "std"), allow(unused_mut))]
    #[cfg_attr(not(feature = "std"), allow(unused_variables))]
    async fn run(mut self, mut ctrl_rx: SmallReceiver<CtrlSignal>) {
        match self.worker.initialize(&mut self.ctx).await {
            Ok(()) => {}
            Err(e) => {
                error!(
                    "Failure during '{}' worker initialisation: {}",
                    self.ctx.address(),
                    e
                );
            }
        }

If I understood correctly this does not stop the worker, but just logs the error. The proposed solution would be to move on to the worker shutdown and context stop ack, without starting the loop in between:

 #[cfg_attr(not(feature = "std"), allow(unused_mut))]
    #[cfg_attr(not(feature = "std"), allow(unused_variables))]
    async fn run(mut self, mut ctrl_rx: SmallReceiver<CtrlSignal>) {
        match self.worker.initialize(&mut self.ctx).await {
            Ok(()) => {}
            Err(e) => {
                error!(
                    "Failure during '{}' worker initialisation: {}",
                    self.ctx.address(),
                    e
                );
                
                // call self.shutdown & ctx.send_stop_ack
                shutdown_and_stop_ack(&mut processor, &mut ctx, &ctx_addr).await;
                return;
            }
        }

Processor is behaving in the same fashion, so I posted worker as a reference.

@pradovic
Copy link
Contributor

I have opened the PR with proposed change: #7654. Pls correct me if I am wrong and I will be happy to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants