Skip to content

Commit 8697fa9

Browse files
committed
perf(epp): derive Scope allowed-key sets at startup registration
Scope looks up a spec registered by RegisterScopeSpecs, keyed by the plugin's typed name, instead of caching lazily by plugin identity. The runner registers every plugin once the full set, including auto-created producers, is known. An unregistered plugin is confined to nothing and the miss is logged once per typed name. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
1 parent 843d227 commit 8697fa9

10 files changed

Lines changed: 127 additions & 38 deletions

File tree

cmd/epp/runner/runner.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,11 @@ func (r *Runner) parseConfigurationPhaseTwo(ctx context.Context, rawConfig *conf
823823
// The plugins will be executed in topologically sorted order to ensure that data is produced before it is consumed.
824824
r.requestControlConfig.OrderDataProducerPlugins(dag)
825825

826+
// Derive the endpoint-scope allowed-key sets while the full plugin set,
827+
// including auto-created producers, is known. A plugin missing here is
828+
// confined to nothing at request time.
829+
datalayer.RegisterScopeSpecs(handle.GetAllPlugins())
830+
826831
r.parserRegistry = cfg.ParserRegistry
827832
logger.Info("loaded configuration from file/text successfully")
828833

pkg/epp/datalayer/endpoint_scope.go

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -190,25 +190,44 @@ func (e *ScopedEndpoint) Clone() fwkdl.AttributeMap {
190190
}
191191

192192
// scopeSpec holds the allowed-key sets derived from a plugin's declarations.
193-
// The maps are built once per plugin and shared read-only by every ScopedEndpoint
194-
// wrapper across all subsequent invocations.
193+
// The maps are shared read-only by every ScopedEndpoint wrapper across all
194+
// invocations.
195195
type scopeSpec struct {
196196
allowedPut map[fwkplugin.DataKey]struct{}
197197
allowedGet map[fwkplugin.DataKey]struct{}
198198
}
199199

200-
// scopeSpecs caches one scopeSpec per plugin instance, keyed by identity.
201-
// Produces() and Consumes() are source-level declarations fixed at plugin
202-
// construction, so the derived sets never change, while Scope runs for every
203-
// filter, scorer, and DataProducer on every request. Plugin instances live for
204-
// the process lifetime, so entries are never evicted.
205-
var scopeSpecs sync.Map // fwkplugin.Plugin -> *scopeSpec
200+
// denyAllSpec confines an unregistered plugin the same way as one that
201+
// declares nothing.
202+
var denyAllSpec = &scopeSpec{}
203+
204+
// scopeSpecs maps a plugin's TypedName().String() to the spec derived from its
205+
// declarations. RegisterScopeSpecs writes it during startup; Scope only reads.
206+
// ValidateAndOrderDataDependencies keys plugins the same way, so within the
207+
// datalayer contract the typed name identifies the plugin.
208+
var (
209+
scopeSpecsMu sync.RWMutex
210+
scopeSpecs = map[string]*scopeSpec{}
211+
212+
// unregisteredReported dedups the error log for plugins missing from
213+
// scopeSpecs, which Scope would otherwise emit on every invocation.
214+
unregisteredReported sync.Map // string -> struct{}
215+
)
206216

207-
func scopeSpecFor(plugin fwkplugin.Plugin) *scopeSpec {
208-
if cached, ok := scopeSpecs.Load(plugin); ok {
209-
return cached.(*scopeSpec)
217+
// RegisterScopeSpecs derives the allowed-key sets from each plugin's
218+
// Produces() and Consumes() declarations and stores them for Scope to look up.
219+
// Declarations are fixed at plugin construction, so the sets are derived once
220+
// here rather than on every Scope invocation. Call it after all plugins are
221+
// instantiated; registering a typed name again replaces its spec.
222+
func RegisterScopeSpecs(plugins []fwkplugin.Plugin) {
223+
scopeSpecsMu.Lock()
224+
defer scopeSpecsMu.Unlock()
225+
for _, plugin := range plugins {
226+
scopeSpecs[plugin.TypedName().String()] = buildScopeSpec(plugin)
210227
}
228+
}
211229

230+
func buildScopeSpec(plugin fwkplugin.Plugin) *scopeSpec {
212231
produces := map[fwkplugin.DataKey]any{}
213232
if producer, ok := plugin.(fwkplugin.ProducerPlugin); ok {
214233
produces = producer.Produces()
@@ -231,9 +250,25 @@ func scopeSpecFor(plugin fwkplugin.Plugin) *scopeSpec {
231250
spec.allowedGet[key] = struct{}{}
232251
}
233252
}
253+
return spec
254+
}
234255

235-
cached, _ := scopeSpecs.LoadOrStore(plugin, spec)
236-
return cached.(*scopeSpec)
256+
// scopeSpecFor returns the registered spec for a plugin's typed name, or the
257+
// deny-all spec when none was registered. The miss is logged once per typed
258+
// name: it indicates a wiring bug, and the resulting confinement also shows up
259+
// through the violation counter as soon as the plugin touches an attribute.
260+
func scopeSpecFor(logger logr.Logger, name string) *scopeSpec {
261+
scopeSpecsMu.RLock()
262+
spec, ok := scopeSpecs[name]
263+
scopeSpecsMu.RUnlock()
264+
if ok {
265+
return spec
266+
}
267+
if _, reported := unregisteredReported.LoadOrStore(name, struct{}{}); !reported {
268+
logger.Error(fmt.Errorf("plugin %q has no registered scope spec; pass it to RegisterScopeSpecs", name),
269+
"Confining an unregistered plugin to nothing")
270+
}
271+
return denyAllSpec
237272
}
238273

239274
// Scope confines endpoints to what plugin declares. extensionPoint labels the
@@ -244,15 +279,14 @@ func scopeSpecFor(plugin fwkplugin.Plugin) *scopeSpec {
244279
// A plugin that declares nothing reaches nothing: an absent declaration is a
245280
// statement that the plugin exchanges no data, not a request to be exempt.
246281
//
247-
// The allowed-key sets are cached per plugin instance on first use, treating
248-
// Produces() and Consumes() as immutable after construction. Plugins must be
249-
// usable as map keys; the pointer-shaped plugins the framework's constructors
250-
// return always are.
282+
// The allowed-key sets come from the registry populated by RegisterScopeSpecs
283+
// at startup, treating Produces() and Consumes() as immutable after
284+
// construction. A plugin that was never registered is confined to nothing.
251285
func Scope(logger logr.Logger, extensionPoint string, plugin fwkplugin.Plugin, endpoints []fwksched.Endpoint) ([]fwksched.Endpoint, *Violations) {
252-
spec := scopeSpecFor(plugin)
286+
typedName := plugin.TypedName()
287+
spec := scopeSpecFor(logger, typedName.String())
253288

254289
violations := &Violations{}
255-
typedName := plugin.TypedName()
256290
// One backing array rather than an allocation per endpoint: this runs for
257291
// every filter and scorer on every request, over the whole candidate set.
258292
wrappers := make([]ScopedEndpoint, len(endpoints))

pkg/epp/datalayer/endpoint_scope_bench_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func benchPlugin() fwkplugin.Plugin {
4545
p := &producerConsumerPlugin{}
4646
p.produces = map[fwkplugin.DataKey]any{producedKey: nil}
4747
p.consumes = &fwkplugin.DataDependencies{Optional: map[fwkplugin.DataKey]any{consumedKey: nil}}
48+
RegisterScopeSpecs([]fwkplugin.Plugin{p})
4849
return p
4950
}
5051

pkg/epp/datalayer/endpoint_scope_test.go

Lines changed: 49 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,21 @@ var (
4242
)
4343

4444
// testPlugin implements Plugin plus whichever of Produces/Consumes the test
45-
// populates, mirroring how real plugins opt into each role.
45+
// populates, mirroring how real plugins opt into each role. The scope registry
46+
// is keyed by typed name, so tests that must not see another test's spec set a
47+
// distinct name.
4648
type testPlugin struct {
49+
name string
4750
produces map[fwkplugin.DataKey]any
4851
consumes *fwkplugin.DataDependencies
4952
}
5053

5154
func (p *testPlugin) TypedName() fwkplugin.TypedName {
52-
return fwkplugin.TypedName{Type: "testPlugin", Name: "mock"}
55+
name := p.name
56+
if name == "" {
57+
name = "mock"
58+
}
59+
return fwkplugin.TypedName{Type: "testPlugin", Name: name}
5360
}
5461

5562
type producerPlugin struct{ testPlugin }
@@ -79,6 +86,7 @@ func scopeProducerConsumer(t *testing.T, endpoint fwksched.Endpoint) (fwksched.E
7986
plug := &producerConsumerPlugin{}
8087
plug.produces = map[fwkplugin.DataKey]any{producedKey: nil}
8188
plug.consumes = &fwkplugin.DataDependencies{Optional: map[fwkplugin.DataKey]any{consumedKey: nil}}
89+
RegisterScopeSpecs([]fwkplugin.Plugin{plug})
8290
scoped, violation := Scope(testLogger(), "test-extension-point", plug, []fwksched.Endpoint{endpoint})
8391
require.Len(t, scoped, 1)
8492
return scoped[0], violation
@@ -115,6 +123,7 @@ func TestScope_ViolationsAreSharedAndReportTheFirstWrite(t *testing.T) {
115123

116124
plug := &producerPlugin{}
117125
plug.produces = map[fwkplugin.DataKey]any{producedKey: nil}
126+
RegisterScopeSpecs([]fwkplugin.Plugin{plug})
118127
scoped, violations := Scope(testLogger(), "test-extension-point", plug, endpoints)
119128

120129
first := fwkplugin.NewDataKey("first-undeclared", "otherPlugin")
@@ -162,7 +171,9 @@ func TestScope_GetHonoursConsumesAndProduces(t *testing.T) {
162171
// the case that previously fell through to unrestricted reads.
163172
func TestScope_PluginDeclaringNothingReachesNothing(t *testing.T) {
164173
endpoint := newEndpoint(t)
165-
scoped, violation := Scope(testLogger(), "test-extension-point", &testPlugin{}, []fwksched.Endpoint{endpoint})
174+
plug := &testPlugin{}
175+
RegisterScopeSpecs([]fwkplugin.Plugin{plug})
176+
scoped, violation := Scope(testLogger(), "test-extension-point", plug, []fwksched.Endpoint{endpoint})
166177
require.Len(t, scoped, 1)
167178

168179
_, ok := scoped[0].Get(consumedKey)
@@ -180,6 +191,7 @@ func TestScope_ProducerWithoutConsumesReadsOnlyItsOwnOutput(t *testing.T) {
180191
endpoint := newEndpoint(t)
181192
plug := &producerPlugin{}
182193
plug.produces = map[fwkplugin.DataKey]any{producedKey: nil}
194+
RegisterScopeSpecs([]fwkplugin.Plugin{plug})
183195

184196
scoped, _ := Scope(testLogger(), "test-extension-point", plug, []fwksched.Endpoint{endpoint})
185197

@@ -248,24 +260,42 @@ func TestUnscope_LeavesUnwrappedEndpointsAlone(t *testing.T) {
248260
assert.Equal(t, endpoints, Unscope(endpoints))
249261
}
250262

251-
func TestScope_AllowedKeySetsAreCachedPerPluginInstance(t *testing.T) {
252-
plugA := &producerConsumerPlugin{}
253-
plugA.produces = map[fwkplugin.DataKey]any{producedKey: nil}
254-
plugA.consumes = &fwkplugin.DataDependencies{Optional: map[fwkplugin.DataKey]any{consumedKey: nil}}
255-
assert.Same(t, scopeSpecFor(plugA), scopeSpecFor(plugA), "same plugin instance must share one cached spec")
263+
// Declarations grant nothing on their own: the allowed sets come from the
264+
// registry, and a plugin nobody registered is confined like one that declares
265+
// nothing.
266+
func TestScope_UnregisteredPluginIsConfinedToNothing(t *testing.T) {
267+
plug := &producerConsumerPlugin{}
268+
plug.name = "never-registered"
269+
plug.produces = map[fwkplugin.DataKey]any{producedKey: nil}
270+
plug.consumes = &fwkplugin.DataDependencies{Optional: map[fwkplugin.DataKey]any{consumedKey: nil}}
256271

257-
plugB := &producerConsumerPlugin{}
258-
plugB.produces = map[fwkplugin.DataKey]any{producedKey: nil}
259-
plugB.consumes = &fwkplugin.DataDependencies{}
260-
assert.NotSame(t, scopeSpecFor(plugA), scopeSpecFor(plugB), "distinct instances must not share a spec")
272+
endpoint := newEndpoint(t)
273+
scoped, violations := Scope(testLogger(), "test-extension-point", plug, []fwksched.Endpoint{endpoint})
274+
require.Len(t, scoped, 1)
275+
276+
_, ok := scoped[0].Get(consumedKey)
277+
assert.False(t, ok, "a declared read must not resolve without registration")
278+
scoped[0].Put(producedKey, cloneableStr("nope"))
279+
_, ok = endpoint.Get(producedKey)
280+
assert.False(t, ok, "a declared write must not reach the endpoint without registration")
281+
assert.Error(t, violations.Write())
282+
}
283+
284+
// The registry is keyed by typed name; registering a name again replaces its
285+
// spec rather than accumulating.
286+
func TestRegisterScopeSpecs_ReregistrationReplacesTheSpec(t *testing.T) {
287+
declaring := &producerConsumerPlugin{}
288+
declaring.name = "replaced"
289+
declaring.produces = map[fwkplugin.DataKey]any{producedKey: nil}
290+
declaring.consumes = &fwkplugin.DataDependencies{Optional: map[fwkplugin.DataKey]any{consumedKey: nil}}
291+
RegisterScopeSpecs([]fwkplugin.Plugin{declaring})
292+
293+
silent := &testPlugin{name: "replaced"}
294+
RegisterScopeSpecs([]fwkplugin.Plugin{silent})
261295

262-
// Confinement is unchanged when the spec comes from the cache.
263296
endpoint := newEndpoint(t)
264-
scoped, _ := Scope(testLogger(), "test-extension-point", plugA, []fwksched.Endpoint{endpoint})
297+
scoped, _ := Scope(testLogger(), "test-extension-point", declaring, []fwksched.Endpoint{endpoint})
265298
require.Len(t, scoped, 1)
266-
_, ok := scoped[0].Get(undeclaredKey)
267-
assert.False(t, ok, "undeclared read must stay rejected on a cached spec")
268-
got, ok := scoped[0].Get(consumedKey)
269-
assert.True(t, ok)
270-
assert.Equal(t, cloneableStr("consumed-value"), got)
299+
_, ok := scoped[0].Get(consumedKey)
300+
assert.False(t, ok, "the registry must serve the last spec registered for a typed name")
271301
}

pkg/epp/framework/plugins/scheduling/profilehandler/disagg/scheduler_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import (
1212
k8stypes "k8s.io/apimachinery/pkg/types"
1313
"sigs.k8s.io/controller-runtime/pkg/log" // Import config for thresholds
1414

15+
"github.com/llm-d/llm-d-router/pkg/epp/datalayer"
1516
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
17+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
1618
fwkrh "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/requesthandling"
1719
fwksched "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/scheduling"
1820
attrprefix "github.com/llm-d/llm-d-router/pkg/epp/framework/plugins/datalayer/attribute/prefix"
@@ -224,6 +226,7 @@ func TestPDSchedule(t *testing.T) {
224226
// initialize scheduler with config
225227
prefixScorer, err := prefix.New(ctx, prefix.PrefixCacheScorerPluginType, "")
226228
assert.NoError(t, err, "Prefix plugin creation returned unexpected error")
229+
datalayer.RegisterScopeSpecs([]fwkplugin.Plugin{prefixScorer})
227230

228231
prefillSchedulerProfile := scheduling.NewSchedulerProfile().
229232
WithFilters(bylabel.NewPrefillRole()).

pkg/epp/requestcontrol/director_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,7 @@ func TestDirector_HandleRequest(t *testing.T) {
10711071
}
10721072
config := NewConfig()
10731073
if test.dataProducerPlugin != nil {
1074+
datalayer.RegisterScopeSpecs([]fwkplugin.Plugin{test.dataProducerPlugin})
10741075
config = config.WithDataProducerPlugins(test.dataProducerPlugin)
10751076
}
10761077
if test.screener != nil {

pkg/epp/requestcontrol/plugin_executor_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/stretchr/testify/assert"
2727
"github.com/stretchr/testify/require"
2828

29+
"github.com/llm-d/llm-d-router/pkg/epp/datalayer"
2930
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
3031
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
3132
fwkrc "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/requestcontrol"
@@ -428,6 +429,7 @@ func TestExecutePluginsAsDAG_EnforcesProducesDeclaration(t *testing.T) {
428429
t.Run("declared write reaches the endpoint", func(t *testing.T) {
429430
endpoint := newEndpoint()
430431
producer := &writingProducer{name: "p", declares: map[fwkplugin.DataKey]any{declared: nil}, writes: declared}
432+
datalayer.RegisterScopeSpecs([]fwkplugin.Plugin{producer})
431433

432434
err := executePluginsAsDAG(context.Background(), []fwkrc.DataProducer{producer},
433435
&fwksched.InferenceRequest{}, []fwksched.Endpoint{endpoint})
@@ -440,6 +442,7 @@ func TestExecutePluginsAsDAG_EnforcesProducesDeclaration(t *testing.T) {
440442
t.Run("undeclared write fails the producer and leaves the endpoint untouched", func(t *testing.T) {
441443
endpoint := newEndpoint()
442444
producer := &writingProducer{name: "p", declares: map[fwkplugin.DataKey]any{declared: nil}, writes: undeclared}
445+
datalayer.RegisterScopeSpecs([]fwkplugin.Plugin{producer})
443446

444447
err := executePluginsAsDAG(context.Background(), []fwkrc.DataProducer{producer},
445448
&fwksched.InferenceRequest{}, []fwksched.Endpoint{endpoint})

pkg/epp/scheduling/scheduler_bench_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ import (
2525
"github.com/google/uuid"
2626
k8stypes "k8s.io/apimachinery/pkg/types"
2727

28+
"github.com/llm-d/llm-d-router/pkg/epp/datalayer"
2829
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
30+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
2931
fwkrh "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/requesthandling"
3032
fwksched "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/scheduling"
3133
"github.com/llm-d/llm-d-router/pkg/epp/framework/plugins/scheduling/picker"
@@ -64,6 +66,9 @@ func BenchmarkSchedule(b *testing.B) {
6466
b.Fatalf("prefix scorer setup: %v", err)
6567
}
6668
loraAffinityScorer := loraaffinity.NewLoraAffinityScorer()
69+
datalayer.RegisterScopeSpecs([]fwkplugin.Plugin{
70+
kvCacheUtilizationScorer, queueingScorer, prefixCacheScorer, loraAffinityScorer,
71+
})
6772

6873
profile := NewSchedulerProfile().
6974
WithScorers(

pkg/epp/scheduling/scheduler_profile_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
tracenoop "go.opentelemetry.io/otel/trace/noop"
3232
k8stypes "k8s.io/apimachinery/pkg/types"
3333

34+
"github.com/llm-d/llm-d-router/pkg/epp/datalayer"
3435
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
3536
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
3637
fwksched "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/scheduling"
@@ -976,6 +977,7 @@ func TestRunScorer_ScopesTheWrappedPluginDeclarations(t *testing.T) {
976977
&fwkdl.Metrics{}, attrs)
977978

978979
scorer := &declaringScorer{key: key, reads: map[string]bool{}}
980+
datalayer.RegisterScopeSpecs([]fwkplugin.Plugin{scorer})
979981
scores := runScorer(context.Background(), nil, false,
980982
NewWeightedScorer(scorer, 1), &fwksched.InferenceRequest{}, []fwksched.Endpoint{endpoint})
981983

pkg/epp/scheduling/scheduler_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ import (
2626
"github.com/stretchr/testify/assert"
2727
k8stypes "k8s.io/apimachinery/pkg/types"
2828

29+
"github.com/llm-d/llm-d-router/pkg/epp/datalayer"
2930
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
31+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
3032
fwksched "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/scheduling"
3133
"github.com/llm-d/llm-d-router/pkg/epp/framework/plugins/scheduling/picker"
3234
"github.com/llm-d/llm-d-router/pkg/epp/framework/plugins/scheduling/picker/maxscore"
@@ -44,6 +46,9 @@ func TestSchedule(t *testing.T) {
4446
prefixCacheScorer, err := schedprefix.New(context.Background(), schedprefix.PrefixCacheScorerPluginType, "approx-prefix-cache-producer")
4547
assert.NoError(t, err)
4648
loraAffinityScorer := loraaffinity.NewLoraAffinityScorer()
49+
datalayer.RegisterScopeSpecs([]fwkplugin.Plugin{
50+
kvCacheUtilizationScorer, queueingScorer, prefixCacheScorer, loraAffinityScorer,
51+
})
4752

4853
defaultProfile := NewSchedulerProfile().
4954
WithScorers(NewWeightedScorer(kvCacheUtilizationScorer, 1),

0 commit comments

Comments
 (0)