Skip to content

fix: raise default WebSocket payload limit to avoid dropping large result frames#6858

Open
aicam wants to merge 1 commit into
apache:mainfrom
aicam:fix/websocket-payload-limit
Open

fix: raise default WebSocket payload limit to avoid dropping large result frames#6858
aicam wants to merge 1 commit into
apache:mainfrom
aicam:fix/websocket-payload-limit

Conversation

@aicam

@aicam aicam commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Raises the default maximum workflow WebSocket message size from 64 KB to 1024 KB (1 MB), in both places the default is defined:

  • common/config/src/main/resources/application.conf — the base config default (web-server.max-workflow-websocket-request-payload-size-kb)
  • bin/k8s/values.yaml — the Helm chart env default (MAX_WORKFLOW_WEBSOCKET_REQUEST_PAYLOAD_SIZE_KB), which overrides the base default in Kubernetes deployments

WebsocketPayloadSizeTuner applies this value to the Jetty WebSocket container's max text/binary message buffer. At the previous 64 KB default, the result/status frame pushed to the browser for a workflow with many operators or a sizable visualization payload (e.g. a Bar Chart) could exceed 65536 bytes; Jetty then drops the entire frame with MessageTooLargeException, so the final result/visualization never reaches the frontend even though the execution completes successfully on the backend. The overshoot is often only a couple KB, so even modest workflows can trip it. 1 MB gives ample headroom for large visualization payloads while remaining well within reasonable WebSocket frame sizes.

This changes only a default value; the limit remains overridable via the MAX_WORKFLOW_WEBSOCKET_REQUEST_PAYLOAD_SIZE_KB environment variable.

How was this PR tested?

Manually. Ran a workflow ending in a Bar Chart whose serialized result frame exceeded 64 KB:

  • Before: the visualization did not render, and the computing unit logged MessageTooLargeException: ... too large for configured max of [65536] at completion.
  • After: the frame is delivered and the chart renders.

Also verified with helm template that the rendered chart env value updates to 1024 and the chart still renders cleanly.

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

Generated-by: Claude Opus 4.8

Any related issues (either this closes or is dependent on)?

Closes #6857

…sult frames

The default max workflow WebSocket message size of 64 KB is too small for
result/status frames from workflows with many operators or a sizable
visualization payload (e.g. Bar Chart). Such frames exceed 65536 bytes and
Jetty drops the whole frame with MessageTooLargeException, so the final
result/visualization never reaches the browser even though the execution
completes successfully. Raise the default to 1024 KB in both the base config
and the Helm chart env default to give ample headroom.

Signed-off-by: ali <ali.risheh876@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

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

  • Contributors with relevant context: @kunwp1
    You can notify them by mentioning @kunwp1 in a comment.

@codecov-commenter

codecov-commenter commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.93%. Comparing base (a351f44) to head (f86d665).
⚠️ Report is 3 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (a351f44) and HEAD (f86d665). Click for more details.

HEAD has 6 uploads less than BASE
Flag BASE (a351f44) HEAD (f86d665)
agent-service 2 0
pyamber 2 0
frontend 2 0
Additional details and impacted files
@@              Coverage Diff              @@
##               main    #6858       +/-   ##
=============================================
- Coverage     77.30%   46.93%   -30.37%     
+ Complexity     3524      229     -3295     
=============================================
  Files          1161       60     -1101     
  Lines         45922     2663    -43259     
  Branches       5101      330     -4771     
=============================================
- Hits          35501     1250    -34251     
+ Misses         8840     1292     -7548     
+ Partials       1581      121     -1460     
Flag Coverage Δ
access-control-service 70.66% <ø> (+0.66%) ⬆️
agent-service ?
amber 27.48% <ø> (-41.62%) ⬇️
computing-unit-managing-service 16.12% <ø> (-4.37%) ⬇️
config-service 63.57% <ø> (-3.10%) ⬇️
file-service 62.37% <ø> (-4.84%) ⬇️
frontend ?
notebook-migration-service 78.94% <ø> (ø)
pyamber ?
workflow-compiling-service 55.00% <ø> (-0.15%) ⬇️

