feat(load-balancer): suspend and resume a pool around a sleeping container - #64
Merged
Conversation
…ainer The pool half of scale-to-zero (#19). Nothing calls it yet -- the idle controller is wired up when the request gate lands -- so proxy behavior is unchanged. SuspendForSleep empties the pool and stops probing, so a container that was deliberately stopped is neither routed to nor dialled once a second for the whole nap. It runs before the containers go down, and the controller holds arriving requests while it does, so the empty pool is never observable. ResumeFromSleep is the subtle half, and it exists because of how a single-target pool behaves -- which is the only shape scale-to-zero actually targets. updateHealthyTargets stops health checking once a lone target first goes healthy, so markHealthy has already fired and waitForHealthyContext is already cancelled. Without re-arming, WaitUntilHealthy returns nil INSTANTLY for a container that has not started, and the wake would report ready and forward its held request straight into a connection refused. Verified by mutation: drop the re-arm and TestLoadBalancer_ResumeFromSleepRearmsWaitUntilHealthy fails in 0.00s with "readiness was reported for a target that never answered". The context is replaced rather than cancelled. WaitUntilHealthy treats any non-deadline cancellation as success, so cancelling would tell a waiter that parked before the resume "healthy" at the very moment every target was marked unverified. The cost is that such a waiter times out instead of ever being released; that is a timeout it can retry rather than a false ready, and it is unreachable in the real flow because the controller serializes wakes behind its generation counter. Targets re-enter as Adding, not Healthy: a successful probe promotes Adding to Healthy while a failed one only ever demotes Healthy to Unhealthy, so a container that never comes up stays out of the pool instead of flapping into it. A woken target is therefore held to exactly the standard a freshly deployed one is. WaitUntilHealthy now reads waitForHealthyContext under the lock. This is the third race from the review that landed in 9e6f28b, deliberately left then: the field was written once in NewLoadBalancer before publication, so it was not a race until something wrote it at runtime. ResumeFromSleep is that writer, so the lock lands in the commit where the second writer appears rather than ahead of it. Reuses BeginHealthChecks rather than adding a restart variant. The plan called for a separate method because BeginHealthChecks assigned stateConsumer outside the inflight lock, but 9e6f28b moved that assignment inside, so there is nothing left to work around. Refs #19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The pool half of scale-to-zero (#19): two
LoadBalancermethods that take targets out of rotation while their container sleeps and put them back when it wakes.Still inert. Nothing calls them yet — the idle controller gets wired up when the request gate lands. Proxy behavior is unchanged.
ResumeFromSleepis the whole point of this PRSuspendForSleepis the obvious half: empty the pool, stop probing, so a deliberately-stopped container is neither routed to nor dialled once a second for the entire nap.The resume is where the real defect lives, and it only shows up in the shape scale-to-zero actually targets — a single-target pool:
updateHealthyTargetsstops health checking once a lone target first goes healthy (load_balancer.go:313). That meansmarkHealthy()has already fired andwaitForHealthyContextis already cancelled. So on wake, without re-arming:The wake reports ready and forwards its held request straight into a connection refused. Verified by mutation — drop the re-arm and
TestLoadBalancer_ResumeFromSleepRearmsWaitUntilHealthyfails in0.00swith "readiness was reported for a target that never answered".Replaced, not cancelled — and what that costs
WaitUntilHealthytreats any non-deadline cancellation as success. Cancelling the old context on resume would therefore tell a waiter that parked before the resume "healthy" at the exact moment every target was marked unverified.So the context is replaced. The cost, which I want visible rather than buried: a waiter spanning a resume is never released and times out instead. That is a timeout it can retry rather than a false ready, and it is unreachable in the real flow — the controller serializes wakes behind its generation counter, so each wake waits on the context its own resume installed.
TestLoadBalancer_ResumeFromSleepDoesNotReleaseAPreviousWaiterpins that behavior explicitly rather than leaving it to be discovered.Targets re-enter as
Adding, notHealthyA successful probe promotes
Adding → Healthy; a failed one only ever demotesHealthy → Unhealthy. So a container that never comes up stays out of the pool instead of flapping into it, and a woken target is held to exactly the standard a freshly deployed one is.The third race, now fixed where it belongs
WaitUntilHealthynow readswaitForHealthyContextunder the lock. This is the third finding from the review that landed in #58, which I deliberately did not fix then: the field was written once inNewLoadBalancerbefore publication, so it was not a race — there was no second writer.ResumeFromSleepis that second writer, so the lock lands in the commit that creates the defect rather than ahead of it.Simplification the plan didn't anticipate
docs/plans/2026-07-29-scale-to-zero.mdcalls for a newRestartHealthChecksmethod, becauseBeginHealthChecksassignedstateConsumeroutside the inflight lock and was only safe before a target served anything. #58 moved that assignment inside the lock, so there is nothing left to work around — this reusesBeginHealthChecksdirectly and adds no method.Test plan
Six tests, all against a single-target pool on purpose:
SuspendForSleepEmptiesThePoolAndStopsProbing— pool empty, and probe count flat across three check intervalsResumeFromSleepRearmsWaitUntilHealthy— the headline; readiness withheld while the backend 503s, granted once it answers. Mutation-verified.ResumeFromSleepDoesNotReleaseAPreviousWaiter— a resume never hands a parked waiter a successResumeFromSleepReentersUnverified— suspended targets leave the pool; woken ones rejoin only after a probeSuspendAndResumeAreSafeUnderConcurrentRouting— 20 sleep/wake cycles against 200 concurrent pool readsgofmt,go vetclean;go test -race— 1251 passDeviations & judgment calls
ResumeFromSleepDoesNotReleaseAPreviousWaiteroriginally asserted the parked waiter would eventually succeed once the backend answered. It does not, and should not — the resume replaced themarkHealthythat would have closed its context. The design is right; my assertion was wrong. It now pins the actual guarantee (no false success) and documents why the timeout is the acceptable outcome.SuspendForSleep/ResumeFromSleeprun once per sleep and once per wake, not per request.WaitUntilHealthygained one uncontended mutex acquire; it is called once per deploy and once per wake, not on the request path.SuspendForSleepclears readers as well as writers. Read targets are replicas whose lifecycle the proxy does not own, so they are never stopped — but leaving them in a pool whose writers are gone would route reads at a service that cannot serve writes. Worth a second opinion if you disagree.Refs #19