-
Notifications
You must be signed in to change notification settings - Fork 235
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
Feature: Shutdown #771
Comments
CC @ericschaal |
can you elaborate more on what the scenario of shutdown look like? like will we store the state & then restart where we left of? If yes, then what would be included in the state. Thanks 😁 |
Good questions! I am running rumqttd alongside other tasks, and we need the flexibility of turning on and off the broker, without having the possibility of shutting down the process itself I am not sure what that should look like internally, but my understanding is that when we kill the process, nothing special happens in terms of state, etc. So, I would imagine a "shutdown" command to mirror that---whatever happens when receiving a kill signal would happen when shutting down (and indeed, you would be able to replace any logic on kill signal with the shutdown command newly created) I don't know enough about the internals yet. Is there any such logic on process kill? |
Regarding your point on restarting where we left off, I see this outside of the scope of this issue (for my use case). However, I think that if it is part of your plan to have restart logic, it should be taken into consideration! |
gotcha! so basically we want to return from Best way I can think of is using let handle = console_thread.spawn(move || {
let mut runtime = tokio::runtime::Builder::new_current_thread();
let runtime = runtime.enable_all().build().unwrap();
runtime.block_on(console::start(console_link))
})?;
// here check the return value of the console, if we return the error
// ( which we will do for shutdown signal, we can return from start
if let Ok(Err(e)) = handle.join() {
// determine what to return
return _;
}; and add a endpoint to console like only concern is what would happen to other spawned threads ( ideally, they will be detached as their join handles will be dropped, but not so sure about the cleanup ) other than that, also the concern of how we would restart 🤔 would be more than happy to help you with this :) but will have to first discuss regarding this feature though. cc: @tekjar @henil |
I wonder will it be possible to have a |
Exactly! I was surprised to find that it is not possible to terminate a thread in std::thread. So, unless you transition to futures (which do have Alternatively, it could be more simple to return a handle in This approach would be quite versatile but breaking, as developers would have to have an async runtime. Additionally, developers would have to
But we could also simply return from |
returning a handle for sending control messages from The handle can act as console, but without HTTP endpoints, and console can use it internally as well haha |
I think returning handle might get tricky:
|
i like the idea of using a shutdown endpoint in console but not everyone might want to use all the things from console (previously so i think adding a new oneshot channel which users can get handle to before calling
as @AlexandreCassagne mentioned i think for now we can ignore cleanup and just drop the handles. |
It may be worthwhile to open another issue to track cleanup |
I was experimenting with this, and I think we can't ignore the cleanup! even if we drop the handles, threads will continue their execution in background, i.e. everything will still be running, even after user requested to shut down. There is no way to kill a thread externally, afaik. And sending some rx to each of the thread, so that thread can listen on it for kill signal ( while they are executing their main task ) is getting kinda complicated haha. I will try further to see if there is something we can figure out! lmk if you have any suggestions, thanks :) |
Hey @swanandx! have you pushed your code somewhere? I'd love to take a look |
Is there any way to migrate from a threads implementation to tokio tasks? This would allow us to use the tokio::task::AbortHandle. |
nope :( I was just playing around locally, I can provide this though: fn main() {
let h = work();
println!("Done with the stream!!");
drop(h);
println!("Dropped!!");
loop {}
}
fn work() -> thread::JoinHandle<()> {
// tokio::spawn(async {
// loop {
// println!("still alive");
// sleep(Duration::from_millis(500)).await;
// }
// })
thread::spawn(|| loop {
println!("alive :)");
sleep(Duration::from_millis(500));
})
} even after we drop the handle, the thread stays alive!
in order to know when to abort , we would still need to use something like not sure about if this excess use of |
I will try to push code with |
here is poc - main...rumqttd-shutdown we return an |
nice! it doesn't look as bad as you suggested yesterday! |
Hey @AlexandreCassagne @swanandx |
Hey @aohotnik, yes, having ability to shutdown would be required especially when rumqttd is used as library ( like case you mentioned ) I got caught up with other work, so this got delayed 😕 btw, will something similar to shown in PoC here would resolve your needs? #771 (comment) Would be cool if you provide the feedback so we can work in getting it from PoC to production haha. We still have to analyse perf / memory usage impact if any ( likely there won't be much ). Thanks! |
@swanandx If it will stop all spun up by broker threads and frees all resources when DLL is unloaded by the main process - I think it would be enough. I'm brainstorming right now, so there is no prototype to test on, but I will be able to use your linked code in a fork to try it out when time comes. |
Expected Behavior
There should be a mechanism for shutting Rumqttd down.
Current Behavior
No such method (that I could find).
Failure Information (for bugs)
N/A
Context
Could you point me in the right direction so I can implement this? Should this be initiated by the link_tx? or other place?
The text was updated successfully, but these errors were encountered: