Skip to content

Commit 62b18e1

Browse files
committed
fix: correct the flow control deprecation notice and tuning guidance
ENABLE_EXPERIMENTAL_FLOW_CONTROL_LAYER=false no longer disables the layer: the variable only ever appended the gate, so a false value falls through to the registered default, which is now enabled. Log the observed value and the featureGates opt-out so the operator who set it can see it is ignored. The 5000 / 1G defaults belong to priorityBands, not to the global flowControl caps, which default to unlimited; sizing advice that names the global knob sends operators to the wrong field and implies a bound that is not configured. The priority-holdback policy lowers admission ceilings, holding gated traffic in queue. It is not a shedding knob, and listing it beside the two that do shed reads as though it were. Also assert the registered gate default, which nothing covered, and the silent cases of the ignored-settings warning, which guard the saturationDetector exclusion that keeps it from firing on every legacy-path startup. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
1 parent 3676671 commit 62b18e1

6 files changed

Lines changed: 108 additions & 21 deletions

File tree

cmd/epp/runner/feature_gate_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ featureGates:
127127
wantEnabled = *tc.wantEnabled
128128
require.Equal(t, wantEnabled, r.featureGates[flowcontrol.FeatureGate],
129129
"the loader should honor the explicit featureGates stanza")
130+
} else {
131+
require.True(t, wantEnabled, "the flowControl gate is registered as enabled by default (#2104)")
130132
}
131133

132134
ds := datastore.NewDatastore(ctx, r.setupMetricsCollection(opts))

