Skip to content
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

Jump to bottom

nixos/haproxy: Implement hitless reloads #89012

Closed
wants to merge 1 commit into from
Closed

nixos/haproxy: Implement hitless reloads #89012

wants to merge 1 commit into from

Conversation

talyz
Copy link
Contributor

@talyz talyz commented May 27, 2020

Motivation for this change

Since 1.8 HAProxy has had support for hitless reloads. Implement support for this and prefer reloading over restarting the service.

For this to work properly, we need to switch the service type back to forking, since notify sadly doesn't allow new processes to be spawned and take over in ExecReload.

Things done

I've stress tested the configuration by using hey and reloading the haproxy service in a loop, yielding good results.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@talyz
nixos/haproxy: Implement hitless reloads
c14cd8a 
Since 1.8 HAProxy has had support for hitless reloads. Implement
support for this and prefer reloading over restarting the service.

For this to work properly, we need to switch the service type back to
forking, since notify sadly doesn't allow new processes to be spawned
and take over in ExecReload.
@talyz talyz requested review from peterhoeg and bjornfor May 27, 2020 12:16
@peterhoeg
Copy link
Member

Based on my reading of https://www.haproxy.com/blog/hitless-reloads-with-haproxy-howto/, forking isn't necessary. Did you try it out without?

@talyz
Copy link
Contributor Author

talyz commented May 28, 2020

Yeah, I tried to get it working with notify first, but ran into trouble since systemd only expects the process started by ExecReload to send a message or signal to the main process and exit, not actually replace the main process. The upstream unit file implements reloads by sending the SIGUSR2 signal to the main process, which should work fine on FHS, but since we don't replace either the binary or configuration, reexecing isn't good enough - we need to replace the process by starting the new binary in ExecReload. This behavior is only possible with forking, it seems, since systemd checks the PIDFile to learn the main process PID even after a reload.

@aanderse aanderse closed this May 29, 2020
@aanderse aanderse reopened this May 29, 2020
@aanderse
Copy link
Member

Oops.

What I meant to say: #88434

@peterhoeg
Copy link
Member

Thanks for doing this @talyz - the referenced PR covers quite a bit more (and has tests), so I suggest to continue the work over there.

@peterhoeg peterhoeg closed this May 29, 2020
@talyz talyz deleted the haproxy-hitless-reloads branch June 16, 2021 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterhoeg peterhoeg Awaiting requested review from peterhoeg

@bjornfor bjornfor Awaiting requested review from bjornfor

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@talyz @peterhoeg @aanderse