refactor: [Scale from Zero] Introduce PodLocator #1950

LukeAVanDrie · 2025-12-04T19:31:17Z

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
This PR refactors the candidate resolution logic out of the Director and into a dedicated PodLocator component.

Reasoning:

Reduce Lock Contention: High-throughput dispatch loops frequently query the Datastore for the same subset of pods. This introduces CachedPodLocator, a decorator that caches resolution results (TTL 50ms), significantly reducing RLock contention on the central Datastore.
Preparation for Lazy Resolution: To support Scale-from-Zero (in a future PR), the Flow Control layer will need to resolve pods after requests have been enqueued. This interface decouples the resolution logic from the Director's immediate request scope.

Changes:

Added contracts.PodLocator interface.
Implemented DatastorePodLocator (logic moved from Director) and CachedPodLocator.
Injected PodLocator into the Director.
Replaced getCandidatePodsForScheduling with locator.Locate().

Note: This is a pure refactor. The sequence of events in HandleRequest remains unchanged (Admission is still checked after resolution).

Which issue(s) this PR fixes:
Tracks #1800

Does this PR introduce a user-facing change?:
NONE

This defines the contract for resolving candidate pods based on request metadata, decoupling the resolution logic from the storage layer.

Introduces DatastorePodLocator and a caching decorator. This reduces contention on the Datastore RWMutex during high-throughput dispatch cycles by caching subset resolution results for a short TTL.

Refactors the Director to use the injected PodLocator interface instead of the private getCandidatePodsForScheduling method. This prepares the Director for lazy resolution without changing current behavior.

netlify · 2025-12-04T19:31:23Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`cce12bf`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/6931e188c0e0d7000981e423
😎 Deploy Preview	https://deploy-preview-1950--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2025-12-04T19:31:28Z

Hi @LukeAVanDrie. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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 · 2025-12-04T19:36:09Z

pkg/epp/flowcontrol/contracts/dependencies.go

I am defining this interface within the Flow Control contracts package as it will serve as the primary contract for lazy candidate resolution during the dispatch cycle.

Crucially, this abstraction decouples the core request lifecycle from specific upstream filtering logic. While the current implementation handles Envoy subsetting, isolating this behavior behind an interface paves the way for promoting it to an EPP Extension Point. This would allow adopters to inject environment-specific or vendor-customized discovery mechanisms in the future without polluting the core directory.

LukeAVanDrie · 2025-12-04T19:40:00Z

@lionelvillard

I have one more PR stacked on top of this that moves the candidate resolution line to after the admission control check. This involves injecting the PodLocator into the FlowControl layer code and performing lazy candidate resolution before each call to the saturation detector.

Splitting that out as a separate PR to keep this one as a behavioral no-op refactor. I will likely need to update some integration tests on the subsequent PR as well.

ahg-g · 2025-12-04T23:20:40Z

/ok-to-test

ahg-g · 2025-12-05T16:51:35Z

Reduce Lock Contention: High-throughput dispatch loops frequently query the Datastore for the same subset of pods. This introduces CachedPodLocator, a decorator that caches resolution results (TTL 50ms), significantly reducing RLock contention on the central Datastore.

What changed that made us concerned about RLock contention? is that expected as a result of the follow up PR? is this why we are introducing this cashing layer?

LukeAVanDrie · 2025-12-05T17:25:50Z

What changed that made us concerned about RLock contention? is that expected as a result of the follow up PR?

