Wait for background workers to finish current jobs before quitting

[jacovdbergh]

This PR utilises the cancellation token provided by sidekiq-rs to wait for currently processing background jobs before exiting.

When the token is canceled, no new jobs will be picked up by the queue, only currently running jobs will be completed.

Waiting can be interrupted by sending another ctrl-c if the user wishes to force quit the app.

The PR aims to do this in a generic way so that a similar approach can be implemented for the postgres queue implementation as well in future.

Feedback on the println! added is welcomed, to fit in with the rest of Loco's way/style of outputting info.

[jondot]

@jacovdbergh logic have changed just a bit, can you take a look at the conflict and see what makes sense?

[jacovdbergh]

@jondot merged in master and fixed conflict. Also fixed clippy and other CI checks, so good to merge from my side if you're happy.

[jondot]

Thanks for this fix.
I think if you could clarify the case -- in terms of how the code reads from top to bottom -- when we only want to run a worker it would be perfect. (referring to the boot process here https://github.com/loco-rs/loco/pull/860/files#diff-d912ff9d8ba1a195bf14282d38de1732d78a6cafdb35140bd3510edf01ccc8bfR89).

Note that there may be already a bug there, say we have a BackgroundQueue and run worker only. router will be None, but the tokio in-process spawn will still happen (as far as I can read this).

It might be worth to explicitly have back the case for (None, true) where there is no router, and the user wants to have a worker online.

Otherwise when the cases are unified, it kind of mingles with also having to keep in mind the handle, makes a mental gymnastic to try to understand all the different cases.

So anything you can do to make the 3 cases distinct might be helpful (1) server only (2) worker+server, inprocess worker (3) worker only. it's also worth remembering that the modes for the worker make or may not make sense in one of the 3 cases (BackgroundQueue, Async, Foreground). For example, Async does not make sense in a stand-alone process.

[jacovdbergh]

The reason I changed it to always spawn the queue into a tokio worker thread is because we need to somehow listen (in another thread) for the shutdown signal.

After the shutdown signal, we join on the queue worker handle, which lets it finish any currently running tasks, and then gracefully quit.

So in short, we always need a worker and handle for the queue worker now, and then the shutdown signal is either awaited in the Router or explicitly by the same code that spawned the queue worker.

Let me know how you think the above can be achieved differently?

[jondot]

Got it now! thanks for the clarification
Could you maybe experiment with returning the (None, true) case (at the cost of duplicating some of the code) so we can see how more readable it is?

[jacovdbergh]

I've refactored the flow for readability, let me know if that's better?

[jondot]

oh wow fantastic! thanks for this, super clear! ❤️