fix: raise default WebSocket payload limit to avoid dropping large result frames - #6858
Conversation
…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>
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6858 +/- ##
============================================
+ Coverage 78.98% 85.40% +6.42%
- Complexity 3785 4473 +688
============================================
Files 1160 1165 +5
Lines 46105 46639 +534
Branches 5115 5220 +105
============================================
+ Hits 36414 39830 +3416
+ Misses 8069 4982 -3087
- Partials 1622 1827 +205
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 375 | 0.229 | 26,265/36,038/36,038 us | 🔴 +157.0% / 🔴 +134.5% |
| 🔴 | bs=100 sw=10 sl=64 | 791 | 0.483 | 123,153/171,569/171,569 us | 🔴 +71.5% / 🔴 +62.1% |
| 🔴 | bs=1000 sw=10 sl=64 | 919 | 0.561 | 1,087,607/1,144,092/1,144,092 us | 🔴 +19.8% / 🔴 +13.7% |
Baseline details
Latest main 875aa72 from 2026-08-20T12:55:51.124Z
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 375 tuples/sec | 815.65 tuples/sec | 777.39 tuples/sec | -54.0% | -51.8% |
| bs=10 sw=10 sl=64 | MB/s | 0.229 MB/s | 0.498 MB/s | 0.474 MB/s | -54.0% | -51.7% |
| bs=10 sw=10 sl=64 | p50 | 26,265 us | 12,086 us | 12,655 us | +117.3% | +107.6% |
| bs=10 sw=10 sl=64 | p95 | 36,038 us | 14,022 us | 15,370 us | +157.0% | +134.5% |
| bs=10 sw=10 sl=64 | p99 | 36,038 us | 17,695 us | 19,088 us | +103.7% | +88.8% |
| bs=100 sw=10 sl=64 | throughput | 791 tuples/sec | 1,073 tuples/sec | 1,008 tuples/sec | -26.3% | -21.5% |
| bs=100 sw=10 sl=64 | MB/s | 0.483 MB/s | 0.655 MB/s | 0.615 MB/s | -26.2% | -21.5% |
| bs=100 sw=10 sl=64 | p50 | 123,153 us | 91,343 us | 99,054 us | +34.8% | +24.3% |
| bs=100 sw=10 sl=64 | p95 | 171,569 us | 100,032 us | 105,817 us | +71.5% | +62.1% |
| bs=100 sw=10 sl=64 | p99 | 171,569 us | 111,942 us | 112,935 us | +53.3% | +51.9% |
| bs=1000 sw=10 sl=64 | throughput | 919 tuples/sec | 1,098 tuples/sec | 1,046 tuples/sec | -16.3% | -12.1% |
| bs=1000 sw=10 sl=64 | MB/s | 0.561 MB/s | 0.67 MB/s | 0.638 MB/s | -16.3% | -12.1% |
| bs=1000 sw=10 sl=64 | p50 | 1,087,607 us | 908,045 us | 962,339 us | +19.8% | +13.0% |
| bs=1000 sw=10 sl=64 | p95 | 1,144,092 us | 963,132 us | 1,006,089 us | +18.8% | +13.7% |
| bs=1000 sw=10 sl=64 | p99 | 1,144,092 us | 1,000,430 us | 1,034,243 us | +14.4% | +10.6% |
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,532.71,200,128000,375,0.229,26265.22,36037.51,36037.51
1,100,10,64,20,2528.46,2000,1280000,791,0.483,123152.78,171568.87,171568.87
2,1000,10,64,20,21769.10,20000,12800000,919,0.561,1087607.00,1144092.29,1144092.29
|
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. |
|
Do we want to make the value dynamically configurable by the admins after the deployment? |
I don't think so, it can't be changed on the fly and is also very deployment specific, something that should be set at the time system was deployed. |
xuang7
added
release/v1.2
|
I was wondering if this PR is ready to merge. |
Yicong-Huang
left a comment
Yicong-Huang
left a comment
There was a problem hiding this comment.
🔴 2 must-fix · 2 advisory · 1 polish — the right fix, but two of the four places that encode the 64 KB default were missed; one of them is why the build is red.
Correctness (2)
application.conf:93— the raised default breaksApplicationConfigSpec'sshouldBe 64;build / amberis already red on it (must-fix, see inline)values.yaml:313—values-development.yamlstill pins"64", so the development chart keeps reproducing #6857 (must-fix, see inline)
Design & architecture (1)
- Above 1 MB the frame is still dropped silently with the run reported
COMPLETED. ShouldCloses #6857stand, or does the silent-failure half get a follow-up issue? (advisory)
Conventions (1)
- Description: the related-issues heading is a retired one; main's template now reads "Any related issues, documentation, discussions?" (advisory)
Polish (1)
WebsocketPayloadSizeTuner.scala:24— the// by default, 64 KiBcomment now contradicts the default (advisory; the file is not in the diff, so no inline anchor)
Verification trace
Checked why a red build coexists with green service checks: Config/jacoco is invoked only by the amber job (.github/workflows/build.yml:311). The platform (config-service) matrix entry runs ConfigService/jacoco — a different sbt project (build.sbt:121 Config vs build.sbt:128 ConfigService) — so it never executes ApplicationConfigSpec. Job 91020659550 reports Tests: succeeded 65, failed 1 / (Config / Test / test) sbt.TestsFailedException.
Also grepped every copy of the value: application.conf:93 and values.yaml:313 updated; values-development.yaml:310 and ApplicationConfigSpec.scala:134 not. No docker .env/compose copy and no frontend mirror exist.
Backport auto-label reportThis
|
…4 KB Addresses review feedback on apache#6858: - ApplicationConfigSpec still asserted the old 64 KB default, failing Config/test (the only job that runs it) - values-development.yaml still pinned "64", so deployments rendered from the development chart kept reproducing apache#6857 - WebsocketPayloadSizeTuner's inline comment still said 64 KiB Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016H6fPkH5EbPEkG9Au5TXsK
6 participants
What changes were proposed in this PR?
Raises the default maximum workflow WebSocket message size from 64 KB to 1024 KB (1 MB), in every place the default is encoded:
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 deploymentsbin/k8s/values-development.yaml— the same env var in the development chart, which otherwise kept reproducing Large result/visualization frames exceed the default WebSocket message limit and are silently dropped #6857common/config/src/test/scala/.../ApplicationConfigSpec.scala— the guard test asserting the defaultWebsocketPayloadSizeTunerapplies 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 withMessageTooLargeException, 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_KBenvironment variable.Any related issues, documentation, discussions?
Closes #6857
How was this PR tested?
Updated the existing guard test in
ApplicationConfigSpec(maxWorkflowWebsocketRequestPayloadSizeKb shouldBe 1024), which covers the base config default.Manually verified the runtime behavior. Ran a workflow ending in a Bar Chart whose serialized result frame exceeded 64 KB:
MessageTooLargeException: ... too large for configured max of [65536]at completion.Also verified with
helm templatethat the rendered chart env value updates to1024and that both charts still render cleanly.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.8