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.

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

Reduce CSI proxy CPU usage #197

Merged
merged 2 commits into from
Feb 7, 2022

Conversation

pradeep-hegde
Copy link
Contributor

@pradeep-hegde pradeep-hegde commented Feb 2, 2022

What type of PR is this?
/kind bug

What this PR does / why we need it:
This change will reduce CPU usage of csi proxy.
Util method to run PowerShell scripts is created. Powershell is invoked with a set of parameters to load and reduce CPU usage.

Which issue(s) this PR fixes:

Partially Fixes #193

Special notes for your reviewer:
CPU usage in the node before this change

PS C:\Users\Administrator> Get-Process | Sort-Object CPU -Descending | Where-Object { $_.CPU -gt 100 }

Handles  NPM(K)    PM(K)      WS(K)     CPU(s)     Id  SI ProcessName
-------  ------    -----      -----     ------     --  -- -----------
    950      98   317244     236636  16,662.83   3256   0 MsMpEng
    623      32    65900      97100  10,761.61   6372   0 kubelet
    616      27     8960      16880   5,448.06   5748   0 vmcompute
   9767      32    63784      78268   4,509.66   3364   0 dockerd
  17637       0      192        136   2,590.88      4   0 System
   1880      20    14792      28340   1,657.06   5676   0 WmiPrvSE
    250      16     9748      18932   1,175.44   8676   0 svchost
    245      16    24576      20384   1,045.70   3236   0 csi-proxy
    558      57    17976      26932     845.13   6044   0 svchost
    652      17     7512      13988     804.33    576   0 svchost
    376      13    11788      16104     465.58   1316   0 svchost
    470      17    17520      26712     416.34   3076   0 svchost
    206      11     2344       8532     353.38   2608   0 svchost
   1025      25     9456      18988     315.16    904   0 lsass
    331      16    43404      40788     200.98   3552   0 svchost
    589      12     6164      10864     170.83    896   0 services
    163      15    34348      43324     128.17  10060  24 csi
    407      24    12380      24336     110.31   3100   0 vmtoolsd
    600      21     2428       5444     100.59    652   0 csrss

CPU usage in one of the nodes with this change

PS C:\Users\Administrator> Get-Process | Sort-Object CPU -Descending | Where-Object { $_.CPU -gt 100 }  

Handles  NPM(K)    PM(K)      WS(K)     CPU(s)     Id  SI ProcessName
-------  ------    -----      -----     ------     --  -- -----------
    501      29    62628      94044   5,303.22   4192   0 kubelet
    569      25     8200      14656   2,648.83   5984   0 vmcompute
   6131      28    59208      75296   1,715.52   3712   0 dockerd
  17306       0      196        132     958.36      4   0 System
    482      20    15480      28612     803.17   4880   0 WmiPrvSE
    278      17     8752      19524     441.97  10932   0 svchost
    536      54    12764      23084     382.58   5564   0 svchost
    632      17     6876      13400     378.16   1016   0 svchost
    235      17    22692      19708     360.34   3420   0 csi-proxy
    465      17    17256      26844     204.75   3216   0 svchost
   1025      25     8476      17684     200.31    748   0 lsass
    359      13    11168      14988     198.03   1256   0 svchost
    264      26     5724      15220     178.78   3212   0 svchost
    209      11     2300       8652     124.41   2364   0 svchost

Note:

  • Above info was collected on the same hardware
  • Windows Defender was disabled to reduce overall system CPU usage

Does this PR introduce a user-facing change?:

Reduced CPU usage by CSI proxy in windows nodes

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 2, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @pradeep-hegde. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 2, 2022
@mauriciopoppe
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 2, 2022
@pradeep-hegde
Copy link
Contributor Author

/kind bug

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Feb 2, 2022
@pradeep-hegde
Copy link
Contributor Author

/remove-kind feature

@k8s-ci-robot k8s-ci-robot removed the kind/feature Categorizes issue or PR as related to a new feature. label Feb 2, 2022
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 3, 2022
@pradeep-hegde pradeep-hegde reopened this Feb 3, 2022
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 3, 2022
@pradeep-hegde
Copy link
Contributor Author

/assign @xing-yang

@xing-yang
Copy link

/assign @jingxu97

Copy link
Contributor

@ddebroy ddebroy left a comment

Choose a reason for hiding this comment

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

Looks great! One suggestion to change the parameters for the util.

pkg/utils/utils.go Outdated Show resolved Hide resolved
Copy link
Contributor

@ddebroy ddebroy left a comment

Choose a reason for hiding this comment

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

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ddebroy, pradeep-hegde

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 4, 2022
@ddebroy
Copy link
Contributor

ddebroy commented Feb 4, 2022

Leaving lgtm to @mauriciopoppe / @jingxu97

@jingxu97
Copy link
Contributor

jingxu97 commented Feb 7, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 7, 2022
@k8s-ci-robot k8s-ci-robot merged commit 556e1ea into kubernetes-csi:master Feb 7, 2022
@pradeep-hegde pradeep-hegde deleted the perf-fix branch February 8, 2022 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

High cpu usage of powershell processes triggered by csi-proxy
6 participants