Feat: memory optimization via enhance informer cache #5683

Somefive · 2023-03-15T09:13:53Z

Description of your changes

Apply 3 methods to optimize the memory usage.

Drop managedFields and kubectl.kubernetes.io/last-applied-configuration when storing Application, ApplicationRevision, ResourceTracker in informer. (This only affect the cache, etcd storage not change)
Reduce the memory usage of ApplicationRevision by sharing the storage of ComponentDefinition, TraitDefinition, WorkflowStepDefinition. (Let them use the same pointer if the hash equals)
Disable the informer cache for ConfigMap caused by Workflow Context. (At the cost of slower Workflow Context updates)

Before Optimization

After Optimization

3k app, 9k apprev, RSS 1.02G(Alloc: 401M, Sys: 897M) -> 739M(Alloc: 203M, Sys: 707M), enhance 25%~30%.

Refactor:

Remove unused flags for optimization.

I have:

Read and followed KubeVela's contribution process.
Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
Run make reviewable to ensure this PR is ready for review.
Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Special notes for your reviewer

codecov · 2023-03-15T09:38:26Z

Codecov Report

Patch coverage: 90.38% and project coverage change: -0.18 ⚠️

Comparison is base (b28b165) 61.59% compared to head (1eaa863) 61.42%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5683      +/-   ##
==========================================
- Coverage   61.59%   61.42%   -0.18%     
==========================================
  Files         216      217       +1     
  Lines       30442    30582     +140     
==========================================
+ Hits        18751    18785      +34     
- Misses       9976    10080     +104     
- Partials     1715     1717       +2

