Skip to content

http: stream WebSocket rows instead of buffering#35394

Merged
antiguru merged 1 commit intoMaterializeInc:mainfrom
antiguru:ws_optimization
Mar 10, 2026
Merged

http: stream WebSocket rows instead of buffering#35394
antiguru merged 1 commit intoMaterializeInc:mainfrom
antiguru:ws_optimization

Conversation

@antiguru
Copy link
Member

Previously, WebSocket::add_result collected all result rows into a Vec<WebSocketResponse> before sending any over the wire.
Each enum instance is sized to the largest variant (~150 bytes for Error/Notice), but Row only needs ~24 bytes.
For large result sets this caused substantial memory overhead.

Stream rows directly over the websocket as they're produced, matching how the Subscribe path already works.

🤖 Generated with Claude Code

Previously, `WebSocket::add_result` collected all result rows into a
`Vec<WebSocketResponse>` before sending any. Each enum instance is sized
to the largest variant (~150 bytes for Error/Notice), but Row only needs
~24 bytes. For large result sets this caused substantial memory overhead.

Stream rows directly over the websocket as they're produced, matching
how the Subscribe path already works.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@antiguru antiguru requested a review from a team as a code owner March 10, 2026 13:45
@antiguru antiguru requested a review from ggevay March 10, 2026 13:45
@github-actions
Copy link

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

  • Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
  • Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
  • Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

@antiguru antiguru requested a review from aljoscha March 10, 2026 13:49
Copy link
Contributor

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@aljoscha aljoscha left a comment

Choose a reason for hiding this comment

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

Looks fine, if ci likes it we are good to merge!

@antiguru antiguru merged commit 6264071 into MaterializeInc:main Mar 10, 2026
127 checks passed
@antiguru antiguru deleted the ws_optimization branch March 10, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants