From 80746a8efdda087847271337c4b53df2dc1a3d35 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Wed, 3 Sep 2025 15:42:41 +0200 Subject: [PATCH] Disable estimating resource size for resources with watch cache disabled Listing all keys from etcd turned out to be too expensive, negativly impacting events POST latency. Events resource is the only resource that by default has watch cache disabled and which includes very large number of small objects making it very costly to list keys. Expected impact: * No apiserver_resource_size_estimate_bytes metric for events. * APF overestimating LIST request cost to events. Fallback assumes object size of 1.5MB, meaning LIST events will always get maxSeats Kubernetes-commit: 3e84e16315ac2b3cb9729a98ed1ae636d8900db6 --- pkg/storage/etcd3/stats.go | 11 ++++++++++- pkg/storage/etcd3/store.go | 7 +++++-- pkg/storage/etcd3/store_test.go | 23 +++++++++++++++++++++-- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/pkg/storage/etcd3/stats.go b/pkg/storage/etcd3/stats.go index 227c46e10..9f0b3f78e 100644 --- a/pkg/storage/etcd3/stats.go +++ b/pkg/storage/etcd3/stats.go @@ -18,6 +18,7 @@ package etcd3 import ( "context" + "errors" "strings" "sync" @@ -78,6 +79,8 @@ type sizeRevision struct { revision int64 } +var errStatsDisabled = errors.New("key size stats disabled") + func (sc *statsCache) Stats(ctx context.Context) (storage.Stats, error) { keys, err := sc.GetKeys(ctx) if err != nil { @@ -100,6 +103,9 @@ func (sc *statsCache) GetKeys(ctx context.Context) ([]string, error) { getKeys := sc.getKeys sc.getKeysLock.Unlock() + if getKeys == nil { + return nil, errStatsDisabled + } // Don't execute getKeys under lock. return getKeys(ctx) } @@ -133,7 +139,10 @@ func (sc *statsCache) cleanKeysIfNeeded(ctx context.Context) { } keys, err := sc.GetKeys(ctx) if err != nil { - klog.InfoS("Error getting keys", "err", err) + if !errors.Is(err, errStatsDisabled) { + klog.InfoS("Error getting keys", "err", err) + } + return } sc.keysLock.Lock() defer sc.keysLock.Unlock() diff --git a/pkg/storage/etcd3/store.go b/pkg/storage/etcd3/store.go index 7f268bf6c..d8b7a00a2 100644 --- a/pkg/storage/etcd3/store.go +++ b/pkg/storage/etcd3/store.go @@ -188,7 +188,7 @@ func New(c *kubernetes.Client, compactor Compactor, codec runtime.Codec, newFunc } // Collecting stats requires properly set resourcePrefix to call getKeys. if resourcePrefix != "" && utilfeature.DefaultFeatureGate.Enabled(features.SizeBasedListCostEstimate) { - stats := newStatsCache(pathPrefix, s.getKeys) + stats := newStatsCache(pathPrefix, nil) s.stats = stats w.stats = stats } @@ -632,7 +632,10 @@ func getNewItemFunc(listObj runtime.Object, v reflect.Value) func() runtime.Obje func (s *store) Stats(ctx context.Context) (stats storage.Stats, err error) { if s.stats != nil { - return s.stats.Stats(ctx) + stats, err := s.stats.Stats(ctx) + if !errors.Is(err, errStatsDisabled) { + return stats, err + } } startTime := time.Now() prefix, err := s.prepareKey(s.resourcePrefix) diff --git a/pkg/storage/etcd3/store_test.go b/pkg/storage/etcd3/store_test.go index 5ae973fcb..3c01fd158 100644 --- a/pkg/storage/etcd3/store_test.go +++ b/pkg/storage/etcd3/store_test.go @@ -384,6 +384,7 @@ func TestStats(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SizeBasedListCostEstimate, sizeBasedListCostEstimate) // Match transformer with cacher tests. ctx, store, _ := testSetup(t) + store.SetKeysFunc(store.getKeys) storagetesting.RunTestStats(ctx, t, store, store.codec, store.transformer, sizeBasedListCostEstimate) }) } @@ -1041,15 +1042,30 @@ func TestPrefixStats(t *testing.T) { tcs := []struct { name string estimate bool + setKeys bool expectStats storage.Stats }{ { - name: "SizeBasedListCostEstimate=false", + name: "SizeBasedListCostEstimate=false,SetKeys=false", + setKeys: false, estimate: false, expectStats: storage.Stats{ObjectCount: 1}, }, { - name: "SizeBasedListCostEstimate=true", + name: "SizeBasedListCostEstimate=false,SetKeys=true", + setKeys: true, + estimate: false, + expectStats: storage.Stats{ObjectCount: 1}, + }, + { + name: "SizeBasedListCostEstimate=true,SetKeys=false", + setKeys: false, + estimate: true, + expectStats: storage.Stats{ObjectCount: 1}, + }, + { + name: "SizeBasedListCostEstimate=true,SetKeys=true", + setKeys: true, estimate: true, expectStats: storage.Stats{ObjectCount: 1, EstimatedAverageObjectSizeBytes: 3}, }, @@ -1058,6 +1074,9 @@ func TestPrefixStats(t *testing.T) { t.Run(tc.name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SizeBasedListCostEstimate, tc.estimate) ctx, store, c := testSetup(t, withPrefix("/registry"), withResourcePrefix("pods")) + if tc.setKeys { + store.SetKeysFunc(store.getKeys) + } _, err := c.KV.Put(ctx, "key", "a") if err != nil { t.Fatal(err)