Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

performance optimization pub #955

Merged
merged 1 commit into from
May 6, 2022
Merged

performance optimization pub #955

merged 1 commit into from
May 6, 2022

Conversation

zmberg
Copy link
Member

@zmberg zmberg commented Apr 19, 2022 •
edited
Loading

Signed-off-by: liheng.zms liheng.zms@alibaba-inc.com

Ⅰ. Describe what this PR does

Performance optimization for the pub GetPubForPod method:

  1. pub event handler reconcile pub controller patch 'related-pub' in pod annotations
  2. GetPubForPod uses pod 'related-pod' annotation to get pub name directly.

@kruise-bot kruise-bot added the size/XL size/XL: 500-999 label Apr 19, 2022
@zmberg zmberg requested review from veophi and removed request for shiyan2016 April 19, 2022 02:21
@kruise-bot kruise-bot added size/XXL and removed size/XL size/XL: 500-999 labels Apr 19, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2022 •
edited
Loading

Codecov Report

Merging #955 (8e048cf) into master (5b52b6e) will decrease coverage by 0.14%.
The diff coverage is 26.35%.

@@            Coverage Diff             @@
##           master     #955      +/-   ##
==========================================
- Coverage   49.99%   49.85%   -0.15%     
==========================================
  Files         119      119              
  Lines       11535    11566      +31     
==========================================
- Hits         5767     5766       -1     
- Misses       4896     4930      +34     
+ Partials      872      870       -2
Flag Coverage Δ
unittests 49.85% <26.35%> (-0.15%) ⬇️

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

Impacted Files Coverage Δ
pkg/control/pubcontrol/utils.go 0.00% <0.00%> (-18.94%) ⬇️
pkg/control/sidecarcontrol/util_hotupgrade.go 0.00% <0.00%> (ø)
pkg/controller/cloneset/sync/api.go 0.00% <0.00%> (ø)
pkg/controller/cloneset/sync/cloneset_update.go 48.24% <0.00%> (ø)
pkg/controller/sidecarset/sidecarset_hotupgrade.go 64.28% <0.00%> (ø)
.../webhook/pod/mutating/pod_create_update_handler.go 0.00% <0.00%> (ø)
...ebhook/pod/validating/pod_create_update_handler.go 0.00% <0.00%> (ø)
...availablebudget/podunavailablebudget_controller.go 33.13% <7.81%> (-6.59%) ⬇️
pkg/control/sidecarcontrol/sidecarset_control.go 36.63% <25.00%> (ø)
pkg/webhook/pod/mutating/sidecarset.go 74.70% <33.33%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b52b6e...8e048cf. Read the comment docs.

@kruise-bot kruise-bot added size/XL size/XL: 500-999 and removed size/XXL labels Apr 27, 2022
sonatype-lift[bot]
sonatype-lift bot reviewed Apr 27, 2022
View reviewed changes
pkg/util/controllerfinder/controller_finder.go
@@ -18,7 +18,6 @@ package controllerfinder

import (
"context"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goimports: File is not goimports-ed

(at-me in a reply with help or ignore)

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift[bot]
sonatype-lift bot reviewed Apr 27, 2022
View reviewed changes
pkg/controller/podunavailablebudget/pub_pod_event_handler.go Outdated
@@ -18,7 +18,8 @@ package podunavailablebudget

import (
"context"
"reflect"
"fmt"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goimports: File is not goimports-ed

(at-me in a reply with help or ignore)

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift[bot]
sonatype-lift bot reviewed Apr 27, 2022
View reviewed changes
pkg/controller/podunavailablebudget/podunavailablebudget_controller.go Outdated
@@ -20,6 +20,9 @@ import (
"context"
"flag"
"fmt"
utilclient "github.com/openkruise/kruise/pkg/util/client"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goimports: File is not goimports-ed

(at-me in a reply with help or ignore)

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

furykerry
furykerry reviewed Apr 29, 2022
View reviewed changes
pkg/controller/podunavailablebudget/pub_pod_event_handler.go Outdated
@@ -144,12 +119,30 @@ func (p *enqueueRequestForPod) updatePod(q workqueue.RateLimitingInterface, old,

}

func (p *enqueueRequestForPod) reconcilePatchPubAnnotationInPod(q workqueue.RateLimitingInterface, pod *corev1.Pod) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reconcilePatchPubAnnotationInPod -> enqueuePatchPubAnnotationRequest
this func does not do the reconcile

@zmberg
performance optimization pub
9604082 
Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com>
furykerry
furykerry reviewed Apr 29, 2022
View reviewed changes
Copy link
Member

@furykerry furykerry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@FillZpp
Copy link
Member

FillZpp commented May 6, 2022

/approve

@kruise-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FillZpp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kruise-bot kruise-bot merged commit a7d8d84 into openkruise:master May 6, 2022
@zmberg zmberg deleted the pub_optimization branch May 11, 2022 10:04
diannaowa pushed a commit to diannaowa/kruise that referenced this pull request Sep 14, 2022
@zmberg @diannaowa
performance optimization pub (openkruise#955)
efc358f 
Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com>
Signed-off-by: Liu Zhenwei <zwliu@thoughtworks.com>
ppbits pushed a commit to ppbits/kruise that referenced this pull request Apr 4, 2024
@zmberg
performance optimization pub (openkruise#955)
87ed264
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@furykerry furykerry furykerry left review comments

@sonatype-lift sonatype-lift sonatype-lift left review comments

@veophi veophi Awaiting requested review from veophi

Assignees

@furykerry furykerry

Labels
approved lgtm size/XL size/XL: 500-999
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zmberg @codecov-commenter @FillZpp @kruise-bot @furykerry