Skip to content

Conversation

rohityadavcloud
Copy link
Member

@rohityadavcloud rohityadavcloud commented Jul 16, 2025

Description

This fixes the console proxy input and output buffer sizes to accommodate responsive user-experience for desktop users using console (novnc).

Fixes #10650

How I tested:

console-improvements.mov

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)
  • build/CI
  • test (unit or integration test code)

Copy link

codecov bot commented Jul 16, 2025

Codecov Report

Attention: Patch coverage is 1.08696% with 91 lines in your changes missing coverage. Please review.

Project coverage is 16.16%. Comparing base (c61a5eb) to head (e878bbe).
Report is 42 commits behind head on 4.20.

Files with missing lines Patch % Lines
...om/cloud/consoleproxy/ConsoleProxyNoVncClient.java 0.00% 48 Missing ⚠️
...leproxy/vnc/network/NioSocketSSLEngineManager.java 0.00% 18 Missing ⚠️
...consoleproxy/vnc/network/NioSocketInputStream.java 0.00% 10 Missing ⚠️
...main/java/com/cloud/consoleproxy/ConsoleProxy.java 20.00% 4 Missing ⚠️
...consoleproxy/vnc/network/NioSocketHandlerImpl.java 0.00% 4 Missing ⚠️
...n/java/com/cloud/consoleproxy/vnc/NoVncClient.java 0.00% 2 Missing ⚠️
.../com/cloud/consoleproxy/vnc/network/NioSocket.java 0.00% 2 Missing ⚠️
...oleproxy/vnc/network/NioSocketTLSOutputStream.java 0.00% 2 Missing ⚠️
...soleproxy/vnc/network/NioSocketTLSInputStream.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               4.20   #11221      +/-   ##
============================================
+ Coverage     16.14%   16.16%   +0.01%     
- Complexity    13256    13277      +21     
============================================
  Files          5656     5656              
  Lines        497893   497818      -75     
  Branches      60374    60375       +1     
============================================
+ Hits          80394    80457      +63     
+ Misses       408539   408407     -132     
+ Partials       8960     8954       -6     
Flag Coverage Δ
uitests 4.00% <ø> (-0.01%) ⬇️
unittests 17.01% <1.08%> (+0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DaanHoogland
Copy link
Contributor

looks good, @rohityadavcloud . jus one question (i might misunderstand the architectural aspect though)

This bumps the input and output stream buffers to 64KiB and uses them
consistent across TLS and non-TLS based VNC connections.

This fixes apache#10650

Co-authored-by: Vishesh Jindal <[email protected]>
Signed-off-by: Rohit Yadav <[email protected]>
@rohityadavcloud rohityadavcloud requested a review from nvazquez July 16, 2025 11:10
@rohityadavcloud
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@rohityadavcloud a [SL] 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.

@weizhouapache
Copy link
Member

amazing findings ! @vishesh92 @rohityadavcloud

@rohityadavcloud
Copy link
Member Author

@vishesh92 can you set the PR to 'ready for review' once you're done with your experimentations if any

@rohityadavcloud rohityadavcloud linked an issue Jul 16, 2025 that may be closed by this pull request
@sureshanaparti sureshanaparti modified the milestones: 4.20.2, 4.21.0 Jul 16, 2025
@adietrich-ussignal
Copy link

Is there value in making this setting tunable via a system configuration setting?

@blueorangutan
Copy link

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

@rohityadavcloud
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

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

@rohityadavcloud
Copy link
Member Author

@adietrich-ussignal currently that’s not possible, these are internal byte buffers which weren’t consistently coded in the first place and without this code-based fix it won’t possible to fix existing ACS environments.

@vishesh92 vishesh92 requested a review from Copilot July 17, 2025 07:09
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes buffer sizes for console proxy connections to improve performance and responsiveness for desktop users accessing VNC consoles through NoVNC. The changes address issue #10650 by implementing configurable buffer sizes, adaptive batching strategies, and socket optimizations.

Key changes include:

  • Introduction of configurable default buffer size (65536 bytes) replacing hardcoded values
  • Implementation of adaptive batching for TLS connections with immediate flushing for non-TLS
  • Socket-level optimizations including TCP_NODELAY and keep-alive settings

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
consoleproxy.properties Adds configurable defaultBufferSize property (65536 bytes)
ConsoleProxy.java Implements buffer size configuration parsing and static field
NioSocketTLSOutputStream.java Replaces SSL session buffer size with configurable size, adds batching logic
NioSocketTLSInputStream.java Updates to use configurable buffer size instead of SSL session size
NioSocketSSLEngineManager.java Implements adaptive batching with explicit flush control
NioSocketInputStream.java Refactors byte reading methods for better performance
NioSocketHandlerImpl.java Updates to use configurable buffer size and new API methods
NioSocketHandler.java Simplifies interface by replacing two methods with single buffer-based method
NioSocket.java Enables TCP optimizations (keep-alive, TCP_NODELAY)
NoVncClient.java Updates to use new buffer-based reading API
ConsoleProxyNoVncClient.java Implements adaptive sleep timing and TLS-aware batching strategy

@vishesh92
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] 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

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 14237

