feat: improved signal handling #79
Conversation
|
I think we're going to need a way to disambiguate between different signals. As our behaviors might be different depending on what signal we receive. Is this possible with |
.dockerignore
Outdated
| @@ -0,0 +1,2 @@ | |||
| .git | |||
There was a problem hiding this comment.
Is this file added intentionally?
There was a problem hiding this comment.
ugh woops I had that in a separate PR and checked it in accidentally because it was slowing down building docker containers a lot on my local. I'll get rid of that
ctrlc crate, by default, only handles
I think if we wanted to handle other signals in specific ways we'd probably want to use the signal-hook crate. There is a Tokio integration which would probably make it easier. |
|
@brayniac Are there any signals we want to handle besides |
|
@swlynch99 yeah I would vote in favor of getting SIGINT working properly (and potentially turning on |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
brayniac
changed the title
brayniac
changed the title
fixes: #74
spawn()which sendsSignal::Shutdownto the admin thread of the processIn my experience locally the workers seem to get the shutdown Signal and stop processing requests. However, it's desirable to ensure that the child threads actually stop. I believe that is occurring because we call
Process.wait()pelikan/src/core/server/src/process.rs
Line 136 in 42534bb
pelikan/src/server/segcache/src/main.rs
Line 123 in a4f63e2
Indeed, when I add println! statements and mess around with
thread::sleep(), it seems like we are in fact waiting for all the worker threads to finish when I sendSIGINTtopelikan_segcache_rsI could see there being an issue with adding/using
ctrlc. It spawns a thread to listen to signals, which seems like a potentially worthwhile tradeoff to make handlingSIGINTeasier, as the other libraries I found like https://docs.rs/signal-hook/latest/signal_hook/ are considerably lower level. If we want to use one of those we could potentially avoid having a dedicated thread by having some currently existing thread check for signals, but I think it's likely to complicate the implementation.