Repository navigation
fix: Keep the leader lease renewing under API load - #30
Conversation
Lease renewals shared each 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 in both the VPC controller and the fabric identity controller: a copy of the base config with its own dialer, no shared rate limiter, and QPS 5, burst 10 - Release the lease when the manager stops, so a rollout or drain hands over at once instead of waiting out the lease. The manager is the only thing either process runs and each exits when it returns, 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. Same fix as datum#301, applied to both controllers in this repo through a shared internal/leaderelection package rather than duplicating the logic per binary. Same connection-isolation and rate-limit-isolation tests, same mutation-safety test. ReleaseOnCancel is safe since each controller's process exits immediately after its manager stops. 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
Both controllers in this repository get the fix. In each, the manager is the only thing the process runs, and the process exits 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