Skip to content

Commit 0e730a3

Browse files
committed
feat(plugin-state): Allow customizing staleness threshold
In decode heavy with non-streaming, it is fairly common that a request may take over 5 minutes (default staleness threshold), which causes plugin state to remove those from accounting, when used with inflight flow control this results in prematurly releasing request or token budget which results in EPP admitting more request than configured. This PR allows us to customize this staleness threshold, it adds a new epp command line argument `--plugin-state-staleness-threshold` defaulting to 5m. Signed-off-by: mohammadkhan <mohammadkhan@digitalocean.com>
1 parent 08dce7a commit 0e730a3

4 files changed

Lines changed: 75 additions & 13 deletions

File tree

cmd/epp/runner/runner.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ func (r *Runner) Run(ctx context.Context) error {
222222
return err
223223
}
224224

225+
// Apply the process-wide plugin-state staleness threshold before any plugin is instantiated.
226+
fwkplugin.SetDefaultStalenessThreshold(opts.PluginStateStalenessThreshold)
227+
225228
// Print flag values, skipping deprecated metric flags configured via engineConfigs
226229
flags := make(map[string]any)
227230
pflag.VisitAll(func(f *pflag.Flag) {

pkg/epp/framework/interface/plugin/plugin_state.go

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,32 @@ import (
2828
)
2929

3030
const (
31-
// stalenessThreshold defines the threshold for considering data as stale.
32-
// if data of a request hasn't been read/write in the last "stalenessThreshold", it is considered as stale data
33-
// and will be cleaned in the next cleanup cycle.
34-
stalenessThreshold = time.Minute * 5
31+
// DefaultStalenessThreshold is the built-in threshold for considering data as stale: if a
32+
// request's data has not been read/written within this duration it is reaped in the next cleanup
33+
// cycle. It applies when no process-wide (SetDefaultStalenessThreshold) override is set.
34+
DefaultStalenessThreshold = time.Minute * 5
3535
// cleanupInterval defines the periodic interval that the cleanup go routine uses to check for stale data.
3636
cleanupInterval = time.Minute
3737
)
3838

39-
// NewPluginState initializes a new PluginState and returns its pointer.
39+
// defaultStalenessThreshold is the process-wide default applied to PluginState instances.
40+
// It may be overridden once during startup via SetDefaultStalenessThreshold.
41+
var defaultStalenessThreshold = DefaultStalenessThreshold
42+
43+
// SetDefaultStalenessThreshold overrides the process-wide default staleness threshold used by
44+
// PluginState instances that do not set an explicit WithStalenessThreshold. It is intended to be
45+
// called once at startup, before any plugin is instantiated. Non-positive values are ignored.
46+
func SetDefaultStalenessThreshold(d time.Duration) {
47+
if d > 0 {
48+
defaultStalenessThreshold = d
49+
}
50+
}
51+
52+
// NewPluginState initializes a new PluginState and returns its pointer. Each instance captures the
53+
// process-wide staleness threshold at construction; configure it via SetDefaultStalenessThreshold
54+
// before instantiating plugins.
4055
func NewPluginState(ctx context.Context) *PluginState {
41-
pluginState := &PluginState{}
56+
pluginState := &PluginState{stalenessThreshold: defaultStalenessThreshold}
4257
go pluginState.cleanup(ctx)
4358
return pluginState
4459
}
@@ -60,6 +75,8 @@ type PluginState struct {
6075
storage sync.Map
6176
// key: RequestID, value: time.Time
6277
requestToLastAccessTime sync.Map
78+
// stalenessThreshold is the inactivity duration after which a request's data is reaped.
79+
stalenessThreshold time.Duration
6380
}
6481

6582
// Read retrieves data with the given "key" in the context of "requestID" from PluginState.
@@ -167,14 +184,14 @@ func (s *PluginState) cleanup(ctx context.Context) {
167184
}
168185

169186
// cleanStaleRequests iterates through all requests and removes those that haven't been
170-
// accessed for longer than stalenessThreshold. This operation is safe to run concurrently
187+
// accessed for longer than the staleness threshold. This operation is safe to run concurrently
171188
// with other operations on the PluginState.
172189
func (s *PluginState) cleanStaleRequests() {
173190
s.requestToLastAccessTime.Range(func(k, v any) bool {
174191
requestID := k.(string)
175192
lastAccessTime := v.(time.Time)
176-
if time.Since(lastAccessTime) > stalenessThreshold {
177-
log.Log.V(logutil.DEBUG).Info("Cleaning up stale request from PluginState", "requestID", requestID, "lastAccessTime", lastAccessTime)
193+
if time.Since(lastAccessTime) > s.stalenessThreshold {
194+
log.Log.V(logutil.DEBUG).Info("Cleaning up stale request from PluginState", "requestID", requestID, "lastAccessTime", lastAccessTime, "stalenessThreshold", s.stalenessThreshold)
178195
s.Delete(requestID) // cleanup stale requests (this is safe in sync.Map)
179196
}
180197
return true

pkg/epp/framework/interface/plugin/plugin_state_test.go

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func TestPluginState_EvictionCallback(t *testing.T) {
109109
data.evictedID = ""
110110
data.evictedKey = ""
111111
state.Write(requestID, key, data)
112-
state.requestToLastAccessTime.Store(requestID, time.Now().Add(-2*stalenessThreshold))
112+
state.requestToLastAccessTime.Store(requestID, time.Now().Add(-2*defaultStalenessThreshold))
113113
state.cleanStaleRequests()
114114
assert.Equal(t, requestID, data.evictedID)
115115
assert.Equal(t, key, data.evictedKey)
@@ -128,7 +128,7 @@ func TestPluginState_Touch(t *testing.T) {
128128
state.Write(requestID, key, data)
129129

130130
// Set last access time to near-stale
131-
nearStale := time.Now().Add(-stalenessThreshold + time.Second*10)
131+
nearStale := time.Now().Add(-defaultStalenessThreshold + time.Second*10)
132132
state.requestToLastAccessTime.Store(requestID, nearStale)
133133

134134
// Touch it
@@ -220,7 +220,7 @@ func TestReadPluginStateKey(t *testing.T) {
220220
}
221221

222222
// TestPluginState_Cleanup verifies the automatic cleanup of stale data.
223-
// It tests that data which hasn't been accessed for longer than stalenessThreshold
223+
// It tests that data which hasn't been accessed for longer than the staleness threshold
224224
// is properly removed from the storage.
225225
func TestPluginState_Cleanup(t *testing.T) {
226226
ctx, cancel := context.WithCancel(logutil.NewTestLoggerIntoContext(context.Background()))
@@ -235,14 +235,42 @@ func TestPluginState_Cleanup(t *testing.T) {
235235
state.Write(requestID, key, data)
236236

237237
// Manually set last access time to far in the past
238-
state.requestToLastAccessTime.Store(requestID, time.Now().Add(-2*stalenessThreshold))
238+
state.requestToLastAccessTime.Store(requestID, time.Now().Add(-2*defaultStalenessThreshold))
239239
// Manually CleanUp
240240
state.cleanStaleRequests()
241241

242242
_, err := state.Read(requestID, key)
243243
assert.Equal(t, ErrNotFound, err)
244244
}
245245

246+
// TestSetDefaultStalenessThreshold verifies that the process-wide default is applied to new
247+
// PluginState instances, that a configured value drives reaping, and that non-positive values are
248+
// ignored.
249+
func TestSetDefaultStalenessThreshold(t *testing.T) {
250+
orig := defaultStalenessThreshold
251+
t.Cleanup(func() { defaultStalenessThreshold = orig })
252+
253+
SetDefaultStalenessThreshold(30 * time.Second)
254+
assert.Equal(t, 30*time.Second, defaultStalenessThreshold)
255+
256+
ctx, cancel := context.WithCancel(logutil.NewTestLoggerIntoContext(context.Background()))
257+
t.Cleanup(cancel)
258+
state := NewPluginState(ctx)
259+
assert.Equal(t, 30*time.Second, state.stalenessThreshold, "new state should inherit the configured default")
260+
261+
// A one-minute-old request is stale under the 30s configured threshold (but would survive the 5m built-in default).
262+
requestID := "req-configured-threshold"
263+
key := StateKey("foo")
264+
state.Write(requestID, key, &pluginTestData{value: "bar"})
265+
state.requestToLastAccessTime.Store(requestID, time.Now().Add(-time.Minute))
266+
state.cleanStaleRequests()
267+
_, err := state.Read(requestID, key)
268+
assert.Equal(t, ErrNotFound, err, "request older than the configured threshold should be reaped")
269+
270+
SetDefaultStalenessThreshold(0)
271+
assert.Equal(t, 30*time.Second, defaultStalenessThreshold, "non-positive value must be ignored")
272+
}
273+
246274
// TestPluginState_DeleteKey verifies that DeleteKey correctly removes only the specified key for a request.
247275
func TestPluginState_DeleteKey(t *testing.T) {
248276
ctx, cancel := context.WithCancel(logutil.NewTestLoggerIntoContext(context.Background()))

pkg/epp/server/options.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535

3636
"github.com/llm-d/llm-d-router/pkg/common/observability/logging"
3737
"github.com/llm-d/llm-d-router/pkg/common/routing"
38+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
3839
)
3940

4041
const (
@@ -95,6 +96,10 @@ type Options struct {
9596
LoRAInfoMetric string // Prometheus metric specification for the LoRA info metrics.
9697
CacheInfoMetric string // Prometheus metric specification for the cache info metrics.
9798
//
99+
// Plugin state.
100+
//
101+
PluginStateStalenessThreshold time.Duration // Inactivity duration after which a request's plugin state is reaped.
102+
//
98103
// Diagnostics.
99104
//
100105
logging.LoggingOptions // Logging configuration.
@@ -137,6 +142,7 @@ func NewOptions() *Options {
137142
KVCacheUsagePercentageMetric: "vllm:kv_cache_usage_perc",
138143
LoRAInfoMetric: "vllm:lora_requests_info",
139144
CacheInfoMetric: "vllm:cache_config_info",
145+
PluginStateStalenessThreshold: fwkplugin.DefaultStalenessThreshold,
140146
LoggingOptions: *logging.NewOptions(),
141147
Tracing: true,
142148
MetricsPort: 9090,
@@ -195,6 +201,10 @@ func (opts *Options) AddFlags(fs *pflag.FlagSet) {
195201
fs.StringVar(&opts.CacheInfoMetric, "cache-info-metric", opts.CacheInfoMetric, "Prometheus metric for the cache info metrics.")
196202
_ = fs.MarkDeprecated("cache-info-metric", "use engineConfigs in EndpointPickerConfig instead")
197203

204+
fs.DurationVar(&opts.PluginStateStalenessThreshold, "plugin-state-staleness-threshold", opts.PluginStateStalenessThreshold,
205+
"Inactivity duration after which a request's plugin state (e.g. in-flight load accounting) is reaped. "+
206+
"Set this above the maximum expected request duration to avoid releasing in-flight accounting for long-running requests.")
207+
198208
opts.LoggingOptions.AddFlags(fs) // Add logging flags.
199209

200210
fs.BoolVar(&opts.Tracing, "tracing", opts.Tracing, "Enables emitting traces.")
@@ -360,6 +370,10 @@ func (opts *Options) Validate() error {
360370
return fmt.Errorf("grpc-max-send-msg-size must be non-negative, got %d", opts.GRPCMaxSendMsgSize)
361371
}
362372

373+
if opts.PluginStateStalenessThreshold <= 0 {
374+
return fmt.Errorf("plugin-state-staleness-threshold must be positive, got %v", opts.PluginStateStalenessThreshold)
375+
}
376+
363377
// Validate deprecated metric flags are not explicitly set
364378
for flagName := range deprecatedMetricFlags {
365379
if f := opts.fs.Lookup(flagName); f != nil && f.Changed {

0 commit comments

Comments
 (0)