Skip to content

test(amber): cover WebSocket payload tuning - #7220

Merged
aglinxinyuan merged 4 commits into
apache:mainfrom
aglinxinyuan:test/websocket-payload-tuning
Aug 1, 2026
Merged

test(amber): cover WebSocket payload tuning#7220
aglinxinyuan merged 4 commits into
apache:mainfrom
aglinxinyuan:test/websocket-payload-tuning

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Adds focused unit coverage for servlet WebSocket container lookup and the configured text and binary payload limits, including exact one-KiB byte conversion.

Any related issues, documentation, discussions?

Closes #7217

How was this PR tested?

  • WorkflowExecutionService/testOnly org.apache.texera.web.resource.WebsocketPayloadSizeTunerSpec — 2 passed.
  • WorkflowExecutionService/Test/scalafmtCheck
  • WorkflowExecutionService/Test/scalafix --check

Mutation proof (each production mutation was reverted):

Production mutation Focused test result
Changed the text-buffer multiplier from 1024 to 1000 0 passed, 2 failed
Removed the binary-buffer setter 0 passed, 2 failed

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Codex (GPT-5)

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • No candidates found from git blame history.

@aglinxinyuan
aglinxinyuan requested review from Copilot and mengw15 August 1, 2026 06:23
@aglinxinyuan
aglinxinyuan marked this pull request as ready for review August 1, 2026 06:23

Copilot AI left a comment

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.

Pull request overview

Adds a focused ScalaTest/ScalaMock unit spec in Amber’s web resources to validate that WebsocketPayloadSizeTuner looks up the servlet ServerContainer correctly and applies configured WebSocket payload limits using exact KiB-to-bytes conversion.

Changes:

  • Added a new unit spec that asserts servlet ServerContainer is retrieved via classOf[ServerContainer].getName.
  • Added assertions that both text and binary buffer limits are set, including the exact 1 KiB == 1024 bytes boundary.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 3 worse · ⚪ 12 noise (<±5%) · 0 without baseline

Compared against main 18ebb0d benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
bs=10 sw=10 sl=64 382 0.233 25,261/36,755/36,755 us ⚪ within ±5% / 🔴 +126.9%
🔴 bs=100 sw=10 sl=64 759 0.463 129,295/163,000/163,000 us 🔴 +8.5% / 🔴 +47.1%
bs=1000 sw=10 sl=64 914 0.558 1,094,077/1,131,739/1,131,739 us ⚪ within ±5% / 🔴 -7.9%
Baseline details

Latest main 18ebb0d from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 382 tuples/sec 399 tuples/sec 765.25 tuples/sec -4.3% -50.1%
bs=10 sw=10 sl=64 MB/s 0.233 MB/s 0.243 MB/s 0.467 MB/s -4.1% -50.1%
bs=10 sw=10 sl=64 p50 25,261 us 24,650 us 12,703 us +2.5% +98.9%
bs=10 sw=10 sl=64 p95 36,755 us 36,194 us 16,199 us +1.5% +126.9%
bs=10 sw=10 sl=64 p99 36,755 us 36,194 us 19,557 us +1.5% +87.9%
bs=100 sw=10 sl=64 throughput 759 tuples/sec 810 tuples/sec 965.22 tuples/sec -6.3% -21.4%
bs=100 sw=10 sl=64 MB/s 0.463 MB/s 0.494 MB/s 0.589 MB/s -6.3% -21.4%
bs=100 sw=10 sl=64 p50 129,295 us 119,210 us 103,830 us +8.5% +24.5%
bs=100 sw=10 sl=64 p95 163,000 us 165,860 us 110,778 us -1.7% +47.1%
bs=100 sw=10 sl=64 p99 163,000 us 165,860 us 118,713 us -1.7% +37.3%
bs=1000 sw=10 sl=64 throughput 914 tuples/sec 913 tuples/sec 992.33 tuples/sec +0.1% -7.9%
bs=1000 sw=10 sl=64 MB/s 0.558 MB/s 0.557 MB/s 0.606 MB/s +0.2% -7.9%
bs=1000 sw=10 sl=64 p50 1,094,077 us 1,095,903 us 1,016,959 us -0.2% +7.6%
bs=1000 sw=10 sl=64 p95 1,131,739 us 1,144,811 us 1,062,501 us -1.1% +6.5%
bs=1000 sw=10 sl=64 p99 1,131,739 us 1,144,811 us 1,082,611 us -1.1% +4.5%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,523.84,200,128000,382,0.233,25261.06,36754.51,36754.51
1,100,10,64,20,2635.60,2000,1280000,759,0.463,129294.80,163000.03,163000.03
2,1000,10,64,20,21887.89,20000,12800000,914,0.558,1094077.47,1131738.99,1131738.99

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@codecov-commenter

codecov-commenter commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.08%. Comparing base (18ebb0d) to head (34cc174).
⚠️ Report is 8 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7220      +/-   ##
============================================
+ Coverage     81.04%   81.08%   +0.03%     
- Complexity     3968     3989      +21     
============================================
  Files          1162     1160       -2     
  Lines         46259    46145     -114     
  Branches       5153     5135      -18     
============================================
- Hits          37490    37415      -75     
+ Misses         7096     7059      -37     
+ Partials       1673     1671       -2     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from ac68ea4
agent-service 77.42% <ø> (-1.08%) ⬇️ Carriedforward from ac68ea4
amber 77.13% <ø> (+0.47%) ⬆️
computing-unit-managing-service 20.49% <ø> (ø) Carriedforward from ac68ea4
config-service 65.62% <ø> (-0.36%) ⬇️ Carriedforward from ac68ea4
file-service 66.59% <ø> (-0.22%) ⬇️ Carriedforward from ac68ea4
frontend 83.09% <ø> (-0.14%) ⬇️ Carriedforward from ac68ea4
notebook-migration-service 78.69% <ø> (-0.20%) ⬇️ Carriedforward from ac68ea4
pyamber 97.36% <ø> (-0.01%) ⬇️ Carriedforward from ac68ea4
workflow-compiling-service 26.31% <ø> (ø) Carriedforward from ac68ea4

*This pull request uses carry forward flags. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mengw15 mengw15 left a comment

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.

LGTM

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Xinyuan Lin <xinyual3@uci.edu>
@aglinxinyuan
aglinxinyuan enabled auto-merge August 1, 2026 07:01
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 1, 2026
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 1, 2026
Merged via the queue into apache:main with commit 268f098 Aug 1, 2026
21 checks passed
@aglinxinyuan
aglinxinyuan deleted the test/websocket-payload-tuning branch August 1, 2026 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tests for WebSocket payload size tuning

4 participants