Skip to content

Commit 5542232

Browse files
committed
refactor(flowcontrol)!: UsageLimitPolicy fills a framework-owned ceilings buffer
ComputeLimit returned a slice parallel to the priorities argument, and the dispatch loop indexed it unchecked: a policy returning a short slice panicked the processor. The policy now writes into a caller-provided buffer that the framework sizes to len(priorities) and pre-fills with 1.0, so a mis-sized result is unrepresentable and an unwritten entry fails open instead of carrying stale state. The processor reuses one buffer across cycles, removing the per-cycle allocation on the 1ms dispatch path. A batch call (rather than per-band) is retained deliberately: duty-cycle policies such as soft-reflective-ceiling advance shared state once per dispatch cycle and require all gated bands to observe the same open or closed decision within a cycle. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
1 parent 26df3f1 commit 5542232

8 files changed

Lines changed: 168 additions & 102 deletions

File tree

pkg/epp/config/loader/flowcontrol_test.go

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -448,13 +448,12 @@ func TestBuildFlowControlConfig(t *testing.T) {
448448
handle := newFlowControlTestHandle(t)
449449

450450
const funcPolicyName = "func-policy"
451-
handle.AddPlugin(funcPolicyName, usagelimits.NewPolicyFunc(funcPolicyName, func(_ context.Context, _ float64, priorities []int) []float64 {
452-
result := make([]float64, len(priorities))
453-
for i := range result {
454-
result[i] = 0.8
455-
}
456-
return result
457-
}))
451+
handle.AddPlugin(funcPolicyName, usagelimits.NewPolicyFunc(funcPolicyName,
452+
func(_ context.Context, _ float64, _ []int, ceilings []float64) {
453+
for i := range ceilings {
454+
ceilings[i] = 0.8
455+
}
456+
}))
458457

459458
const structPolicyName = "struct-policy"
460459
handle.AddPlugin(structPolicyName, &constantPointEightPolicy{})
@@ -490,7 +489,7 @@ func TestBuildFlowControlConfig(t *testing.T) {
490489
apiConfig: nil,
491490
assertion: func(t *testing.T, cfg *flowcontrol.Config) {
492491
require.NotNil(t, cfg.UsageLimitPolicy, "UsageLimitPolicy should be resolved even when not explicitly configured")
493-
ceilings := cfg.UsageLimitPolicy.ComputeLimit(context.Background(), 0.5, []int{0})
492+
ceilings := computeLimits(t, cfg.UsageLimitPolicy, 0.5, []int{0})
494493
assert.Equal(t, []float64{1.0}, ceilings, "Default noop policy should return 1.0 (no gating)")
495494
},
496495
},
@@ -501,7 +500,7 @@ func TestBuildFlowControlConfig(t *testing.T) {
501500
},
502501
assertion: func(t *testing.T, cfg *flowcontrol.Config) {
503502
require.NotNil(t, cfg.UsageLimitPolicy, "UsageLimitPolicy should be resolved from the handle")
504-
ceilings := cfg.UsageLimitPolicy.ComputeLimit(context.Background(), 0.5, []int{0})
503+
ceilings := computeLimits(t, cfg.UsageLimitPolicy, 0.5, []int{0})
505504
assert.Equal(t, []float64{1.0}, ceilings, "Noop policy should return 1.0 (no gating)")
506505
},
507506
},
@@ -512,7 +511,6 @@ func TestBuildFlowControlConfig(t *testing.T) {
512511
},
513512
assertion: func(t *testing.T, cfg *flowcontrol.Config) {
514513
require.NotNil(t, cfg.UsageLimitPolicy)
515-
ctx := context.Background()
516514
for _, tc := range []struct {
517515
name string
518516
priority int
@@ -522,7 +520,7 @@ func TestBuildFlowControlConfig(t *testing.T) {
522520
{"half saturation", 1, 0.5},
523521
{"full saturation", 5, 1.0},
524522
} {
525-
assert.Equal(t, []float64{0.8}, cfg.UsageLimitPolicy.ComputeLimit(ctx, tc.saturation, []int{tc.priority}),
523+
assert.Equal(t, []float64{0.8}, computeLimits(t, cfg.UsageLimitPolicy, tc.saturation, []int{tc.priority}),
526524
"func-based policy should return 0.8 at %s", tc.name)
527525
}
528526
},
@@ -534,7 +532,6 @@ func TestBuildFlowControlConfig(t *testing.T) {
534532
},
535533
assertion: func(t *testing.T, cfg *flowcontrol.Config) {
536534
require.NotNil(t, cfg.UsageLimitPolicy)
537-
ctx := context.Background()
538535
for _, tc := range []struct {
539536
name string
540537
priority int
@@ -544,7 +541,7 @@ func TestBuildFlowControlConfig(t *testing.T) {
544541
{"half saturation", 1, 0.5},
545542
{"full saturation", 5, 1.0},
546543
} {
547-
assert.Equal(t, []float64{0.8}, cfg.UsageLimitPolicy.ComputeLimit(ctx, tc.saturation, []int{tc.priority}),
544+
assert.Equal(t, []float64{0.8}, computeLimits(t, cfg.UsageLimitPolicy, tc.saturation, []int{tc.priority}),
548545
"struct-based policy should return 0.8 at %s", tc.name)
549546
}
550547
},
@@ -608,12 +605,22 @@ func (p *constantPointEightPolicy) TypedName() fwkplugin.TypedName {
608605
}
609606
}
610607

611-
func (p *constantPointEightPolicy) ComputeLimit(_ context.Context, _ float64, priorities []int) []float64 {
612-
result := make([]float64, len(priorities))
613-
for i := range result {
614-
result[i] = 0.8
608+
func (p *constantPointEightPolicy) ComputeLimit(_ context.Context, _ float64, _ []int, ceilings []float64) {
609+
for i := range ceilings {
610+
ceilings[i] = 0.8
615611
}
616-
return result
617612
}
618613

619614
var _ fwkfc.UsageLimitPolicy = (*constantPointEightPolicy)(nil)
615+
616+
// computeLimits invokes a UsageLimitPolicy with a framework-style output buffer (pre-filled with
617+
// 1.0, sized to priorities) and returns the filled ceilings.
618+
func computeLimits(t *testing.T, p fwkfc.UsageLimitPolicy, saturation float64, priorities []int) []float64 {
619+
t.Helper()
620+
ceilings := make([]float64, len(priorities))
621+
for i := range ceilings {
622+
ceilings[i] = 1.0
623+
}
624+
p.ComputeLimit(context.Background(), saturation, priorities, ceilings)
625+
return ceilings
626+
}

pkg/epp/flowcontrol/controller/internal/processor.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ type Processor struct {
8787
// it needs no synchronization.
8888
poolEmpty bool
8989

90+
// ceilings is the reusable output buffer handed to the UsageLimitPolicy each dispatch cycle,
91+
// avoiding a per-cycle allocation. Only accessed from the Run goroutine, so it needs no
92+
// synchronization.
93+
ceilings []float64
94+
9095
// wg is used to wait for background tasks (cleanup sweep) to complete on shutdown.
9196
wg sync.WaitGroup
9297
isShuttingDown atomic.Bool
@@ -381,7 +386,8 @@ func (p *Processor) dispatchCycle(ctx context.Context) bool {
381386
metrics.RecordFlowControlPoolSaturation(p.poolName, saturation)
382387

383388
priorities := p.registry.AllOrderedPriorityLevels()
384-
ceilings := p.usageLimitPolicy.ComputeLimit(ctx, saturation, priorities)
389+
ceilings := p.ceilingsBuffer(len(priorities))
390+
p.usageLimitPolicy.ComputeLimit(ctx, saturation, priorities, ceilings)
385391

386392
for i, priority := range priorities {
387393
// --- Viability Check (Saturation/HoL Blocking) ---
@@ -423,6 +429,20 @@ func (p *Processor) dispatchCycle(ctx context.Context) bool {
423429
return false
424430
}
425431

432+
// ceilingsBuffer returns the reusable ceilings buffer sized to n, every element reset to 1.0 (no
433+
// gating). Pre-filling guarantees that an entry the policy does not write fails open rather than
434+
// carrying a stale value from the previous cycle.
435+
func (p *Processor) ceilingsBuffer(n int) []float64 {
436+
if cap(p.ceilings) < n {
437+
p.ceilings = make([]float64, n)
438+
}
439+
buf := p.ceilings[:n]
440+
for i := range buf {
441+
buf[i] = 1.0
442+
}
443+
return buf
444+
}
445+
426446
// selectItem applies the configured fairness and ordering policies to select a single item.
427447
func (p *Processor) selectItem(
428448
ctx context.Context,

pkg/epp/framework/interface/flowcontrol/plugins.go

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -143,32 +143,42 @@ type SaturationDetector interface {
143143
//
144144
// Integration:
145145
// This policy is called during dispatch decision-making, before a request is allowed to proceed. For each
146-
// priority band, the returned ceiling is compared against current saturation. If saturation exceeds the
146+
// priority band, the computed ceiling is compared against current saturation. If saturation exceeds the
147147
// ceiling for a given priority, requests at that priority are gated (not dispatched). The dispatch loop
148148
// visits bands from highest to lowest priority and stops at the first gated band; lower bands are not
149149
// considered on that call.
150150
//
151-
// Conformance: Implementations MUST ensure all methods are goroutine-safe. Returned ceilings MUST be
151+
// The framework calls ComputeLimit exactly once per dispatch cycle. This is a contract term, not an
152+
// implementation detail: dispatch-spreading policies use the call itself as their time base (one tick
153+
// per cycle), and computing all ceilings in one call is what lets every gated band observe the same
154+
// open/closed decision within a cycle. The batch shape exists to preserve both properties.
155+
//
156+
// Conformance: Implementations MUST ensure all methods are goroutine-safe. Computed ceilings MUST be
152157
// monotonically non-increasing in the given priority order (highest priority first): because the
153158
// dispatch loop stops at the first gated band, a lower band whose ceiling exceeds that of a higher band
154159
// can be marked open on calls where it is unreachable, starving it.
155160
type UsageLimitPolicy interface {
156161
plugin.Plugin
157162

158163
// ComputeLimit calculates usage ceilings for all currently active priority levels based on current
159-
// saturation. The plugin observes the active priority domain (which changes dynamically as workloads
160-
// come and go) and computes relative ceilings from scratch on each call.
164+
// saturation, writing the ceiling for the n-th priority into the n-th element of the
165+
// caller-provided ceilings buffer. The plugin observes the active priority domain (which changes
166+
// dynamically as workloads come and go) and computes relative ceilings from scratch on each call.
167+
//
168+
// The framework guarantees len(ceilings) == len(priorities) and pre-fills every element with 1.0
169+
// (no gating), so an entry the plugin does not write fails open. Writing into the framework-owned
170+
// buffer means a result of the wrong size cannot exist; there is no return value to validate.
161171
//
162172
// Parameters:
163173
// - ctx: Request context for logging, tracing, etc.
164174
// - saturation: Current pool-wide resource saturation as a fraction [0.0, 1.0]
165-
// - priorities: Ordered list of currently active priority levels (highest first)
166-
//
167-
// Returns:
168-
// - ceilings: Computed ceiling for each given priority (n-th ceiling is assigned to the given n-th priority)
175+
// - priorities: Ordered list of currently active priority levels (highest first).
176+
// The slice is a shared snapshot owned by the framework: read-only, and it MUST NOT be
177+
// retained after the call returns.
178+
// - ceilings: Output buffer owned by the framework, valid only for the duration of the call.
179+
// Ceiling semantics per element:
169180
// - 0.0 = fully gated (cannot dispatch regardless of current saturation)
170181
// - 1.0 = no gating (can dispatch until fully saturated)
171182
// - Values between 0.0 and 1.0 reserve capacity headroom
172-
//
173-
ComputeLimit(ctx context.Context, saturation float64, priorities []int) (ceilings []float64)
183+
ComputeLimit(ctx context.Context, saturation float64, priorities []int, ceilings []float64)
174184
}

pkg/epp/framework/plugins/flowcontrol/usagelimits/priorityholdback/priority_holdback.go

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ type priorityHoldbackPolicy struct {
5656
name string
5757
cMax float64
5858
cMin float64
59-
computeFn func(cMin, cMax float64, priorities []int) (ceilings []float64)
59+
computeFn func(cMin, cMax float64, priorities []int, ceilings []float64)
6060
}
6161

6262
var _ flowcontrol.UsageLimitPolicy = &priorityHoldbackPolicy{}
6363

6464
func newPriorityHoldbackPolicy(cfg config) *priorityHoldbackPolicy {
65-
var fn func(cMin, cMax float64, priorities []int) (ceilings []float64)
65+
var fn func(cMin, cMax float64, priorities []int, ceilings []float64)
6666
switch cfg.domain {
6767
case domainRank:
6868
fn = computeLimitStepwiseSpread
@@ -95,37 +95,36 @@ func (p *priorityHoldbackPolicy) TypedName() plugin.TypedName {
9595
}
9696
}
9797

98-
// ComputeLimit returns an admission ceiling for each priority. With a single active priority,
99-
// holdback is bypassed (ceiling = cMax) to preserve work-conserving behavior.
100-
func (p *priorityHoldbackPolicy) ComputeLimit(_ context.Context, _ float64, priorities []int) (ceilings []float64) {
98+
// ComputeLimit writes an admission ceiling for each priority into the caller-provided buffer.
99+
// With a single active priority, holdback is bypassed (ceiling = cMax) to preserve
100+
// work-conserving behavior.
101+
func (p *priorityHoldbackPolicy) ComputeLimit(_ context.Context, _ float64, priorities []int, ceilings []float64) {
101102
if len(priorities) == 0 {
102-
return []float64{}
103+
return
103104
}
104105
if len(priorities) == 1 {
105-
return []float64{p.cMax}
106+
ceilings[0] = p.cMax
107+
return
106108
}
107109
// Ceilings are monotonically decreasing as priorities are ordered from highest to lowest per UsageLimitPolicy contract.
108110
// New strategies (e.g. sigmoid/static definition) could require explicit monotizing sweep.
109-
return p.computeFn(p.cMin, p.cMax, priorities)
111+
p.computeFn(p.cMin, p.cMax, priorities, ceilings)
110112
}
111113

112114
// computeLimitStepwiseSpread divides [cMin, cMax] into equal steps by rank.
113115
// c_i = cMax - i * (cMax - cMin) / (N - 1)
114-
func computeLimitStepwiseSpread(cMin, cMax float64, priorities []int) (ceilings []float64) {
115-
ceilings = make([]float64, len(priorities))
116+
func computeLimitStepwiseSpread(cMin, cMax float64, priorities []int, ceilings []float64) {
116117
spread := cMax - cMin
117118
n := float64(len(priorities) - 1)
118119
for i := range priorities {
119120
ceilings[i] = cMax - float64(i)*spread/n
120121
}
121-
return ceilings
122122
}
123123

124124
// computeLimitLinearProportional scales ceilings proportionally to numerical priority values.
125125
// r_i = (p_i - pMin) / (pMax - pMin)
126126
// c_i = cMin + r_i * (cMax - cMin)
127-
func computeLimitLinearProportional(cMin, cMax float64, priorities []int) (ceilings []float64) {
128-
ceilings = make([]float64, len(priorities))
127+
func computeLimitLinearProportional(cMin, cMax float64, priorities []int, ceilings []float64) {
129128
pMin := float64(priorities[len(priorities)-1])
130129
pMax := float64(priorities[0])
131130
pRange := pMax - pMin
@@ -135,12 +134,11 @@ func computeLimitLinearProportional(cMin, cMax float64, priorities []int) (ceili
135134
for i := range priorities {
136135
ceilings[i] = cMax
137136
}
138-
return ceilings
137+
return
139138
}
140139
spread := cMax - cMin
141140
for i := range priorities {
142141
r := (float64(priorities[i]) - pMin) / pRange
143142
ceilings[i] = cMin + r*spread
144143
}
145-
return ceilings
146144
}

0 commit comments

Comments
 (0)