@vishesh92
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] 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

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

@vishesh92
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] 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

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

@rohityadavcloud
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

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

@sureshanaparti
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-13842)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 59103 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11221-t13842-kvm-ol8.zip
Smoke tests completed. 141 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@blueorangutan
Copy link

[SF] Trillian test result (tid-13844)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 60153 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11221-t13844-kvm-ol8.zip
Smoke tests completed. 140 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_deployVMInSharedNetwork Failure 478.92 test_network.py

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

lgtm, checked the consoles for some instances in vmware/kvm/xenserver.

@sureshanaparti sureshanaparti merged commit 111d87b into apache:4.20 Jul 24, 2025
25 of 26 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Apache CloudStack 4.21.0 Jul 24, 2025
@DaanHoogland DaanHoogland deleted the console-proxy-performance branch July 24, 2025 11:07
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Aug 6, 2025
…#11221)

* console-proxy: fix stream buffer sizes to improve console performance

This bumps the input and output stream buffers to 64KiB and uses them
consistent across TLS and non-TLS based VNC connections.

This fixes apache#10650

Co-authored-by: Vishesh Jindal <[email protected]>
Signed-off-by: Rohit Yadav <[email protected]>

* Make buffer size configurable & other improvements for CPU & memory utilisation

* Setup batching of data for TLS connections to the VNC server

* Apply suggestions from code review

* Fix buffer size for xenserver

---------

Signed-off-by: Rohit Yadav <[email protected]>
Co-authored-by: Vishesh Jindal <[email protected]>
Co-authored-by: vishesh92 <[email protected]>
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Aug 25, 2025
…#11221)

* console-proxy: fix stream buffer sizes to improve console performance

This bumps the input and output stream buffers to 64KiB and uses them
consistent across TLS and non-TLS based VNC connections.

This fixes apache#10650

Co-authored-by: Vishesh Jindal <[email protected]>
Signed-off-by: Rohit Yadav <[email protected]>

* Make buffer size configurable & other improvements for CPU & memory utilisation

* Setup batching of data for TLS connections to the VNC server

* Apply suggestions from code review

* Fix buffer size for xenserver

---------

Signed-off-by: Rohit Yadav <[email protected]>
Co-authored-by: Vishesh Jindal <[email protected]>
Co-authored-by: vishesh92 <[email protected]>
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Sep 4, 2025
…#11221)

* console-proxy: fix stream buffer sizes to improve console performance

This bumps the input and output stream buffers to 64KiB and uses them
consistent across TLS and non-TLS based VNC connections.

This fixes apache#10650

Co-authored-by: Vishesh Jindal <[email protected]>
Signed-off-by: Rohit Yadav <[email protected]>

* Make buffer size configurable & other improvements for CPU & memory utilisation

* Setup batching of data for TLS connections to the VNC server

* Apply suggestions from code review

* Fix buffer size for xenserver

---------

Signed-off-by: Rohit Yadav <[email protected]>
Co-authored-by: Vishesh Jindal <[email protected]>
Co-authored-by: vishesh92 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Windows Instance console user experience
8 participants