Feat: optimize empty patch request #5600
Conversation
Somefive
requested review from
FogDong,
barnettZQG,
wonderflow,
leejanee and
jefree-cat
as code owners
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #5600 +/- ##
===========================================
- Coverage 61.56% 45.25% -16.31%
===========================================
Files 311 274 -37
Lines 48018 41176 -6842
===========================================
- Hits 29561 18636 -10925
- Misses 15391 20822 +5431
+ Partials 3066 1718 -1348
Flags with carried forward coverage won't be shown. Click here to find out more.
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 at Codecov. |
Somefive
force-pushed
the
feat/optimize-empty-patch-request
branch
5 times, most recently
from
5a9ce71
to
058beb3
Compare
| if patch == nil { | ||
| return true | ||
| } | ||
| data, _ := patch.Data(nil) |
There was a problem hiding this comment.
why pass a nil into the patch.Data?
There was a problem hiding this comment.
Because that is the interface parameter. The generated patch already contain data.
Reference
| @@ -632,3 +640,16 @@ func (r *Reconciler) matchControllerRequirement(app *v1beta1.Application) bool { | |||
| } | |||
| return true | |||
| } | |||
|
|
|||
| const ( | |||
| originalAppKey contextKey = "original-app" | |||
There was a problem hiding this comment.
| originalAppKey contextKey = "original-app" | |
| originalAppKey contextKey = "original-app-obj" |
There was a problem hiding this comment.
Refactored to use iota.
| originalAppKey contextKey = "original-app" | ||
| ) | ||
|
|
||
| func withOriginalApp(ctx context.Context, app *v1beta1.Application) context.Context { |
There was a problem hiding this comment.
can we make this function to be a method of ctrlrec.NewReconcileContext
There was a problem hiding this comment.
NewReconcileContext is not only used for application controller. It is used for all KubeVela eco-system controllers.
Somefive
force-pushed
the
feat/optimize-empty-patch-request
branch
from
058beb3
to
57939a1
Compare
|
|
||
| const ( | ||
| // ComponentNamespaceContextKey is the key in context that defines the override namespace of component | ||
| ComponentNamespaceContextKey contextKey = iota |
There was a problem hiding this comment.
will it be overriden by context[0] = "other-thing"？
There was a problem hiding this comment.
No. There is no int key 0. It will be pkg/controller/core.oam.dev/v1alpha2/application:contextKey(0).
Somefive
force-pushed
the
feat/optimize-empty-patch-request
branch
from
57939a1
to
cc7959c
Compare
Signed-off-by: Somefive <yd219913@alibaba-inc.com>
Somefive
force-pushed
the
feat/optimize-empty-patch-request
branch
from
cc7959c
to
6f0825e
Compare
Description of your changes
Enhance performance by reducing reconcile time from 56ms to 44ms, ~20% optimization.
I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested
Special notes for your reviewer