registry: switch to fine-grained leasing for flow lifecycle#2127
Merged
k8s-ci-robot merged 3 commits intokubernetes-sigs:mainfrom Jan 14, 2026
Merged
Conversation
k8s-ci-robot
added
kind/bug
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
k8s-ci-robot
added
the
needs-ok-to-test
Contributor
|
Hi @LukeAVanDrie. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
size/L
LukeAVanDrie
commented
Jan 12, 2026
LukeAVanDrie
commented
Jan 12, 2026
Contributor
Author
|
/cc @evacchi |
Contributor
|
@LukeAVanDrie: GitHub didn't allow me to request PR reviews from the following users: evacchi. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
LukeAVanDrie
added a commit
to LukeAVanDrie/gateway-api-inference-extension
that referenced
this pull request
Jan 12, 2026
This commit refactors the FlowController's request lifecycle management to hold the Flow Registry lease (`WithConnection`) for the entire duration of the request, including the queueing phase. Previously, the lease was only held during the instantaneous distribution phase. If a flow had requests waiting in a queue (e.g., during scale-from-zero) but no new incoming traffic, the registry would incorrectly identify the flow as Idle and garbage collect it, orphaning the queued requests. Changes: - Hoisted `WithConnection` in `EnqueueAndWait` to wrap the retry loop and `awaitFinalization`. - Updated `ActiveFlowConnection` interface to expose `FlowKey()`, preventing data clumps in internal signatures. - Refactored `selectDistributionCandidates` to use the active connection instead of re-acquiring it. - Added a regression test (`Regression_LeaseHeldDuringQueueing`) ensuring the lease remains valid while the processor blocks. This change relies on the optimistic concurrency model introduced in PR kubernetes-sigs#2127 to ensure that holding long-lived leases does not block the Garbage Collector or cause writer starvation.
evacchi
reviewed
Jan 13, 2026
Contributor
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
ahg-g
reviewed
Jan 13, 2026
Contributor
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g, LukeAVanDrie The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Contributor
Author
|
/hold Want to address @evacchi 's feedback and double check one more condition. |
k8s-ci-robot
added
the
do-not-merge/hold
aishukamal
reviewed
Jan 13, 2026
Replaces the `flowState.gcLock` mutex with an atomic reference counting
mechanism ("leasing") and optimistic concurrency loop.
This change decouples flow lifecycle management from request processing,
solving two critical issues:
1. Deadlock Potential: Removes the lock hierarchy between Registry,
Shards, and Flows, eliminating the risk of deadlocks during complex
GC or scaling operations.
2. Prereq for Request Starvation (Issue kubernetes-sigs#1982): Allows long-running
queued requests to maintain a "lease" on a flow without holding a
blocking mutex, preventing the GC from orphaning active queues.
The `WithConnection` hot path is now lock-free, using a `sync.Map` and
atomic CAS loop to pin flow state objects safely.
Updates:
- `pkg/epp/flowcontrol/registry`: Switch to optimistic model.
- `pkg/epp/flowcontrol/registry`: Update tests to verify atomic behavior.
Switches flowState internal management from independent atomics to a `sync.Mutex`. This ensures state transitions (lease updates and timestamp clearing) are atomic, simplifying the logic and preventing inconsistent states. Introduces a `markedForDeletion` flag to fix the stalled GC race, where a request could acquire a flow that the GC had already decided to delete. Hardens cleanupFlowResources by using a Write Lock and re-checking the map. This prevents the physical deletion of queues for flows that were resurrected (JIT provisioned) during the cleanup phase.
LukeAVanDrie
force-pushed
the
refactor/registry-optimistic-flow-gc
branch
from
3fa82e3 to
1814907
Compare
k8s-ci-robot
removed
the
lgtm
Adds a targeted test case `ShouldBackOff_WhenFlowIsMarkedForDeletion_ButStillInMap` to verify the robustness of the flow acquisition retry loop. This ensures that if a request encounters a flow object that is logically deleted (marked) but physically present in the map, it correctly backs off and waits for a fresh object rather than resurrecting the dead one.
k8s-ci-robot
added
size/XL
aishukamal
reviewed
Jan 13, 2026
Contributor
aishukamal
left a comment
aishukamal
left a comment
There was a problem hiding this comment.
Thanks for fixing the race! The changes look good to me but just want to point out that with this fix, we are going back to pessimistic concurrency control (use of locks), so I suggest updating the PR title to reflect that.
LukeAVanDrie
changed the title
Contributor
Author
|
/remove-hold |
k8s-ci-robot
removed
the
do-not-merge/hold
Contributor
|
/lgtm |
k8s-ci-robot
added
the
lgtm
LukeAVanDrie
added a commit
to LukeAVanDrie/gateway-api-inference-extension
that referenced
this pull request
Jan 16, 2026
This commit refactors the FlowController's request lifecycle management to hold the Flow Registry lease (`WithConnection`) for the entire duration of the request, including the queueing phase. Previously, the lease was only held during the instantaneous distribution phase. If a flow had requests waiting in a queue (e.g., during scale-from-zero) but no new incoming traffic, the registry would incorrectly identify the flow as Idle and garbage collect it, orphaning the queued requests. Changes: - Hoisted `WithConnection` in `EnqueueAndWait` to wrap the retry loop and `awaitFinalization`. - Updated `ActiveFlowConnection` interface to expose `FlowKey()`, preventing data clumps in internal signatures. - Refactored `selectDistributionCandidates` to use the active connection instead of re-acquiring it. - Added a regression test (`Regression_LeaseHeldDuringQueueing`) ensuring the lease remains valid while the processor blocks. This change relies on the optimistic concurrency model introduced in PR kubernetes-sigs#2127 to ensure that holding long-lived leases does not block the Garbage Collector or cause writer starvation.
k8s-ci-robot
pushed a commit
that referenced
this pull request
Jan 21, 2026
This commit refactors the FlowController's request lifecycle management to hold the Flow Registry lease (`WithConnection`) for the entire duration of the request, including the queueing phase. Previously, the lease was only held during the instantaneous distribution phase. If a flow had requests waiting in a queue (e.g., during scale-from-zero) but no new incoming traffic, the registry would incorrectly identify the flow as Idle and garbage collect it, orphaning the queued requests. Changes: - Hoisted `WithConnection` in `EnqueueAndWait` to wrap the retry loop and `awaitFinalization`. - Updated `ActiveFlowConnection` interface to expose `FlowKey()`, preventing data clumps in internal signatures. - Refactored `selectDistributionCandidates` to use the active connection instead of re-acquiring it. - Added a regression test (`Regression_LeaseHeldDuringQueueing`) ensuring the lease remains valid while the processor blocks. This change relies on the optimistic concurrency model introduced in PR #2127 to ensure that holding long-lived leases does not block the Garbage Collector or cause writer starvation.
This was referenced Jan 23, 2026
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
5 participants
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.
What type of PR is this?
/kind bug
/kind cleanup
What this PR does / why we need it:
This PR refactors the
FlowRegistryconcurrency model, shifting from a rigid, sharded locking hierarchy to a decoupled, fine-grained leasing model for flow lifecycle management.Context
The previous architecture relied on a brittle lock hierarchy (
Registry->Shard->Flow). This coupling made it difficult to safely implement Garbage Collection for parent resources (like Priority Bands) without risking deadlocks. Additionally, the previous GC logic relied on a mutex held during the entire distribution phase. This made it impossible to hold a reference during the (potentially long) queueing phase without blocking the GC completely.Changes
sync.Map) paired with fine-grained per-flow locking (sync.Mutex). This isolates contention to individual flows, allowing the hot path (WithConnection) to proceed without blocking on shard-level or global locks.markedForDeletion) and optimistic retry loops. This allows the system to safely detect and recover from race conditions (such as a flow being deleted by GC while a request is attempting to acquire it) without requiring "stop-the-world" pauses.Note: This is a surgical fix intended to stabilize the flow control layer and unblock future work (like Band GC). While we are currently evaluating the performance profile of the sharded architecture, this change improves maintainability and correctness in the immediate term without requiring a full structural rewrite.
Which issue(s) this PR fixes:
Does this PR introduce a user-facing change?: