I think this backport is faithful and ready for a human to look at. I'm not recommending it skip human review, though. It changes the activator's hot path on a release branch, and one deliberate omission has a concrete effect on deploys that someone should sign off on.
What holds up on this tree
wakeAllWaiters, poolState.revision, versionRef.strategy, the maxInstances computed at the top of requestPoolCapacityTo, and the cond/errors imports in manager.go. wg.Go needs Go 1.25+, and go.mod is on 1.26.10.AcquireOverflow adds 1 to used and returns size 1. Ingress hands that same size back, and AutoStrategy.releaseCapacity subtracts it.RenewLease returns the same *Lease it was given, so a long-running overflow request keeps its Overflow flag across expireLeases ticks. It doesn't turn into a reusable size-1 lease.invalidateSandboxLeases or invalidateAppLeases drops an overflow lease mid-request, the later releaseLease won't find it in ar.leases, so it doesn't release it a second time.proxyWithLease, the deferred recover releases and re-panics only on the panic path. Normal returns still go through the caller's releaseLease/invalidateLease.TestScaleDownDoesNotOverwriteConcurrentScaleUp pins this.For a human to decide
controllers/deployment/launcher.go:480, desiredInstances := int64(1)). With over-target admission, every request for the new version that misses the ingress cache goes to that single sandbox instead of parking. checkForSandbox also deliberately hands every queued waiter to the first routable sandbox. So right after a deploy of a busy service, one fresh sandbox takes all of the new version's traffic until the demand-sized scale-up catches up. That's still better than the 120s hangs this fixes, and 1783c6966 seems to exist for exactly this moment. Someone should confirm that post-deploy burst is acceptable for miren.cloud's web service on v0.17.3, or decide to bring 1783c6966 along.AcquireLease now holds the write lock for its whole scan, and waiters register before the capacity request. Existing activator tests may have quietly depended on the old timing.Verdict: ready
{
"owner": "mirendev",
"repo": "runtime",
"number": 1427,
"verdict": "ready",
"event": "comment",
"summary": "I think this backport is faithful and ready for a human to look at. I'm not recommending it skip human review, though. It changes the activator's hot path on a release branch, and one deliberate omission has a concrete effect on deploys that someone should sign off on.\n\n**What holds up on this tree**\n- **It compiles against release/0.17.** Everything the backported code calls exists here: `wakeAllWaiters`, `poolState.revision`, `versionRef.strategy`, the `maxInstances` computed at the top of `requestPoolCapacityTo`, and the `cond`/`errors` imports in `manager.go`. `wg.Go` needs Go 1.25+, and `go.mod` is on 1.26.10.\n- **Overflow leases stay balanced.**\n - `AcquireOverflow` adds 1 to `used` and returns size 1. Ingress hands that same size back, and `AutoStrategy.releaseCapacity` subtracts it.\n - `RenewLease` returns the same `*Lease` it was given, so a long-running overflow request keeps its `Overflow` flag across `expireLeases` ticks. It doesn't turn into a reusable size-1 lease.\n - If `invalidateSandboxLeases` or `invalidateAppLeases` drops an overflow lease mid-request, the later `releaseLease` won't find it in `ar.leases`, so it doesn't release it a second time.\n- **The abort fix releases exactly once.** In `proxyWithLease`, the deferred recover releases and re-panics only on the panic path. Normal returns still go through the caller's `releaseLease`/`invalidateLease`.\n- **The scale-down change is self-contained.** It patches against the revision from the listing and treats a conflict as \"try next tick\". `TestScaleDownDoesNotOverwriteConcurrentScaleUp` pins this.\n\n**For a human to decide**\n1. **Deploys without 1783c6966.** On this branch a new auto-mode pool starts at one instance (`controllers/deployment/launcher.go:480`, `desiredInstances := int64(1)`). With over-target admission, every request for the new version that misses the ingress cache goes to that single sandbox instead of parking. `checkForSandbox` also deliberately hands every queued waiter to the first routable sandbox. So right after a deploy of a busy service, one fresh sandbox takes all of the new version's traffic until the demand-sized scale-up catches up. That's still better than the 120s hangs this fixes, and 1783c6966 seems to exist for exactly this moment. Someone should confirm that post-deploy burst is acceptable for miren.cloud's web service on v0.17.3, or decide to bring 1783c6966 along.\n2. **The etcd-backed tests haven't run on this branch.** You said you're relying on CI for them, so please check that run before merging. `AcquireLease` now holds the write lock for its whole scan, and waiters register before the capacity request. Existing activator tests may have quietly depended on the old timing.",
"posted_to_pr": true,
"draft": true,
"head_sha": "2abcdf46a75f9f295144607885d5b1c0e50e00fe"
}