Repository navigation
fix: Keep the leader lease renewing under API load - #301
Conversation
Lease renewals shared the manager's API connection and client-side rate limiter with every other request. When the API server throttled or the connection slowed, renewals queued behind controller traffic, the renew deadline passed, and the replica lost its lease and restarted. This was seen in production on 2 Oct for milo, where lease GET and PUT timed out awaiting headers during throttling. Key changes: - Build a dedicated client config for leader election: a copy of the base config with its own dialer, no shared rate limiter, and QPS 5, burst 10 - Default release-on-cancel to true in both the flag and the deployment manifest, so a rollout or drain hands over at once instead of waiting out the lease. The manager is the only thing the process runs and the command returns when it stops, so no controller can act after the lease is released - Add tests showing the base config is untouched, and that leader election ignores a saturated main limiter and opens its own connection
|
Two limits found in review, neither a blocker. The change that matters is the dedicated dialer, which gives lease renewal its own connection. Nothing here sets a custom rate limiter on the base config, so dropping the shared limiter and the test for a saturated limiter guard a case this binary does not hit today. The tests cover the helper, not the wiring: removing the leader election config or release on cancel from the manager options would leave them green. Release on cancel is safe without a shutdown wrapper because controller-runtime stops every leader-elected runnable before it releases the lease, and this binary runs no leader-only work outside the manager. |
kevwilliams
left a comment
kevwilliams
left a comment
There was a problem hiding this comment.
Sound. Gives leader election its own rest.Config, clearing the shared rate limiter and dialer so lease renewals get their own connection and their own small QPS/burst, independent of main controller traffic. TestLeaderElectionUsesItsOwnConnection confirms a genuinely separate TCP connection, not just distinct struct fields, and TestLeaderElectionIgnoresSaturatedMainLimiter saturates the main client's limiter and confirms lease renewal still succeeds. ReleaseOnCancel is safe here since the process exits immediately after the manager stops, so nothing can keep acting as leader after the lease releases. Matches the cited production incident. CI green.
3 participants
Summary
A leader replica could lose its lease and restart whenever the API server throttled requests or the connection slowed, because lease renewals queued on the same connection and behind the same rate limit as all other controller traffic. This happened to milo in production on 2 Oct: milo-os/milo#811
Lease renewals now get their own connection and their own small rate limit, so other traffic cannot hold them up.
A stopping leader now gives up its lease, so a rollout or drain hands over straight away instead of waiting for the lease to expire.
Why releasing the lease on shutdown is safe
Releasing the lease on shutdown was already an option here, but it was off by default and in the shipped deployment. It is now on in both.
The manager is the only thing this process runs, and the command returns as soon as the manager stops, so no controller can keep working after the lease is released.
Test plan
Related to https://github.com/datum-cloud/infra/issues/6603