Backport upstream changes to watch cache enablement #16398
Conversation
openshift-merge-robot
added
the
approved
openshift-ci-robot
added
the
size/L
|
/unassign |
smarterclayton
force-pushed
the
disablecache
branch
from
38ea496
to
ef6d782
Compare
|
/retest |
Backport the change that allows a global default watch cache size as well as being able to disable an individual watch cache item
Remove some complexity in RESTOptionsGetter and add default watch cache sizes for resources that are read by nodes.
Any resource named by the heuristics gets a watch cache by default. Admins can restore the previous behavior by setting `--default-watch-cache-size` to a positive integer. This reduces the amount of total memory allocated on large cluster significantly at minor cost in CPU on the etcd process and an increase in network bandwidth to etcd.
smarterclayton
force-pushed
the
disablecache
branch
from
ef6d782
to
01aeb23
Compare
|
/retest |
| @@ -150,7 +148,7 @@ func BuildKubeAPIserverOptions(masterConfig configapi.MasterConfig) (*kapiserver | |||
| server.Etcd.StorageConfig.KeyFile = masterConfig.EtcdClientInfo.ClientCert.KeyFile | |||
| server.Etcd.StorageConfig.CertFile = masterConfig.EtcdClientInfo.ClientCert.CertFile | |||
| server.Etcd.StorageConfig.CAFile = masterConfig.EtcdClientInfo.CA | |||
| server.Etcd.DefaultWatchCacheSize = DefaultWatchCacheSize | |||
| server.Etcd.DefaultWatchCacheSize = 0 | |||
There was a problem hiding this comment.
This is what is setting us to "off by default", right?
| @@ -507,6 +505,20 @@ func buildKubeApiserverConfig( | |||
| return originLongRunningRequestRE.MatchString(r.URL.Path) || kubeLongRunningFunc(r, requestInfo) | |||
| } | |||
|
|
|||
| if apiserverOptions.Etcd.EnableWatchCache { | |||
| glog.V(2).Infof("Initializing cache sizes based on %dMB limit", apiserverOptions.GenericServerRunOptions.TargetRAMMB) | |||
| sizes := cachesize.NewHeuristicWatchCacheSizes(apiserverOptions.GenericServerRunOptions.TargetRAMMB) | |||
There was a problem hiding this comment.
do we set this target RAMMB to anything by default?
There was a problem hiding this comment.
do we set this target RAMMB to anything by default?
I'm not seeing where we write a default here, which would set all the heuristic ones to zero by default, right?
There was a problem hiding this comment.
There's a "min" function on the heuristic so we always get something even at 0
|
Anything else? |
|
ping @deads2k |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, smarterclayton The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue (batch tested with PRs 16546, 16398, 16157) |
|
@smarterclayton: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@openshift/svt |
Disables the watch cache for most resources by default, except those accessed by many clients. This has been shown to have minor impacts on the production workload.
Fixes #16112