VStream: Prevent buffering entire transactions (OOM risk), instead send chunks to client

[twthorn]

Description

There is a bug that causes OOM errors for vtgate when a very large transaction (e.g., multi-GB) but with many reasonably sized operations is sent over VStream.

The problem is caused by this logic. We buffer with eventss the entire transaction before sending it. Very large transactions eg multi-GB can cause OOM errors. Example described here

it was from an 11GB size transaction (11 MB per row * 1000 rows). So it seems there was some issue with breaking this transaction up between tablet to vtgate and vtgate to client. We are still looking into it, but wanted to share early findings

This PR aims to fix this by allowing for locking across multiple received event batches from a tablet. And thus allows for sending chunked transactions even before the COMMIT is received, while still preserving the order for the VStream (ie even for multi-shard, the transactions cannot be interleaved, each transaction is sent in its entirety before sending the next transaction of any shard).

I am open to putting this behind a flag. There may be performance implication from this additional locking.

Another approach may be a size in bytes like vstream_packet_size for a tablet, but for the vtgate. If that size in bytes is exceeded a lock is acquired, and then we will start sending the transaction as chunks (and stop accumulating it in memory). Open to discussion on this.

Testing

For reproduction, I added a test that fails without this change (asserts that transactions should NOT be accumulated before sending):

tthornton at tthornt-ltm9dwz in ~/git/twthorn/vitess on fix_vtgate_oom [$]
$ go test -v ./go/vt/vtgate -run TestVStreamLargeTransactionMemory 2>&1 | tail -30
=== RUN   TestVStreamLargeTransactionMemory
E1103 15:49:17.252008   26813 vstream_manager.go:442] Error in vstream for keyspace:"TestVStream" shard:"-20" gtid:"pos": context canceled
context ended while streaming from TestVStream/-20
    vstream_manager_test.go:2403: Max memory used: 57 MB
    vstream_manager_test.go:2407:
                Error Trace:    /Users/tthornton/git/twthorn/vitess/go/vt/vtgate/vstream_manager_test.go:2407
                Error:          "57" is not less than "25"
                Test:           TestVStreamLargeTransactionMemory
                Messages:       Memory usage should stay low due to immediate transaction chunk sending. VTGate should not accumulate all 50 chunks (~50MB) before sending.
--- FAIL: TestVStreamLargeTransactionMemory (0.01s)
FAIL
FAIL    vitess.io/vitess/go/vt/vtgate   1.147s
FAIL

With these changes, the test passes.

Docs PR: vitessio/website#2028

Related Issue(s)

• Fixes: Bug Report: VTGates OOM on large transactions #18850

Checklist

• "Backport to:" labels have been added if this change should be back-ported to release branches
• If this change is to be back-ported to previous releases, a justification is included in the PR description
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added or is not required

Deployment Notes

AI Disclosure

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.

[codecov]

Codecov Report

❌ Patch coverage is 80.32787% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.81%. Comparing base (79af4c1) to head (0fa4663).
⚠️ Report is 42 commits behind head on main.

Files with missing lines	Patch %	Lines
go/vt/vtgate/vstream_manager.go	80.32%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #18849      +/-   ##
==========================================
+ Coverage   69.73%   69.81%   +0.07%     
==========================================
  Files        1608     1610       +2     
  Lines      214776   215360     +584     
==========================================
+ Hits       149781   150347     +566     
- Misses      64995    65013      +18     

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

[Copilot]

Pull request overview

This PR addresses a critical OOM (Out Of Memory) issue in VTGate when streaming very large transactions (multi-GB) through VStream. The root cause was that VTGate buffered entire transactions in memory before sending them to clients, even when transactions were chunked from tablets.

Key Changes:

• Introduces a configurable transaction chunk size threshold (default 128MB) that triggers lock-based contiguous delivery for large transactions
• Implements dynamic lock acquisition when transactions exceed the threshold to ensure non-interleaved delivery across shards while still allowing chunked transmission
• Adds comprehensive unit and e2e tests to verify chunking behavior and prevent transaction interleaving

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
proto/vtgate.proto	Adds transaction_chunk_size field to VStreamFlags for configurable chunking threshold
go/vt/proto/vtgate/vtgate.pb.go	Generated protobuf code for the new transaction_chunk_size field
go/vt/proto/vtgate/vtgate_vtproto.pb.go	Generated vtproto code for serialization/deserialization of transaction_chunk_size
go/vt/vtgate/vstream_manager.go	Core implementation: adds transaction state tracking, dynamic lock acquisition for large transactions, and chunked event sending
go/vt/vtgate/vstream_manager_test.go	New unit test verifying that large transactions from one shard don't interleave with events from other shards
go/test/endtoend/vreplication/vstream_test.go	Updates e2e tests with 1KB chunk size to ensure chunking is actually tested
go/test/endtoend/vreplication/initial_data_test.go	Adds helper function to insert large transactions for testing chunking behavior
go/test/endtoend/vreplication/vreplication_test.go	Minor refactoring to move connection initialization earlier in test function

---

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

[mattlord]

I also added the needs website docs label as we'll need to update this page: https://vitess.io/docs/24.0/reference/vreplication/vstream/

[mattlord]

At this point there may already be interleaved events, no? Is that a problem?

[twthorn]

There will actually not be interleaved events.

This is because current/default behavior is to send transactions atomically, only once all events from BEGIN to COMMIT are accumulated.

There are two cases when we reach this code:

1. The lock is not held - this means that other streams are sending transactions atomically. When we go to acquire the lock, atomic transactions may have completed just before us. But they are atomic so it's fine. All other streams will be halted while we send our chunked transaction.
2. The lock is held - this means another shard is holding the lock and chunking. In this case we wait until that shard has finished its transaction. Then we acquire the lock, thus the complete, chunked transaction of another shard will be sent prior to us beginning to chunk our shard's transaction.

In either case, the events between different transactions of different shards are not interleaved. The only interleaving happens at the transaction-level, ie whole transactions interleaved across shards (not inter-transaction event level)

[mattlord]

I'm approving now, although I'd like to discuss my comments before we merge (still need a second reviewer). Please let me know what you think.

Nice work on this, @twthorn ! I appreciate your patience and persistence on this. ❤️

[mattlord]

Do we still need this now that the flag's default is also 0? I guess it makes it cleaner when we later make it opt-out. Totally fine to leave it here.

[twthorn]

Yes, I think it's good to have it there for future ease of updating, and i added a comment related to that. It also makes the code explicit and self documenting