Flag	Coverage Δ
core-unittests	`55.76% <80.00%> (+0.02%)`	⬆️
e2e-multicluster-test	`25.85% <88.46%> (+0.19%)`	⬆️
e2e-rollout-tests	`22.40% <88.46%> (+0.46%)`	⬆️
e2etests	`27.63% <88.46%> (-0.35%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/features/controller_features.go	`100.00% <ø> (ø)`
pkg/workflow/workflow.go	`100.00% <ø> (ø)`
pkg/cache/informer.go	`88.80% <88.80%> (ø)`
cmd/core/app/server.go	`59.09% <100.00%> (+0.18%)`	⬆️
pkg/appfile/parser.go	`63.76% <100.00%> (-0.52%)`	⬇️
pkg/cache/optimize.go	`62.50% <100.00%> (+9.86%)`	⬆️
...er/core.oam.dev/v1alpha2/application/dispatcher.go	`94.40% <100.00%> (ø)`
...ller/core.oam.dev/v1alpha2/application/revision.go	`71.48% <100.00%> (-0.88%)`	⬇️
pkg/controller/flags.go	`100.00% <100.00%> (ø)`

... and 10 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

charts/vela-core/values.yaml

wonderflow · 2023-03-16T02:58:05Z

pkg/cache/informer.go

+}
+
+// NewClientDisableCacheFor get ClientDisableCacheFor for building controller
+func NewClientDisableCacheFor() []client.Object {


Suggested change

func NewClientDisableCacheFor() []client.Object {

func Resources2DisableCache() []client.Object {

This is aligned with the naming of controller-runtime manager. See

kubevela/cmd/core/app/server.go

Line 165 in afc5c8c

ClientDisableCacheFor: cache.NewClientDisableCacheFor(),

they're different, the naming is trying to become a sentence:

Client disable cache for "some resources", so the field should be "some resources", not copy-pase the name.

wonderflow · 2023-03-16T03:05:39Z

charts/vela-core/values.yaml

  enableInMemoryWorkflowContext: false
  disableResourceApplyDoubleCheck: false
  enableResourceTrackerDeleteOnlyTrigger: true
+  informerCache: true
+  applicationRevisionDefinitionStorage: true
+  disableWorkflowContextConfigMapCache: true


Shall we provide a new optimize section for these fine-grained cache items?

Suggested change

disableWorkflowContextConfigMapCache: true

disableCacheItems:

disableWorkflowContextConfigMapCache: false

BTW, the parameter name is confusion, it's default to disable the cache or not?

I think it should be

Suggested change

disableWorkflowContextConfigMapCache: true

disableCacheItems:

workflowContextConfigMapCache: false

Changed to default enabled featuregate

charts/vela-core/values.yaml

wonderflow · 2023-03-16T03:10:50Z

pkg/cache/optimize.go

@@ -60,6 +62,9 @@ func BuildCache(ctx context.Context, scheme *runtime.Scheme, shardingObjects ...
 				return nil, err
 			}
 		}
+		if OptimizeInformerCache && OptimizeApplicationRevisionDefinitionStorage {
+			go DefaultDefinitionCache.Get().Start(ctx, c, ApplicationRevisionDefinitionCachePruneDuration)


does all cache using a go routine? can we align with them?

yes. all definition cache use one single goroutine.

pkg/controller/flags.go

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

barnettZQG

LGTM

Somefive requested review from chivalryq, wonderflow, FogDong, barnettZQG, leejanee and jefree-cat as code owners March 15, 2023 09:13

Somefive marked this pull request as draft March 15, 2023 09:14

Somefive force-pushed the feat/support-apprev-def-cache branch 7 times, most recently from 4a4fad3 to afc5c8c Compare March 16, 2023 01:52

Somefive marked this pull request as ready for review March 16, 2023 02:31

wonderflow reviewed Mar 16, 2023

View reviewed changes

charts/vela-core/values.yaml Outdated Show resolved Hide resolved

wonderflow reviewed Mar 16, 2023

View reviewed changes

charts/vela-core/values.yaml Outdated Show resolved Hide resolved

barnettZQG reviewed Mar 16, 2023

View reviewed changes

charts/vela-core/values.yaml Outdated Show resolved Hide resolved

wonderflow reviewed Mar 16, 2023

View reviewed changes

Somefive force-pushed the feat/support-apprev-def-cache branch 2 times, most recently from 538c98c to 1a4a5e9 Compare March 16, 2023 03:04

wonderflow reviewed Mar 16, 2023

View reviewed changes

pkg/controller/flags.go Show resolved Hide resolved

Somefive force-pushed the feat/support-apprev-def-cache branch from a7a2f0a to c28b89d Compare March 16, 2023 04:10

wonderflow approved these changes Mar 17, 2023

View reviewed changes

Somefive force-pushed the feat/support-apprev-def-cache branch from c28b89d to 6e42ec2 Compare March 17, 2023 09:14

Somefive added 2 commits March 17, 2023 17:14

Feat: support apprev definition cache

ed9b31e

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

Feat: ignore last applied configuration and managed fields

331fb80

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

Somefive added 5 commits March 17, 2023 17:14

refactor

07c0338

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

revert object cache

0fddad9

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

refactor cache

e4d4733

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

add workflow context cache optimization

731ab05

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

refactor to use featuregate

1eaa863

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

Somefive force-pushed the feat/support-apprev-def-cache branch from 6e42ec2 to 1eaa863 Compare March 17, 2023 09:15

FogDong approved these changes Mar 20, 2023

View reviewed changes

barnettZQG approved these changes Mar 20, 2023

View reviewed changes

chivalryq merged commit 50458bc into kubevela:master Mar 20, 2023

Somefive deleted the feat/support-apprev-def-cache branch June 20, 2023 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: memory optimization via enhance informer cache #5683

Feat: memory optimization via enhance informer cache #5683

Somefive commented Mar 15, 2023 •

edited

Loading

codecov bot commented Mar 15, 2023 •

edited

Loading

wonderflow Mar 16, 2023

Somefive Mar 16, 2023

wonderflow Mar 16, 2023

Somefive Mar 16, 2023

wonderflow Mar 16, 2023

chivalryq Mar 16, 2023

Somefive Mar 16, 2023

wonderflow Mar 16, 2023

Somefive Mar 16, 2023

barnettZQG left a comment

	func NewClientDisableCacheFor() []client.Object {
	func Resources2DisableCache() []client.Object {

	disableWorkflowContextConfigMapCache: true
	disableCacheItems:
	disableWorkflowContextConfigMapCache: false

	disableWorkflowContextConfigMapCache: true
	disableCacheItems:
	workflowContextConfigMapCache: false

Feat: memory optimization via enhance informer cache #5683

Feat: memory optimization via enhance informer cache #5683

Conversation

Somefive commented Mar 15, 2023 • edited Loading

Description of your changes

How has this code been tested

Special notes for your reviewer

codecov bot commented Mar 15, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barnettZQG left a comment

Choose a reason for hiding this comment

Somefive commented Mar 15, 2023 •

edited

Loading

codecov bot commented Mar 15, 2023 •

edited

Loading