Set deserialization cache size based on target memory usage #34000

wojtek-t · 2016-10-04T07:24:46Z

Special notes for your reviewer:

This is the PR we talked about yesterday.

Release note:

To reduce memory usage to reasonable levels in smaller clusters, kube-apiserver now sets the deserialization cache size based on the target memory usage.

This change is

wojtek-t · 2016-10-04T07:25:43Z

cmd/kube-apiserver/app/server.go

+		// target memory usage.
+		glog.V(2).Infof("Initalizing deserialization cache size based on %dMB limit", s.TargetRAMMB)
+
+		// This is the heuristics that from memory capacity is trying to infer


FYI - This is pretty-much copy from:
https://github.com/kubernetes/kubernetes/blob/master/pkg/registry/cachesize/cachesize.go#L75

k8s-ci-robot · 2016-10-04T08:34:35Z

Jenkins GCI GCE e2e failed for commit 7852734. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

lavalamp · 2016-10-04T18:11:30Z

pkg/genericapiserver/options/server_run_options.go

-		DeserializationCacheSize: DefaultDeserializationCacheSize,
+		// Default cache size to 0 - if unset, its size will be set based on target
+		// memory usage.
+		DeserializationCacheSize: 0,


This will break federation apiserver if they use this cache?

lavalamp · 2016-10-04T18:15:08Z

cmd/kube-apiserver/app/server.go

+		// on that value.
+		// From our documentation, we officially recomment 120GB machines for
+		// 2000 nodes, and we scale from that point. Thus we assume ~60MB of
+		// capacity per node.


our max object size is 900kb, IIRC. I would suggest an alternate heuristic, which is to decide e.g. 1/3 of this memory will be used for the deserialization cache, and then divide by the max object size to see how big the cache should be.

