-
Notifications
You must be signed in to change notification settings - Fork 284
Preparedata timeout config #2111
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,17 +59,22 @@ type EndpointPickerConfig struct { | |
| // FlowControl configures the Flow Control layer. | ||
| // This configuration is only respected if the "flowControl" FeatureGate is enabled. | ||
| FlowControl *FlowControlConfig `json:"flowControl,omitempty"` | ||
|
|
||
| // +optional | ||
| // RequestControl configures the request control stage of the EPP pipeline. | ||
| RequestControl *RequestControlConfig `json:"requestControl,omitempty"` | ||
| } | ||
|
|
||
| func (cfg EndpointPickerConfig) String() string { | ||
| return fmt.Sprintf( | ||
| "{FeatureGates: %v, Plugins: %v, SchedulingProfiles: %v, Data: %v, SaturationDetector: %v, FlowControl: %v}", | ||
| "{FeatureGates: %v, Plugins: %v, SchedulingProfiles: %v, Data: %v, SaturationDetector: %v, FlowControl: %v, RequestControl: %v}", | ||
| cfg.FeatureGates, | ||
| cfg.Plugins, | ||
| cfg.SchedulingProfiles, | ||
| cfg.Data, | ||
| cfg.SaturationDetector, | ||
| cfg.FlowControl, | ||
| cfg.RequestControl, | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -183,6 +188,24 @@ type SaturationDetector struct { | |
| MetricsStalenessThreshold metav1.Duration `json:"metricsStalenessThreshold,omitempty"` | ||
| } | ||
|
|
||
| // RequestControlConfig configures the request control stage. | ||
| type RequestControlConfig struct { | ||
| // +optional | ||
| // PrepareDataTimeout defines the timeout for PrepareData plugins. | ||
|
tsj-30 marked this conversation as resolved.
|
||
| // If omitted, defaults to 400ms. | ||
| PrepareDataTimeout *metav1.Duration `json:"prepareDataTimeout,omitempty"` | ||
| } | ||
|
|
||
| func (rc *RequestControlConfig) String() string { | ||
| if rc == nil { | ||
| return "{}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is misleading since there is a default value for the parameters.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually disagree. The use of the String() function here is in logging of the configuration. The configuration gets logged twice during startup. Once before the defaults are applied and again after the defaults are applied. The default value for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we default earlier in the right places, then I agree. My point as implemented it was not reflecting that we are applying a default. |
||
| } | ||
| if rc.PrepareDataTimeout == nil { | ||
| return "{PrepareDataTimeout: default(400ms)}" | ||
| } | ||
| return "{PrepareDataTimeout: " + rc.PrepareDataTimeout.String() + "}" | ||
| } | ||
|
|
||
| func (sd *SaturationDetector) String() string { | ||
| result := "" | ||
| if sd != nil { | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| /* | ||
|
tsj-30 marked this conversation as resolved.
|
||
| Copyright 2025 The Kubernetes Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package runner | ||
|
|
||
| import ( | ||
| "context" | ||
| "reflect" | ||
| "testing" | ||
| "time" | ||
|
|
||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
|
||
| configapi "sigs.k8s.io/gateway-api-inference-extension/apix/config/v1alpha1" | ||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/datalayer" | ||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/datastore" | ||
| ) | ||
|
|
||
| func TestParseConfigurationPhaseTwoAppliesPrepareDataTimeout(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| r := NewRunner() | ||
| r.registerInTreePlugins() | ||
|
|
||
| rawConfig := &configapi.EndpointPickerConfig{ | ||
| RequestControl: &configapi.RequestControlConfig{ | ||
| PrepareDataTimeout: &metav1.Duration{Duration: 125 * time.Millisecond}, | ||
| }, | ||
| } | ||
|
|
||
| ctx := context.Background() | ||
| epFactory := datalayer.NewEndpointFactory(nil, time.Millisecond) | ||
| ds := datastore.NewDatastore(ctx, epFactory, 0) | ||
|
|
||
| if _, err := r.parseConfigurationPhaseTwo(ctx, rawConfig, ds); err != nil { | ||
| t.Fatalf("parseConfigurationPhaseTwo failed: %v", err) | ||
| } | ||
|
|
||
| got := readPrepareDataTimeout(t, r.requestControlConfig) | ||
| want := 125 * time.Millisecond | ||
| if got != want { | ||
| t.Fatalf("prepareDataTimeout = %v, want %v", got, want) | ||
| } | ||
| } | ||
|
|
||
| func readPrepareDataTimeout(t *testing.T, cfg any) time.Duration { | ||
| t.Helper() | ||
|
|
||
| v := reflect.ValueOf(cfg).Elem().FieldByName("prepareDataTimeout") | ||
| if v.Kind() != reflect.Int64 { | ||
| t.Fatalf("unexpected kind for prepareDataTimeout: %v", v.Kind()) | ||
| } | ||
| return time.Duration(v.Int()) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,10 +17,14 @@ limitations under the License. | |
| package requestcontrol | ||
|
|
||
| import ( | ||
| "time" | ||
|
|
||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/framework/interface/plugin" | ||
| fwk "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/framework/interface/requestcontrol" | ||
| ) | ||
|
|
||
| const defaultPrepareDataTimeout = 400 * time.Millisecond | ||
|
|
||
| // NewConfig creates a new Config object and returns its pointer. | ||
| func NewConfig() *Config { | ||
| return &Config{ | ||
|
|
@@ -30,6 +34,7 @@ func NewConfig() *Config { | |
| responseReceivedPlugins: []fwk.ResponseReceived{}, | ||
| responseStreamingPlugins: []fwk.ResponseStreaming{}, | ||
| responseCompletePlugins: []fwk.ResponseComplete{}, | ||
| prepareDataTimeout: defaultPrepareDataTimeout, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -41,6 +46,7 @@ type Config struct { | |
| responseReceivedPlugins []fwk.ResponseReceived | ||
| responseStreamingPlugins []fwk.ResponseStreaming | ||
| responseCompletePlugins []fwk.ResponseComplete | ||
| prepareDataTimeout time.Duration | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the default timeout shouldn't be set here. Instead it should be set in This includes being properly set in the configuration. used when none was specified. |
||
| } | ||
|
|
||
| // WithPreRequestPlugins sets the given plugins as the PreRequest plugins. | ||
|
|
@@ -77,6 +83,12 @@ func (c *Config) WithPrepareDataPlugins(plugins ...fwk.PrepareDataPlugin) *Confi | |
| return c | ||
| } | ||
|
|
||
| // WithPrepareDataTimeout sets the timeout for PrepareData plugins. | ||
| func (c *Config) WithPrepareDataTimeout(timeout time.Duration) *Config { | ||
| c.prepareDataTimeout = timeout | ||
| return c | ||
| } | ||
|
|
||
| // WithAdmissionPlugins sets the given plugins as the AdmitRequest plugins. | ||
| func (c *Config) WithAdmissionPlugins(plugins ...fwk.AdmissionPlugin) *Config { | ||
| c.admissionPlugins = plugins | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.