Yes, this is specifically to support the scale-from-zero logic in the follow-up PR (#1952).

In the current legacy path, we resolve pods once per request. In the Flow Control path, the ShardProcessor needs to re-evaluate saturation state frequently for HoL requests (once per dispatch attempt--effectively polling to see if new capacity has appeared). Without this caching layer, that polling loop (1ms at its slowest) would hammer the Datastore RLock, potentially starving the writer threads trying to update the endpoint state.

is this why we are introducing this cashing layer?

Yes. I opted to encapsulate this inside the PodLocator decorator rather than adding complexity to the FlowController internals. This keeps the controller logic focused purely on queuing mechanics, while the locator handles the performance optimization transparently.

Perhaps this CachedPodLocator change belongs in a separate PR or even #1952. I thought it would be easiest to review alongside the DatastorePodLocator delegate though.

ahg-g

It seems the most expensive part is that we are creating a full snapshot of the pod metrics per request, we should find a way to take a snapshot and share it among requests with a "view" on that cache based on the latest subset.

ahg-g · 2025-12-05T20:10:15Z

pkg/epp/requestcontrol/locator.go

+
+// DatastorePodLocator implements contracts.PodLocator by querying the EPP Datastore.
+// It centralizes the logic for resolving candidate pods based on request metadata (specifically Envoy subset filters).
+type DatastorePodLocator struct {


I think we are converging on endpoints instead of pods, right @kfswain ?

ahg-g · 2025-12-05T20:13:13Z

pkg/epp/flowcontrol/contracts/dependencies.go

+//
+// This interface allows the Flow Controller to fetch a fresh list of pods dynamically during the dispatch cycle,
+// enabling support for "Scale-from-Zero" scenarios where pods may not exist when the request is first enqueued.
+type PodLocator interface {


why not keep the candidates term instead? like EndpointCandidates or CandidateEndpoints?

Are you just referring to the interface name here? Both of these seem better to me.

Yes, referring to the name.

We can merge this as is and then rename after the other PR merges just so we don't cause conflicts with the second PR.

LukeAVanDrie · 2025-12-05T20:29:15Z

It seems the most expensive part is that we are creating a full snapshot of the pod metrics per request, we should find a way to take a snapshot and share it among requests with a "view" on that cache based on the latest subset.

This is what the caching decorator is for. I am caching by "subset key" though. Not per-pod. If two requests share the same subset, we use the cached results. This handles 99% of the calls which are sourced from tight loop spinning on the HoL request in Flow Control. Cache misses for most unique requests are probably fine as this is how EPP operated before this change.

I was trying to strike a balance between caching on request ID and caching on pod namepsaced name.

ahg-g · 2025-12-05T20:52:12Z

It seems the most expensive part is that we are creating a full snapshot of the pod metrics per request, we should find a way to take a snapshot and share it among requests with a "view" on that cache based on the latest subset.

This is what the caching decorator is for. I am caching by "subset key" though. Not per-pod. If two requests share the same subset, we use the cached results. This handles 99% of the calls which are sourced from tight loop spinning on the HoL request in Flow Control. Cache misses for most unique requests are probably fine as this is how EPP operated before this change.

I was trying to strike a balance between caching on request ID and caching on pod namepsaced name.

Great!

ahg-g · 2025-12-06T04:24:03Z

/lgtm
/approve

k8s-ci-robot · 2025-12-06T04:24:10Z

[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

Needs approval from an approver in each of these files:

~~OWNERS~~ [ahg-g]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

LukeAVanDrie added 3 commits December 4, 2025 19:12

contracts: add PodLocator for candidate resolution

a734712

This defines the contract for resolving candidate pods based on request metadata, decoupling the resolution logic from the storage layer.

requestcontrol: implement CachedPodLocator

716fe89

Introduces DatastorePodLocator and a caching decorator. This reduces contention on the Datastore RWMutex during high-throughput dispatch cycles by caching subset resolution results for a short TTL.

director: delegate candidate resolution

cce12bf

Refactors the Director to use the injected PodLocator interface instead of the private getCandidatePodsForScheduling method. This prepares the Director for lazy resolution without changing current behavior.

k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Dec 4, 2025

k8s-ci-robot requested review from nirrozenbaum and shmuelk December 4, 2025 19:31

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 4, 2025

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Dec 4, 2025

LukeAVanDrie commented Dec 4, 2025

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 4, 2025

LukeAVanDrie mentioned this pull request Dec 4, 2025

feat: Enable Scale-from-Zero #1952

Draft

ahg-g reviewed Dec 5, 2025

View reviewed changes

k8s-ci-robot assigned ahg-g Dec 6, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 6, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2025

k8s-ci-robot merged commit a88e4fb into kubernetes-sigs:main Dec 6, 2025
11 of 12 checks passed

refactor: [Scale from Zero] Introduce PodLocator #1950

refactor: [Scale from Zero] Introduce PodLocator #1950

Conversation

LukeAVanDrie commented Dec 4, 2025

Uh oh!

netlify bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for gateway-api-inference-extension ready!

Uh oh!

k8s-ci-robot commented Dec 4, 2025

Uh oh!

LukeAVanDrie Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

LukeAVanDrie commented Dec 4, 2025

Uh oh!

ahg-g commented Dec 4, 2025

Uh oh!

ahg-g commented Dec 5, 2025

Uh oh!

LukeAVanDrie commented Dec 5, 2025

Uh oh!

ahg-g left a comment

Choose a reason for hiding this comment

Uh oh!

ahg-g Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

ahg-g Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

LukeAVanDrie Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

ahg-g Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

ahg-g Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

LukeAVanDrie commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahg-g commented Dec 5, 2025

Uh oh!

ahg-g commented Dec 6, 2025

Uh oh!

k8s-ci-robot commented Dec 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify bot commented Dec 4, 2025 •

edited

Loading

LukeAVanDrie commented Dec 5, 2025 •

edited

Loading