☔ 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.

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main a351f44 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 375 0.229 25,623/35,260/35,260 us 🟢 -12.0% / 🔴 +112.5%
🔴 bs=100 sw=10 sl=64 784 0.478 127,781/145,769/145,769 us 🔴 +6.6% / 🔴 +33.6%
bs=1000 sw=10 sl=64 889 0.542 1,122,792/1,166,915/1,166,915 us ⚪ within ±5% / 🔴 +12.0%
Baseline details

Latest main a351f44 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 375 tuples/sec 388 tuples/sec 754.55 tuples/sec -3.4% -50.3%
bs=10 sw=10 sl=64 MB/s 0.229 MB/s 0.237 MB/s 0.461 MB/s -3.4% -50.3%
bs=10 sw=10 sl=64 p50 25,623 us 23,040 us 12,816 us +11.2% +99.9%
bs=10 sw=10 sl=64 p95 35,260 us 40,058 us 16,594 us -12.0% +112.5%
bs=10 sw=10 sl=64 p99 35,260 us 40,058 us 19,806 us -12.0% +78.0%
bs=100 sw=10 sl=64 throughput 784 tuples/sec 812 tuples/sec 969.38 tuples/sec -3.4% -19.1%
bs=100 sw=10 sl=64 MB/s 0.478 MB/s 0.496 MB/s 0.592 MB/s -3.6% -19.2%
bs=100 sw=10 sl=64 p50 127,781 us 119,886 us 103,584 us +6.6% +23.4%
bs=100 sw=10 sl=64 p95 145,769 us 150,783 us 109,097 us -3.3% +33.6%
bs=100 sw=10 sl=64 p99 145,769 us 150,783 us 117,304 us -3.3% +24.3%
bs=1000 sw=10 sl=64 throughput 889 tuples/sec 901 tuples/sec 1,004 tuples/sec -1.3% -11.4%
bs=1000 sw=10 sl=64 MB/s 0.542 MB/s 0.55 MB/s 0.613 MB/s -1.5% -11.5%
bs=1000 sw=10 sl=64 p50 1,122,792 us 1,113,408 us 1,002,357 us +0.8% +12.0%
bs=1000 sw=10 sl=64 p95 1,166,915 us 1,177,564 us 1,046,463 us -0.9% +11.5%
bs=1000 sw=10 sl=64 p99 1,166,915 us 1,177,564 us 1,073,661 us -0.9% +8.7%
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,533.66,200,128000,375,0.229,25623.31,35260.09,35260.09
1,100,10,64,20,2551.47,2000,1280000,784,0.478,127780.83,145769.01,145769.01
2,1000,10,64,20,22505.37,20000,12800000,889,0.542,1122792.19,1166915.39,1166915.39

@kunwp1

kunwp1 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Is there any downside of increasing the default size to 1MB?

@aicam

aicam commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Is there any downside of increasing the default size to 1MB?

We were using this in a session with users with workflows with more than 10 operators, they all failed to run and get result of workflow, after increasing this number, all fixed. So I believe there is no downside since was tested in a live session with >10 users.

@aicam
aicam requested review from kunwp1 and removed request for mengw15 July 24, 2026 21:09
Comment thread bin/k8s/values.yaml
- name: SCHEDULE_GENERATOR_ENABLE_COST_BASED_SCHEDULE_GENERATOR
value: "true"
- name: MAX_WORKFLOW_WEBSOCKET_REQUEST_PAYLOAD_SIZE_KB
value: "64"

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.

why are those values kept twice in the source code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

One is config which has default value and one is in Kubernetes which overwrite local config. Some important config envs are in Kubernetes setting.

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.

ok. I assume the ones in kubernetes is a subset of overriding the defaults.

@Yicong-Huang

Copy link
Copy Markdown
Contributor

@aicam do we need this in v1.2?

cc @xuang7

@aicam

aicam commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

@aicam do we need this in v1.2?

cc @xuang7

@aicam do we need this in v1.2?

cc @xuang7

I think so, the symptom we had today was that users click on Run button and nothing happen. It happened in workflows which have heavy data for example a visualization operator which has more than 64kb data.

@aicam
aicam requested a review from Yicong-Huang July 24, 2026 21:19
@xuang7 xuang7 added the release/v1.2 back porting to release/v1.2 label Jul 24, 2026

@kunwp1 kunwp1 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!

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

Labels

common fix infra release/v1.2 back porting to release/v1.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Large result/visualization frames exceed the default WebSocket message limit and are silently dropped

5 participants