Skip to content

feat: add inference request and received time in FlowControlRequest #2475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
k8s-ci-robot merged 3 commits into from
Mar 9, 2026
Merged

feat: add inference request and received time in FlowControlRequest #2475

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
662c4c1
feat: add request headers and received time in FlowControlRequest
loicmarchal Mar 3, 2026
852d504
replace headers by LLMRequest from scheduling layer
loicmarchal Mar 4, 2026
9173f5b
Merge branch 'main' into feature/add-req-headers-to-flowcontrolrequest
loicmarchal Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/epp/framework/interface/flowcontrol/mocks/mocks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
type MockFlowControlRequest struct {
FlowKeyV flowcontrol.FlowKey
ByteSizeV uint64
HeadersV map[string]string
ReceivedTimestampV time.Time
InitialEffectiveTTLV time.Duration
IDV string
MetadataV map[string]any
Expand Down Expand Up @@ -83,6 +85,8 @@ func NewMockFlowControlRequest(

func (m *MockFlowControlRequest) FlowKey() flowcontrol.FlowKey { return m.FlowKeyV }
func (m *MockFlowControlRequest) ByteSize() uint64 { return m.ByteSizeV }
func (m *MockFlowControlRequest) Headers() map[string]string { return m.HeadersV }
func (m *MockFlowControlRequest) ReceivedTimestamp() time.Time { return m.ReceivedTimestampV }
func (m *MockFlowControlRequest) InitialEffectiveTTL() time.Duration { return m.InitialEffectiveTTLV }
func (m *MockFlowControlRequest) ID() string { return m.IDV }
func (m *MockFlowControlRequest) GetMetadata() map[string]any { return m.MetadataV }
Expand Down
6 changes: 6 additions & 0 deletions pkg/epp/framework/interface/flowcontrol/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ type FlowControlRequest interface {
// for managing byte-based capacity limits and for `contracts.FlowRegistry` statistics.
ByteSize() uint64

// Headers returns the request's headers.
Headers() map[string]string

// ReceivedTimestamp returns the timestamp when the request was received.
ReceivedTimestamp() time.Time

// InitialEffectiveTTL returns the suggested Time-To-Live for this request.
// This value is treated as a hint; the `controller.FlowController` may override it based on its own configuration or
// policies. A zero value indicates the request has no specific TTL preference, and a system-wide default should be
Expand Down
6 changes: 6 additions & 0 deletions pkg/epp/requestcontrol/admission.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ func (fcac *FlowControlAdmissionController) Admit(
fairnessID: reqCtx.FairnessID,
priority: priority,
requestByteSize: uint64(reqCtx.RequestSize),
reqHeaders: reqCtx.Request.Headers,

@ahg-g ahg-g Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LukeAVanDrie I am wondering if we should unify the request related state on LLMRequest (and its future successor InferenceRequest?

@LukeAVanDrie LukeAVanDrie Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that will also work and may be ultimately cleaner for the Flow Control layer. Do we want to do that here or in a follow-up? For example this means we can roll the priority field into LLMRequest.RequestObjectives too. I don't want to push that cleanup/simplification work onto this PR though. I can take that on as a no-op refactoring effort later.

@LukeAVanDrie LukeAVanDrie Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, one thing I did try to do in the Flow Control layer is ensure that any plugin author only has read-only access to request / flow / priority band state. If we embed this LLMRequest we leak a mutable pointer reference to the policies, but I also understand if you are not concerned about being overly defensive here in the pursuit of streamlining our types.

@LukeAVanDrie LukeAVanDrie Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @loicmarchal !

@loicmarchal loicmarchal Mar 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahg-g @LukeAVanDrie I replaced the headers by the LLMRequest that also contain the headers. Does that work as a preliminary step to include a more generic InferenceRequest suggested in #2447 later?
I left some of the accessor functions such as ID() and TargetModelName(), although we could access those info from the request itself, to not change the contract too much. Is this ok or should it be refactored?

@LukeAVanDrie LukeAVanDrie Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with this for now. I am happy to do that in a follow-up to not muddy this PR with a ton of mock / test changes.

@loicmarchal loicmarchal Mar 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @LukeAVanDrie !
If you think it should be done, I don't mind cleaning up the code and the mock / test as part of this PR. Let me know what you think is best.

receivedTimestamp: reqCtx.RequestReceivedTimestamp,
reqMetadata: reqCtx.Request.Metadata,
inferencePoolName: fcac.poolName,
modelName: reqCtx.IncomingModelName,
Expand All @@ -177,6 +179,8 @@ type flowControlRequest struct {
fairnessID string
priority int
requestByteSize uint64
reqHeaders map[string]string
receivedTimestamp time.Time
reqMetadata map[string]any
inferencePoolName string
modelName string
Expand All @@ -188,6 +192,8 @@ var _ flowcontrol.FlowControlRequest = &flowControlRequest{}
func (r *flowControlRequest) ID() string { return r.requestID }
func (r *flowControlRequest) InitialEffectiveTTL() time.Duration { return 0 } // Use controller default.
func (r *flowControlRequest) ByteSize() uint64 { return r.requestByteSize }
func (r *flowControlRequest) Headers() map[string]string { return r.reqHeaders }
func (r *flowControlRequest) ReceivedTimestamp() time.Time { return r.receivedTimestamp }
func (r *flowControlRequest) GetMetadata() map[string]any { return r.reqMetadata }
func (r *flowControlRequest) InferencePoolName() string { return r.inferencePoolName }
func (r *flowControlRequest) ModelName() string { return r.modelName }
Expand Down