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: f779cf6381917267aa54460b7e66b9a7cc165677

Author: serathius

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()

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)

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)