Merge pull request #91818 from wojtek-t/remove_cachesize

Remove heuristic watchcache sizes

cmd/kube-apiserver/app/BUILD

@@ -25,7 +25,6 @@ go_library(
         "//pkg/master/controller/crdregistration:go_default_library",
         "//pkg/master/reconcilers:go_default_library",
         "//pkg/master/tunneler:go_default_library",
-        "//pkg/registry/cachesize:go_default_library",
         "//pkg/registry/rbac/rest:go_default_library",
         "//pkg/serviceaccount:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1:go_default_library",

cmd/kube-apiserver/app/server.go

@@ -78,7 +78,6 @@ import (
 	"k8s.io/kubernetes/pkg/master"
 	"k8s.io/kubernetes/pkg/master/reconcilers"
 	"k8s.io/kubernetes/pkg/master/tunneler"
-	"k8s.io/kubernetes/pkg/registry/cachesize"
 	rbacrest "k8s.io/kubernetes/pkg/registry/rbac/rest"
 	"k8s.io/kubernetes/pkg/serviceaccount"
 )
@@ -663,15 +662,8 @@ func Complete(s *options.ServerRunOptions) (completedServerRunOptions, error) {
 	}
 
 	if s.Etcd.EnableWatchCache {
-		klog.V(2).Infof("Initializing cache sizes based on %dMB limit", s.GenericServerRunOptions.TargetRAMMB)
+		// Ensure that overrides parse correctly.
-		sizes := cachesize.NewHeuristicWatchCacheSizes(s.GenericServerRunOptions.TargetRAMMB)
+		if _, err := serveroptions.ParseWatchCacheSizes(s.Etcd.WatchCacheSizes); err != nil {
-		if userSpecified, err := serveroptions.ParseWatchCacheSizes(s.Etcd.WatchCacheSizes); err == nil {
-			for resource, size := range userSpecified {
-				sizes[resource] = size
-			}
-		}
-		s.Etcd.WatchCacheSizes, err = serveroptions.WriteWatchCacheSizes(sizes)
-		if err != nil {
 			return options, err
 		}
 	}

pkg/registry/BUILD

@@ -44,7 +44,6 @@ filegroup(
         "//pkg/registry/batch/cronjob:all-srcs",
         "//pkg/registry/batch/job:all-srcs",
         "//pkg/registry/batch/rest:all-srcs",
-        "//pkg/registry/cachesize:all-srcs",
         "//pkg/registry/certificates/certificates:all-srcs",
         "//pkg/registry/certificates/rest:all-srcs",
         "//pkg/registry/coordination/lease:all-srcs",

pkg/registry/cachesize/BUILD

@@ -1,26 +0,0 @@
-package(default_visibility = ["//visibility:public"])
-
-load(
-    "@io_bazel_rules_go//go:def.bzl",
-    "go_library",
-)
-
-go_library(
-    name = "go_default_library",
-    srcs = ["cachesize.go"],
-    importpath = "k8s.io/kubernetes/pkg/registry/cachesize",
-    deps = ["//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library"],
-)
-
-filegroup(
-    name = "package-srcs",
-    srcs = glob(["**"]),
-    tags = ["automanaged"],
-    visibility = ["//visibility:private"],
-)
-
-filegroup(
-    name = "all-srcs",
-    srcs = [":package-srcs"],
-    tags = ["automanaged"],
-)

pkg/registry/cachesize/OWNERS

@@ -1,9 +0,0 @@
-# See the OWNERS docs at https://go.k8s.io/owners
-
-reviewers:
-- wojtek-t
-- gmarek
-- soltysh
-- madhusudancs
-- mml
-- caseydavenport

pkg/registry/cachesize/cachesize.go

@@ -1,54 +0,0 @@
-/*
-Copyright 2014 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package cachesize
-
-import (
-	"k8s.io/apimachinery/pkg/runtime/schema"
-)
-
-// NewHeuristicWatchCacheSizes returns a map of suggested watch cache sizes based on total
-// memory.
-func NewHeuristicWatchCacheSizes(expectedRAMCapacityMB int) map[schema.GroupResource]int {
-	// From our documentation, we officially recommend 120GB machines for
-	// 2000 nodes, and we scale from that point. Thus we assume ~60MB of
-	// capacity per node.
-	// TODO: Revisit this heuristics
-	clusterSize := expectedRAMCapacityMB / 60
-
-	// We should specify cache size for a given resource only if it
-	// is supposed to have non-default value.
-	//
-	// TODO: Figure out which resource we should have non-default value.
-	watchCacheSizes := make(map[schema.GroupResource]int)
-	watchCacheSizes[schema.GroupResource{Resource: "replicationcontrollers"}] = maxInt(5*clusterSize, 100)
-	watchCacheSizes[schema.GroupResource{Resource: "endpoints"}] = maxInt(10*clusterSize, 1000)
-	watchCacheSizes[schema.GroupResource{Resource: "endpointslices", Group: "discovery.k8s.io"}] = maxInt(10*clusterSize, 1000)
-	watchCacheSizes[schema.GroupResource{Resource: "nodes"}] = maxInt(5*clusterSize, 1000)
-	watchCacheSizes[schema.GroupResource{Resource: "pods"}] = maxInt(50*clusterSize, 1000)
-	watchCacheSizes[schema.GroupResource{Resource: "services"}] = maxInt(5*clusterSize, 1000)
-	watchCacheSizes[schema.GroupResource{Resource: "events"}] = 0
-	watchCacheSizes[schema.GroupResource{Resource: "apiservices", Group: "apiregistration.k8s.io"}] = maxInt(5*clusterSize, 1000)
-	watchCacheSizes[schema.GroupResource{Resource: "leases", Group: "coordination.k8s.io"}] = maxInt(5*clusterSize, 1000)
-	return watchCacheSizes
-}
-
-func maxInt(a, b int) int {
-	if a > b {
-		return a
-	}
-	return b
-}

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -1018,7 +1018,7 @@ func (t CRDRESTOptionsGetter) GetRESTOptions(resource schema.GroupResource) (gen
 		CountMetricPollPeriod:   t.CountMetricPollPeriod,
 	}
 	if t.EnableWatchCache {
-		ret.Decorator = genericregistry.StorageWithCacher(t.DefaultWatchCacheSize)
+		ret.Decorator = genericregistry.StorageWithCacher()
 	}
 	return ret, nil
 }

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler_test.go

@@ -475,7 +475,7 @@ func testHandlerConversion(t *testing.T, enableWatchCache bool) {
 		CountMetricPollPeriod:   time.Minute,
 	}
 	if enableWatchCache {
-		restOptionsGetter.Decorator = genericregistry.StorageWithCacher(100)
+		restOptionsGetter.Decorator = genericregistry.StorageWithCacher()
 	}
 
 	handler, err := NewCustomResourceDefinitionHandler(

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/storage_factory.go

@@ -34,7 +34,7 @@ import (
 )
 
 // Creates a cacher based given storageConfig.
-func StorageWithCacher(capacity int) generic.StorageDecorator {
+func StorageWithCacher() generic.StorageDecorator {
 	return func(
 		storageConfig *storagebackend.Config,
 		resourcePrefix string,
@@ -49,18 +49,11 @@ func StorageWithCacher(capacity int) generic.StorageDecorator {
 		if err != nil {
 			return s, d, err
 		}
-		if capacity <= 0 {
-			klog.V(5).Infof("Storage caching is disabled for %s", objectTypeToString(newFunc()))
-			return s, d, nil
-		}
 		if klog.V(5).Enabled() {
-			klog.Infof("Storage caching is enabled for %s with capacity %v", objectTypeToString(newFunc()), capacity)
+			klog.Infof("Storage caching is enabled for %s", objectTypeToString(newFunc()))
 		}
 
-		// TODO: we would change this later to make storage always have cacher and hide low level KV layer inside.
-		// Currently it has two layers of same storage interface -- cacher and low level kv.
 		cacherConfig := cacherstorage.Config{
-			CacheCapacity:  capacity,
 			Storage:        s,
 			Versioner:      etcd3.APIObjectVersioner{},
 			ResourcePrefix: resourcePrefix,

staging/src/k8s.io/apiserver/pkg/server/options/etcd.go

@@ -35,6 +35,7 @@ import (
 	serverstorage "k8s.io/apiserver/pkg/server/storage"
 	"k8s.io/apiserver/pkg/storage/storagebackend"
 	storagefactory "k8s.io/apiserver/pkg/storage/storagebackend/factory"
+	"k8s.io/klog/v2"
 )
 
 type EtcdOptions struct {
@@ -238,12 +239,15 @@ func (f *SimpleRestOptionsFactory) GetRESTOptions(resource schema.GroupResource)
 		if err != nil {
 			return generic.RESTOptions{}, err
 		}
-		cacheSize, ok := sizes[resource]
+		size, ok := sizes[resource]
-		if !ok {
+		if ok && size > 0 {
-			cacheSize = f.Options.DefaultWatchCacheSize
+			klog.Warningf("Dropping watch-cache-size for %v - watchCache size is now dynamic", resource)
+		}
+		if ok && size <= 0 {
+			ret.Decorator = generic.UndecoratedStorage
+		} else {
+			ret.Decorator = genericregistry.StorageWithCacher()
 		}
-		// depending on cache size this might return an undecorated storage
-		ret.Decorator = genericregistry.StorageWithCacher(cacheSize)
 	}
 	return ret, nil
 }
@@ -272,12 +276,15 @@ func (f *StorageFactoryRestOptionsFactory) GetRESTOptions(resource schema.GroupR
 		if err != nil {
 			return generic.RESTOptions{}, err
 		}
-		cacheSize, ok := sizes[resource]
+		size, ok := sizes[resource]
-		if !ok {
+		if ok && size > 0 {
-			cacheSize = f.Options.DefaultWatchCacheSize
+			klog.Warningf("Dropping watch-cache-size for %v - watchCache size is now dynamic", resource)
+		}
+		if ok && size <= 0 {
+			ret.Decorator = generic.UndecoratedStorage
+		} else {
+			ret.Decorator = genericregistry.StorageWithCacher()
 		}
-		// depending on cache size this might return an undecorated storage
-		ret.Decorator = genericregistry.StorageWithCacher(cacheSize)
 	}
 
 	return ret, nil

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go

@@ -50,7 +50,6 @@ type ServerRunOptions struct {
 	// We intentionally did not add a flag for this option. Users of the
 	// apiserver library can wire it to a flag.
 	MaxRequestBodyBytes       int64
-	TargetRAMMB               int
 	EnablePriorityAndFairness bool
 }
 
@@ -108,9 +107,6 @@ func (s *ServerRunOptions) DefaultAdvertiseAddress(secure *SecureServingOptions)
 // Validate checks validation of ServerRunOptions
 func (s *ServerRunOptions) Validate() []error {
 	errors := []error{}
-	if s.TargetRAMMB < 0 {
-		errors = append(errors, fmt.Errorf("--target-ram-mb can not be negative value"))
-	}
 
 	if s.LivezGracePeriod < 0 {
 		errors = append(errors, fmt.Errorf("--livez-grace-period can not be a negative value"))
@@ -165,8 +161,10 @@ func (s *ServerRunOptions) AddUniversalFlags(fs *pflag.FlagSet) {
 		"List of allowed origins for CORS, comma separated.  An allowed origin can be a regular "+
 		"expression to support subdomain matching. If this list is empty CORS will not be enabled.")
 
-	fs.IntVar(&s.TargetRAMMB, "target-ram-mb", s.TargetRAMMB,
+	deprecatedTargetRAMMB := 0
-		"Memory limit for apiserver in MB (used to configure sizes of caches, etc.)")
+	fs.IntVar(&deprecatedTargetRAMMB, "target-ram-mb", deprecatedTargetRAMMB,
+		"DEPRECATED: Memory limit for apiserver in MB (used to configure sizes of caches, etc.)")
+	fs.MarkDeprecated("target-ram-mb", "This flag will be removed in v1.23")
 
 	fs.StringVar(&s.ExternalHost, "external-hostname", s.ExternalHost,
 		"The hostname to use when generating externalized URLs for this master (e.g. Swagger API Docs or OpenID Discovery).")

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go

@@ -31,21 +31,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 		testOptions *ServerRunOptions
 		expectErr   string
 	}{
-		{
-			name: "Test when TargetRAMMB is negative value",
-			testOptions: &ServerRunOptions{
-				AdvertiseAddress:            net.ParseIP("192.168.10.10"),
-				CorsAllowedOriginList:       []string{"10.10.10.100", "10.10.10.200"},
-				MaxRequestsInFlight:         400,
-				MaxMutatingRequestsInFlight: 200,
-				RequestTimeout:              time.Duration(2) * time.Minute,
-				MinRequestTimeout:           1800,
-				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
-				MaxRequestBodyBytes:         10 * 1024 * 1024,
-				TargetRAMMB:                 -65536,
-			},
-			expectErr: "--target-ram-mb can not be negative value",
-		},
 		{
 			name: "Test when MaxRequestsInFlight is negative value",
 			testOptions: &ServerRunOptions{
@@ -57,7 +42,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MinRequestTimeout:           1800,
 				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				MaxRequestBodyBytes:         10 * 1024 * 1024,
-				TargetRAMMB:                 65536,
 			},
 			expectErr: "--max-requests-inflight can not be negative value",
 		},
@@ -72,7 +56,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MinRequestTimeout:           1800,
 				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				MaxRequestBodyBytes:         10 * 1024 * 1024,
-				TargetRAMMB:                 65536,
 			},
 			expectErr: "--max-mutating-requests-inflight can not be negative value",
 		},
@@ -87,7 +70,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MinRequestTimeout:           1800,
 				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				MaxRequestBodyBytes:         10 * 1024 * 1024,
-				TargetRAMMB:                 65536,
 			},
 			expectErr: "--request-timeout can not be negative value",
 		},
@@ -102,7 +84,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MinRequestTimeout:           -1800,
 				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				MaxRequestBodyBytes:         10 * 1024 * 1024,
-				TargetRAMMB:                 65536,
 			},
 			expectErr: "--min-request-timeout can not be negative value",
 		},
@@ -117,7 +98,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MinRequestTimeout:           1800,
 				JSONPatchMaxCopyBytes:       -10 * 1024 * 1024,
 				MaxRequestBodyBytes:         10 * 1024 * 1024,
-				TargetRAMMB:                 65536,
 			},
 			expectErr: "--json-patch-max-copy-bytes can not be negative value",
 		},
@@ -132,7 +112,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MinRequestTimeout:           1800,
 				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				MaxRequestBodyBytes:         -10 * 1024 * 1024,
-				TargetRAMMB:                 65536,
 			},
 			expectErr: "--max-resource-write-bytes can not be negative value",
 		},
@@ -147,7 +126,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MinRequestTimeout:           1800,
 				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				MaxRequestBodyBytes:         10 * 1024 * 1024,
-				TargetRAMMB:                 65536,
 				LivezGracePeriod:            -time.Second,
 			},
 			expectErr: "--livez-grace-period can not be a negative value",
@@ -163,7 +141,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MinRequestTimeout:           1800,
 				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				MaxRequestBodyBytes:         10 * 1024 * 1024,
-				TargetRAMMB:                 65536,
 				ShutdownDelayDuration:       -time.Second,
 			},
 			expectErr: "--shutdown-delay-duration can not be negative value",
@@ -179,7 +156,6 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MinRequestTimeout:           1800,
 				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				MaxRequestBodyBytes:         10 * 1024 * 1024,
-				TargetRAMMB:                 65536,
 			},
 		},
 	}