(It would of course be better to just actually measure the collective sizes of the objects in the cache but that's much much harder. A TODO mentioning this might be good.)

I also considered it (when thinking about it for the sizes of caches in "Cacher"), but decided that it's not the best option for now. First of all, because the target memory is not super accurate now (it actually is more-or-less target memory for all components on master machine). Second, because it will result in waaaaay bigger memory usage for larger clusters, which is also not very good in my opinion.

So I would prefer to leave it as is (especially since we want to cherrypick) and probably solve it better later (for 1.5 maybe?). [Though I extended the TODO so that it contains what you basically wrote].

OK, I think we should probably reconsider in the future (it would be good to explicitly set how much memory is allowed for this, so admins get predictable usage) but I guess this is still a big improvement, so I won't block it over this.

jessfraz · 2016-10-04T18:45:45Z

shouldn't this have a release note for the changelog for 1.4.1

lavalamp · 2016-10-04T19:40:52Z

I updated the release note in the OP

jessfraz · 2016-10-05T01:20:32Z

thanks!

k8s-ci-robot · 2016-10-05T08:52:32Z

Jenkins GCE Node e2e failed for commit 2bfcb1a. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

lavalamp · 2016-10-05T19:43:37Z

federation/cmd/federation-apiserver/app/server.go

@@ -82,6 +82,10 @@ func Run(s *options.ServerRunOptions) error {
 	// TODO: register cluster federation resources here.
 	resourceConfig := genericapiserver.NewResourceConfig()

+	if s.StorageConfig.DeserializationCacheSize == 0 {
+		// When size of cache is not explicitly set, set it to 50000
+		s.StorageConfig.DeserializationCacheSize = 50000


@quinton-hoole @nikhiljindal how much memory do you expect federation apiserver to consume?

lavalamp · 2016-10-05T19:45:06Z

/lgtm

jessfraz · 2016-10-05T20:02:08Z

@k8s-bot test this issue #32430

k8s-github-robot · 2016-10-05T20:46:53Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2016-10-05T21:23:39Z

Automatic merge from submit-queue

jessfraz · 2016-10-05T22:47:54Z

@wojtek-t does this need a cherrypick PR to 1.4

wojtek-t · 2016-10-06T07:41:12Z

@jessfraz - yes we want this cherrypick. I've opened #34213

jessfraz · 2016-10-06T17:28:21Z

thanks!

@lavalamp

…00-upstream-release-1.4 Automatic merge from submit-queue Cherrypick: Set deserialization cache size based on target memory usage Cherrypick of #34000 **Release note**: ```release-note To reduce memory usage to reasonable levels in smaller clusters, kube-apiserver now sets the deserialization cache size based on the target memory usage. ``` @lavalamp - FYI

k8s-cherrypick-bot · 2016-10-06T19:23:41Z

Commit found in the "release-1.4" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

@lavalamp

…ck-of-#34000-upstream-release-1.4 Automatic merge from submit-queue Cherrypick: Set deserialization cache size based on target memory usage Cherrypick of kubernetes#34000 **Release note**: ```release-note To reduce memory usage to reasonable levels in smaller clusters, kube-apiserver now sets the deserialization cache size based on the target memory usage. ``` @lavalamp - FYI

wojtek-t added the release-note-none Denotes a PR that doesn't merit a release note. label Oct 4, 2016

wojtek-t added this to the v1.4 milestone Oct 4, 2016

wojtek-t assigned lavalamp Oct 4, 2016

googlebot added the cla: yes label Oct 4, 2016

wojtek-t added cherrypick-candidate and removed cla: yes labels Oct 4, 2016

k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 4, 2016

wojtek-t commented Oct 4, 2016

View reviewed changes

lavalamp reviewed Oct 4, 2016

View reviewed changes

lavalamp added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Oct 4, 2016

Set deserialization cache size based on target memory usage

2bfcb1a

wojtek-t force-pushed the set_cache_size branch from 7852734 to 2bfcb1a Compare October 5, 2016 08:00

googlebot added the cla: yes label Oct 5, 2016

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 5, 2016

lavalamp reviewed Oct 5, 2016

View reviewed changes

k8s-github-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 5, 2016

k8s-github-robot merged commit bd3664c into kubernetes:master Oct 5, 2016

jessfraz added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 6, 2016

wojtek-t mentioned this pull request Oct 6, 2016

Cherrypick: Set deserialization cache size based on target memory usage #34213

Merged

k8s-cherrypick-bot removed the cherrypick-candidate label Oct 6, 2016

sjenning mentioned this pull request Oct 26, 2016

reduce default deserialization cache size openshift/origin#10719

Closed

wojtek-t deleted the set_cache_size branch November 23, 2016 14:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set deserialization cache size based on target memory usage #34000

Set deserialization cache size based on target memory usage #34000

wojtek-t commented Oct 4, 2016 •

edited by lavalamp

Loading

wojtek-t Oct 4, 2016

k8s-ci-robot commented Oct 4, 2016

lavalamp Oct 4, 2016

wojtek-t Oct 5, 2016

lavalamp Oct 4, 2016

wojtek-t Oct 5, 2016

lavalamp Oct 5, 2016

jessfraz commented Oct 4, 2016

lavalamp commented Oct 4, 2016

jessfraz commented Oct 5, 2016

k8s-ci-robot commented Oct 5, 2016

lavalamp Oct 5, 2016

lavalamp commented Oct 5, 2016

jessfraz commented Oct 5, 2016

k8s-github-robot commented Oct 5, 2016

k8s-github-robot commented Oct 5, 2016

jessfraz commented Oct 5, 2016

wojtek-t commented Oct 6, 2016

jessfraz commented Oct 6, 2016

k8s-cherrypick-bot commented Oct 6, 2016

Set deserialization cache size based on target memory usage #34000

Set deserialization cache size based on target memory usage #34000

Conversation

wojtek-t commented Oct 4, 2016 • edited by lavalamp Loading

wojtek-t Oct 4, 2016

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 4, 2016

lavalamp Oct 4, 2016

Choose a reason for hiding this comment

wojtek-t Oct 5, 2016

Choose a reason for hiding this comment

lavalamp Oct 4, 2016

Choose a reason for hiding this comment

wojtek-t Oct 5, 2016

Choose a reason for hiding this comment

lavalamp Oct 5, 2016

Choose a reason for hiding this comment

jessfraz commented Oct 4, 2016

lavalamp commented Oct 4, 2016

jessfraz commented Oct 5, 2016

k8s-ci-robot commented Oct 5, 2016

lavalamp Oct 5, 2016

Choose a reason for hiding this comment

lavalamp commented Oct 5, 2016

jessfraz commented Oct 5, 2016

k8s-github-robot commented Oct 5, 2016

k8s-github-robot commented Oct 5, 2016

jessfraz commented Oct 5, 2016

wojtek-t commented Oct 6, 2016

jessfraz commented Oct 6, 2016

k8s-cherrypick-bot commented Oct 6, 2016

wojtek-t commented Oct 4, 2016 •

edited by lavalamp

Loading