Skip to content

cpvm: fix cpu usage for console vm when using vnc over websockets - #6970

Merged
weizhouapache merged 1 commit into
apache:4.18from
alexandru-bagu:console-vm-cpu-usage-fix
Aug 14, 2023
Merged

weizhouapache merged 1 commit into
apache:4.18from
alexandru-bagu:console-vm-cpu-usage-fix

Conversation

@alexandru-bagu

@alexandru-bagu alexandru-bagu commented Dec 10, 2022

Copy link
Copy Markdown
Contributor

Description

On a VMware ESXi 7 environment the Console VM would always use 100% cpu after one connection and it would not stop until a VM restart (or well cloud service restart inside of the VM).

This PR fixes this issue by adding a Thread.sleep(1).

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@sonarqubecloud

Copy link
Copy Markdown

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell D 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@codecov

codecov Bot commented Dec 10, 2022

Copy link
Copy Markdown

Codecov Report

Merging #6970 (0c85543) into 4.18 (66cbe0a) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               4.18    #6970   +/-   ##
=========================================
  Coverage     12.93%   12.93%           
  Complexity     8941     8941           
=========================================
  Files          2715     2715           
  Lines        256107   256107           
  Branches      39938    39938           
=========================================
  Hits          33128    33128           
  Misses       218820   218820           
  Partials       4159     4159           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@yadvr yadvr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, sleeping for 1ms; however I'm not sure if this could cause/introduce some kind of latency? cc @nvazquez

@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@sonarqubecloud

Copy link
Copy Markdown

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell C 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@yadvr yadvr added this to the 4.19.0.0 milestone Apr 19, 2023

@yadvr yadvr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs testing but otherwise LGTM

@yadvr
yadvr requested a review from shwstppr April 19, 2023 07:41
@yadvr

yadvr commented Apr 19, 2023

Copy link
Copy Markdown
Member

cc @nvazquez
@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5934

@shwstppr

shwstppr commented May 1, 2023

Copy link
Copy Markdown
Contributor

Tested this with a VMware 7 env and I'm not able to see any difference in the CPU usage.
This is what I did:

  • Setup a 4.18 env and started console on a couple of VMs. Soon CPU usage for CPVM was 100%
  • Upgraded env to this PR's packages and started console on a couple of VMs. CPU usage for CPVM again reached 100% within a minute or two.
  • After restarting cloud service on the CPVM CPU usage was <10%.

Not sure if we need to increase the sleep time or use a different approach. I'll try to test
@alexandru-bagu can you please have a look as well? Also, can you please rebase your change to 4.18 branch so that once we have a working fix it can go into 4.18.1

cc @rohityadavcloud @nvazquez

CPU usage from vCenter (Lows only when cloud service is restarted)
Screenshot from 2023-05-01 16-15-38

@yadvr

yadvr commented May 23, 2023

Copy link
Copy Markdown
Member

cc @nvazquez to review
@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rohityadavcloud a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6110

@yadvr

yadvr commented Aug 8, 2023

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@rohityadavcloud a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@yadvr

yadvr commented Aug 8, 2023

Copy link
Copy Markdown
Member

@blueorangutan test alma8 vmware-7u3

@blueorangutan

Copy link
Copy Markdown

@rohityadavcloud [SF] unsupported parameters provided. Supported mgmt server os are: centos7, centos6, suse15, alma8, ubuntu18, ubuntu22, ubuntu20, rocky8, alma9. Supported hypervisors are: kvm-centos6, kvm-centos7, kvm-rocky8, kvm-alma8, kvm-alma9, kvm-ubuntu18, kvm-ubuntu20, kvm-ubuntu22, kvm-suse15, vmware-55u3, vmware-60u2, vmware-65u2, vmware-67u3, vmware-70u1, vmware-70u2, vmware-70u3, vmware-80, vmware-80u1, xenserver-65sp1, xenserver-71, xenserver-74, xcpng74, xcpng76, xcpng80, xcpng81, xcpng82

@yadvr

yadvr commented Aug 9, 2023

Copy link
Copy Markdown
Member

@blueorangutan test alma8 vmware-70u3

@blueorangutan

Copy link
Copy Markdown

@rohityadavcloud a [SF] Trillian-Jenkins test job (alma8 mgmt + vmware-70u3) has been kicked to run smoke tests

@weizhouapache

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@weizhouapache a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6749

@yadvr

yadvr commented Aug 11, 2023

Copy link
Copy Markdown
Member

@blueorangutan test alma8 vmware-70u3

@blueorangutan

Copy link
Copy Markdown

@rohityadavcloud a [SF] Trillian-Jenkins test job (alma8 mgmt + vmware-70u3) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-7389)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server a8
Total time taken: 43568 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6970-t7389-vmware-70u3.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

updateFrontEndActivityTime();
}
connectionAlive = client.isVncOverWebSocketConnectionAlive();
try {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a nested try should be in a separate method

@weizhouapache

Copy link
Copy Markdown
Member

Tested this with a VMware 7 env and I'm not able to see any difference in the CPU usage. This is what I did:

  • Setup a 4.18 env and started console on a couple of VMs. Soon CPU usage for CPVM was 100%
  • Upgraded env to this PR's packages and started console on a couple of VMs. CPU usage for CPVM again reached 100% within a minute or two.
  • After restarting cloud service on the CPVM CPU usage was <10%.

Not sure if we need to increase the sleep time or use a different approach. I'll try to test @alexandru-bagu can you please have a look as well? Also, can you please rebase your change to 4.18 branch so that once we have a working fix it can go into 4.18.1

cc @rohityadavcloud @nvazquez

CPU usage from vCenter (Lows only when cloud service is restarted) Screenshot from 2023-05-01 16-15-38

@shwstppr
thanks for the testing
do you think #7826 will fix the issue ?

@weizhouapache weizhouapache changed the title fix cpu usage for console vm when using vnc over websockets cpvm: fix cpu usage for console vm when using vnc over websockets Aug 14, 2023
@weizhouapache

Copy link
Copy Markdown
Member

@alexandru-bagu
can you test #7826 ?

@alexandru-bagu

Copy link
Copy Markdown
Contributor Author

I haven't had the CPU issue after I applied my own patch for it. I don't know how to trigger the cpu usage for either issues though.
Testing that commit would yield no new information for me.

@weizhouapache

Copy link
Copy Markdown
Member

I haven't had the CPU issue after I applied my own patch for it. I don't know how to trigger the cpu usage for either issues though. Testing that commit would yield no new information for me.

ok , thanks @alexandru-bagu

@nvazquez @shwstppr @DaanHoogland @rohityadavcloud
do you think we should merge this #6970 or #7826 or both ? the vmware7 issue might be fixed by #7826 as well

@DaanHoogland

Copy link
Copy Markdown
Contributor

@nvazquez @shwstppr @DaanHoogland @rohityadavcloud do you think we should merge this #6970 or #7826 or both ? the vmware7 issue might be fixed by #7826 as well

By the looks of the code both would help, but I do not know whether that would be overkill. Is it worth testing all four combinations of those two patches? cc @JoaoJandre @alexandru-bagu

@yadvr

yadvr commented Aug 14, 2023

Copy link
Copy Markdown
Member

I've hit this issue on vmware7u3, now I no longer have the env; but this should be easy to test in our lab cc @weizhouapache @DaanHoogland

@weizhouapache

Copy link
Copy Markdown
Member

verified ok

without this PR, the CPU usage increased to 100% even when only 1 vm console is open.
with this PR, the cpu usage is still low when 10 vm consoles are open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants