Skip to content

Commit 9da6cd2

Browse files
committed
refactor: improve config validation in precise-prefix-cache-scorer
- Extract hardcoded subscription timeout to named constant - Add validation for pod discovery configuration - Validate socket port range when discoverPods is enabled This prevents runtime panics from invalid configuration and makes the timeout value more maintainable.
1 parent c910eeb commit 9da6cd2

File tree

2 files changed

+123
-3
lines changed

2 files changed

+123
-3
lines changed

pkg/plugins/scorer/precise_prefix_cache.go

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ import (
2727
const (
2828
// PrecisePrefixCachePluginType is the type-name of the PrecisePrefixCacheScorer plugin.
2929
PrecisePrefixCachePluginType = "precise-prefix-cache-scorer"
30+
31+
// defaultSubscriptionTimeout defines how long to keep inactive endpoint
32+
// subscriptions before removing them from the KV-events pool.
33+
defaultSubscriptionTimeout = 10 * time.Minute
3034
)
3135

3236
// PrecisePrefixCachePluginConfig holds the configuration for the
@@ -94,6 +98,18 @@ func New(ctx context.Context, config PrecisePrefixCachePluginConfig) (*PrecisePr
9498
config.TokenProcessorConfig = kvblock.DefaultTokenProcessorConfig()
9599
}
96100

101+
// Validate pod discovery configuration
102+
if config.KVEventsConfig != nil && config.KVEventsConfig.DiscoverPods {
103+
if config.KVEventsConfig.PodDiscoveryConfig == nil {
104+
return nil, errors.New("podDiscoveryConfig is required when discoverPods is enabled")
105+
}
106+
if config.KVEventsConfig.PodDiscoveryConfig.SocketPort <= 0 ||
107+
config.KVEventsConfig.PodDiscoveryConfig.SocketPort > 65535 {
108+
return nil, fmt.Errorf("invalid socket port: must be between 1 and 65535, got %d",
109+
config.KVEventsConfig.PodDiscoveryConfig.SocketPort)
110+
}
111+
}
112+
97113
tokenProcessor := kvblock.NewChunkedTokenDatabase(config.TokenProcessorConfig)
98114

99115
// initialize the indexer
@@ -114,9 +130,8 @@ func New(ctx context.Context, config PrecisePrefixCachePluginConfig) (*PrecisePr
114130
// initialize the subscribers cache only if endpoint discovery is enabled
115131
if config.KVEventsConfig.DiscoverPods {
116132
// initialize the subscribers TTL cache
117-
subscriptionTimeout := 10 * time.Minute
118133
subscribersCache = ttlcache.New[string, struct{}](
119-
ttlcache.WithTTL[string, struct{}](subscriptionTimeout),
134+
ttlcache.WithTTL[string, struct{}](defaultSubscriptionTimeout),
120135
)
121136
subscribersCache.OnEviction(func(ctx context.Context, reason ttlcache.EvictionReason,
122137
item *ttlcache.Item[string, struct{}],
@@ -125,7 +140,7 @@ func New(ctx context.Context, config PrecisePrefixCachePluginConfig) (*PrecisePr
125140
subscribersManager.RemoveSubscriber(ctx, item.Key())
126141
}
127142
})
128-
go cleanCachePeriodically(ctx, subscribersCache, subscriptionTimeout)
143+
go cleanCachePeriodically(ctx, subscribersCache, defaultSubscriptionTimeout)
129144
}
130145
if config.KVEventsConfig.ZMQEndpoint != "" {
131146
// setup local subscriber to support global socket mode
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package scorer
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/llm-d/llm-d-kv-cache/pkg/kvcache"
8+
"github.com/llm-d/llm-d-kv-cache/pkg/kvevents"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestNew_PodDiscoveryValidation(t *testing.T) {
14+
ctx := context.Background()
15+
16+
tests := []struct {
17+
name string
18+
config PrecisePrefixCachePluginConfig
19+
expectError bool
20+
errorMsg string
21+
}{
22+
{
23+
name: "valid config with pod discovery disabled",
24+
config: PrecisePrefixCachePluginConfig{
25+
IndexerConfig: &kvcache.Config{},
26+
KVEventsConfig: &kvevents.Config{
27+
DiscoverPods: false,
28+
},
29+
},
30+
expectError: false,
31+
},
32+
{
33+
name: "missing podDiscoveryConfig when discoverPods enabled",
34+
config: PrecisePrefixCachePluginConfig{
35+
IndexerConfig: &kvcache.Config{},
36+
KVEventsConfig: &kvevents.Config{
37+
DiscoverPods: true,
38+
PodDiscoveryConfig: nil,
39+
},
40+
},
41+
expectError: true,
42+
errorMsg: "podDiscoveryConfig is required when discoverPods is enabled",
43+
},
44+
{
45+
name: "invalid socket port - zero",
46+
config: PrecisePrefixCachePluginConfig{
47+
IndexerConfig: &kvcache.Config{},
48+
KVEventsConfig: &kvevents.Config{
49+
DiscoverPods: true,
50+
PodDiscoveryConfig: &kvevents.PodDiscoveryConfig{
51+
SocketPort: 0,
52+
},
53+
},
54+
},
55+
expectError: true,
56+
errorMsg: "invalid socket port: must be between 1 and 65535, got 0",
57+
},
58+
{
59+
name: "invalid socket port - negative",
60+
config: PrecisePrefixCachePluginConfig{
61+
IndexerConfig: &kvcache.Config{},
62+
KVEventsConfig: &kvevents.Config{
63+
DiscoverPods: true,
64+
PodDiscoveryConfig: &kvevents.PodDiscoveryConfig{
65+
SocketPort: -1,
66+
},
67+
},
68+
},
69+
expectError: true,
70+
errorMsg: "invalid socket port: must be between 1 and 65535, got -1",
71+
},
72+
{
73+
name: "invalid socket port - too large",
74+
config: PrecisePrefixCachePluginConfig{
75+
IndexerConfig: &kvcache.Config{},
76+
KVEventsConfig: &kvevents.Config{
77+
DiscoverPods: true,
78+
PodDiscoveryConfig: &kvevents.PodDiscoveryConfig{
79+
SocketPort: 65536,
80+
},
81+
},
82+
},
83+
expectError: true,
84+
errorMsg: "invalid socket port: must be between 1 and 65535, got 65536",
85+
},
86+
}
87+
88+
for _, tt := range tests {
89+
t.Run(tt.name, func(t *testing.T) {
90+
_, err := New(ctx, tt.config)
91+
92+
if tt.expectError {
93+
require.Error(t, err)
94+
assert.Contains(t, err.Error(), tt.errorMsg)
95+
} else {
96+
// Note: This will still fail due to missing tokenizer config,
97+
// but it should pass the pod discovery validation
98+
if err != nil {
99+
assert.NotContains(t, err.Error(), "podDiscoveryConfig")
100+
assert.NotContains(t, err.Error(), "socket port")
101+
}
102+
}
103+
})
104+
}
105+
}

0 commit comments

Comments
 (0)