etix / mirrorbits

Mirrorbits is a geographical download redirector written in Go for distributing files efficiently across a set of mirrors.
MIT License
503 stars 91 forks source link

service: Support systemd notify type #90

Closed dbnicholson closed 5 years ago

dbnicholson commented 5 years ago

With the current default of Type=simple, systemd considers mirrorbits started as soon as the mirrorbits process starts. However, neither the RPC nor HTTP ports are bound at that point, so services that start After mirrorbits may fail if systemd starts them immediately.

Change to Type=notify and send the READY=1 message through the notify socket after the ports are bound but just before the HTTP server starts. If systemd is not in use (the NOTIFY_SOCKET environment variable is not set), then this is a no-op.

This is against the 0.5 branch (with the 0.5.1 commits) since I don't have a Go 1.11 environment handy to use the new Go module support. I can work on that, but I wanted to get an idea if this was useful or not. For me it makes our bootup process reliable since we have a service that starts after mirrorbits that exports the mirmon list to a static HTTP server path. It currently fails since the RPC port isn't bound yet.

etix commented 5 years ago

Thank you for your contribution, this is indeed very helpful.

To be able to merge your code I will need it on top of master. Don't worry about the Go version, I can always rework your code before merging so it's compatible with Go 1.11.

dbnicholson commented 5 years ago

The change was trivial enough that I just squashed it into the last patch and force pushed again.

etix commented 5 years ago

Can you try rebasing on master as well?

dbnicholson commented 5 years ago

Can you try rebasing on master as well?

Well, half of the branch won't work because of the module setup differences and I don't have a go 1.11 setup for testing. That's why I did it against the 0.5 branch. Let me try on a separate branch since I'd want this one against 0.5 anyways.

dbnicholson commented 5 years ago

I updated this branch with the SdNotify call under the Getenv conditional. I'll look at master now.

etix commented 5 years ago

Merged on master (not in the 0.5 branch), authorship preserved. Thank you for your contribution.