uber-go / fx

A dependency injection based application framework for Go.
https://uber-go.github.io/fx/
MIT License
5.48k stars 283 forks source link

feat(signal): add StopDelay option #1205

Closed Koumbaya closed 1 month ago

Koumbaya commented 1 month ago

Hello 👋. Fx "under the hood" handling of the shutdown signals is preventing us from fixing a race condition between sidecars and services at the k8s level.

For non-fx go programs, we handle this by using a main context with a delayed call to the CancelFunc when Notify triggers on a syscall. With fx, this could also be handled by adding some time.Sleep to every OnStop hook but it's not really elegant nor flexible.

This is my attempt at fixing that, another less "specific to my problem" solution would be to have a way to interact with or replace the signalReceivers.notify function maybe ?

I'm not super happy with m.app.receivers.delaySignal because it rely only on the order of operation in app.go New() and the fact that everything is in the same package, but I wanted to avoid changing newSignalReceivers signature.

Happy to hear what you think about it, what needs to be changed etc 👍

Koumbaya commented 1 month ago

Hmmmm, I thought OnStop hooks were called in parallel, but they're not, so an even simpler solution would be to just add a last OnStop hook to our application, that will be executed first and that just sleeps X seconds. I think this render this PR not so useful...