cmd/epp/runner/runner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ func (r *Runner) parseConfigurationPhaseTwo(ctx context.Context, rawConfig *conf
766766

767767
func applyDeprecatedEnvFeatureGate(envVar, featureName, featureGate string, rawConfig *configapi.EndpointPickerConfig) {
768768
if _, ok := os.LookupEnv(envVar); ok {
769-
setupLog.Info(fmt.Sprintf("The %s environment variable is deprecated and will be removed in a future release; the %s is enabled by default and is configured via featureGates in the config file", envVar, featureName))
769+
setupLog.Info(fmt.Sprintf("The %s environment variable (current value %q) is deprecated and will be removed in a future release; the %s is enabled by default, so this variable no longer disables it. To disable it, set featureGates: [%q] in the config file", envVar, os.Getenv(envVar), featureName, featureGate+"=false"))
770770
if env.GetEnvBool(envVar, false, setupLog) {
771771
if rawConfig.FeatureGates == nil {
772772
rawConfig.FeatureGates = make(configapi.FeatureGates, 0)

docs/operations.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,13 @@ The EPP acts as the routing intelligence engine. Its resource usage scales prima
2121
- **Inflight Requests Impact**: Memory usage increases with the number of concurrent inflight requests and the output (decode) token length.
2222
- **Flow Control Queues**: With flow control enabled (the default), requests that cannot dispatch
2323
under saturation are buffered in EPP memory, including their request bodies. The buffered volume
24-
is bounded per priority band by `flowControl` `maxRequests` (default 5000) and `maxBytes`
25-
(default 1G); budget for the sum of the per-band `maxBytes` limits of the priority levels your
26-
traffic actually uses, on top of the inflight-request sizing above. Lower these limits (or set a
27-
shorter `defaultRequestTTL`) to trade queueing for earlier shedding.
24+
is bounded per priority band by `priorityBands[].maxRequests` (default 5000) and `maxBytes`
25+
(default 1G), which `defaultPriorityBand` sets as a template for bands you do not list; budget for
26+
the sum of the per-band `maxBytes` limits of the priority levels your traffic actually uses, on top
27+
of the inflight-request sizing above. The global `flowControl.maxRequests` / `maxBytes` caps
28+
default to unlimited, so set a global `maxBytes` under the container memory limit: at the per-band
29+
default, a handful of bands clears the sizing guidance below before any band cap engages. Lower
30+
these limits (or set a shorter `defaultRequestTTL`) to trade queueing for earlier shedding.
2831
- **Sizing Guidelines**:
2932
- For a request rate of 50 to 100 requests/second with 1k output tokens, EPP requires between **4 GiB and 6 GiB** of memory.
3033
- For workloads with longer output lengths (such as 5k output tokens), memory usage can reach **20+ GiB** due to the accumulation of state for concurrent inflight requests.

pkg/epp/config/loader/configloader_test.go

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -851,24 +851,68 @@ func TestInstantiateAndConfigure(t *testing.T) {
851851
}
852852

853853
// TestFlowControlConfigIgnoredWarning verifies that a flowControl config section combined with a
854-
// disabled flowControl feature gate logs a warning that the settings are ignored.
854+
// disabled flowControl feature gate logs a warning that the settings are ignored, and that the
855+
// warning stays silent otherwise. The silent cases carry the weight here: ensureSaturationDetector
856+
// populates FlowControl for every config, so a predicate that ignored its saturationDetector
857+
// exclusion would warn at every legacy-path startup.
855858
func TestFlowControlConfigIgnoredWarning(t *testing.T) {
856859
// Not parallel because it modifies the global plugin registry.
857860
registerTestPlugins(t)
858861
RegisterFeatureGate(flowcontrol.FeatureGate, true)
859862

860-
writer := &strings.Builder{}
861-
logger := logging.NewTestLoggerWithWriter(writer)
863+
testCases := []struct {
864+
name string
865+
configText string
866+
gateEnabled bool
867+
wantWarn bool
868+
}{
869+
{
870+
name: "settings ignored under an explicit opt-out",
871+
configText: successflowControlConfigDisabledText,
872+
wantWarn: true,
873+
},
874+
{
875+
name: "opt-out with no flowControl section",
876+
configText: successFlowControlDisabledNoSectionText,
877+
},
878+
{
879+
name: "opt-out with only a saturation detector",
880+
configText: successFlowControlDisabledSaturationDetectorText,
881+
},
882+
{
883+
name: "settings honored when the gate is on",
884+
configText: successFlowControlConfigText,
885+
gateEnabled: true,
886+
},
887+
}
862888

863-
rawConfig, _, err := LoadRawConfig([]byte(successflowControlConfigDisabledText), logger)
864-
require.NoError(t, err)
889+
for _, tc := range testCases {
890+
t.Run(tc.name, func(t *testing.T) {
891+
writer := &strings.Builder{}
892+
logger := logging.NewTestLoggerWithWriter(writer)
865893

866-
handle := testutils.NewTestHandle(context.Background())
867-
cfg, err := InstantiateAndConfigure(rawConfig, handle, logger)
868-
require.NoError(t, err)
869-
require.Nil(t, cfg.FlowControlConfig, "flow control config should not be built when the gate is disabled")
870-
require.Contains(t, writer.String(), "flowControl feature gate is disabled",
871-
"the ignored flowControl section should be called out in the logs")
894+
rawConfig, _, err := LoadRawConfig([]byte(tc.configText), logger)
895+
require.NoError(t, err)
896+
897+
handle := testutils.NewTestHandle(context.Background())
898+
cfg, err := InstantiateAndConfigure(rawConfig, handle, logger)
899+
require.NoError(t, err)
900+
901+
if tc.gateEnabled {
902+
require.NotNil(t, cfg.FlowControlConfig, "flow control config should be built when the gate is on")
903+
} else {
904+
require.Nil(t, cfg.FlowControlConfig, "flow control config should not be built when the gate is disabled")
905+
}
906+
907+
if tc.wantWarn {
908+
require.Contains(t, writer.String(), "flowControl feature gate is disabled",
909+
"the ignored flowControl section should be called out in the logs")
910+
} else {
911+
require.NotContains(t, writer.String(), "flowControl feature gate is disabled",
912+
"nothing is being ignored, so the warning should stay silent")
913+
}
914+
})
915+
}
872916
}
873917

874918
// TestBuildDataLayerConfigEmptySourcesWarning verifies that an empty sources list

pkg/epp/config/loader/testdata_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,38 @@ flowControl:
279279
maxBytes: "1024"
280280
`
281281

282+
// successFlowControlDisabledNoSectionText is the plain legacy-path config: an explicit opt-out with
283+
// nothing for the loader to report as ignored.
284+
const successFlowControlDisabledNoSectionText = `
285+
apiVersion: llm-d.ai/v1alpha1
286+
kind: EndpointPickerConfig
287+
plugins:
288+
- name: maxScore
289+
type: max-score-picker
290+
schedulingProfiles:
291+
- name: default
292+
plugins:
293+
- pluginRef: maxScore
294+
featureGates: ["flowControl=false"]
295+
`
296+
297+
// successFlowControlDisabledSaturationDetectorText pairs an explicit opt-out with a saturation
298+
// detector, which the legacy admission path honors and so must not be reported as ignored.
299+
const successFlowControlDisabledSaturationDetectorText = `
300+
apiVersion: llm-d.ai/v1alpha1
301+
kind: EndpointPickerConfig
302+
plugins:
303+
- name: maxScore
304+
type: max-score-picker
305+
schedulingProfiles:
306+
- name: default
307+
plugins:
308+
- pluginRef: maxScore
309+
featureGates: ["flowControl=false"]
310+
saturationDetector:
311+
pluginRef: utilization-detector
312+
`
313+
282314
// successComplexFlowControlConfigText tests that Flow Control configuration with custom plugins is correctly loaded.
283315
const successComplexFlowControlConfigText = `
284316
apiVersion: llm-d.ai/v1alpha1

pkg/epp/flowcontrol/README.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,17 @@ threshold and monitor scrape health when running with flow control on.
5656

5757
Tuning knobs, all under the `flowControl:` config section:
5858

59-
* Per-band `maxRequests` / `maxBytes` — lower these to shed excess load earlier instead of queueing
60-
it (for example, to approximate the legacy immediate-shed behavior for sheddable traffic).
61-
* `defaultRequestTTL` — bounds how long a request may wait in queue before it is evicted.
62-
* The priority-holdback usage-limit policy — gates low-priority traffic earlier as utilization
63-
rises; configure it via `usageLimitPolicyPluginRef`.
59+
* Per-band `maxRequests` / `maxBytes` — the shedding knobs. Lower them to reject excess load at the
60+
queue boundary instead of buffering it (for example, to approximate the legacy immediate-shed
61+
behavior for sheddable traffic).
62+
* `defaultRequestTTL` — the queue-wait budget, and the other way a request is shed. When the pool
63+
has no endpoints the queue acts as a scale-from-zero waiting room and requests hold for the full
64+
budget, so keep it under the client or gateway deadline unless you want requests to survive a cold
65+
start.
66+
* The priority-holdback usage-limit policy — a gating knob, not a shedding one. It lowers the
67+
admission ceiling for low-priority traffic as utilization rises, so that traffic waits in queue
68+
rather than being rejected; it sheds only by way of the two limits above. Configure it via
69+
`usageLimitPolicyPluginRef`.
6470

6571
### High Level Architecture
6672

0 commit comments

Comments
 (0)