Skip to content

Release gRPC stream call messages that are no longer needed - #730

Open
Myllyenko wants to merge 1 commit into
ydb-platform:masterfrom
Myllyenko:fix/stream-call-retention
Open

Myllyenko wants to merge 1 commit into
ydb-platform:masterfrom
Myllyenko:fix/stream-call-retention

Conversation

@Myllyenko

Copy link
Copy Markdown

Release gRPC stream call messages that are no longer needed

Problem

Two gRPC stream call classes in core keep possibly large messages in memory long after they're needed:

  • ReadWriteStreamCall queues outgoing messages in messagesQueue while the transport isn't ready to send, for example when the network is slow. Once the call is half-closed, cancelled, or closed by the server, those messages can never be sent: gRPC's ClientCall.isReady() returns false after halfClose(), so flush() never empties the queue. But nothing cleared the queue either, and messages passed to sendNext() after closing were added to it too.
    • The messages stayed in memory until the call object was garbage-collected. On a stalled connection, a half-closed call can live until the keepalive timeout or the deadline.
    • For the topic writer, each queued item is a WriteRequest possibly dozens megabytes-long. After a reconnect the writer re-sends the same message data on a new stream, so the dead queue kept the payloads in memory even after the server acknowledged them.
  • ReadStreamCall stores its request in a final field for the whole lifetime of the stream, but only uses it once, in start(). For server-streaming calls with large requests (for example, queries with big parameters), the request stayed in memory until the last response arrived.

Fix

  • ReadWriteStreamCall: close(), cancel() and onClose() now mark the call as stopped and clear the queue, and sendNext() drops messages once the call is stopped. Nothing changes in what gets delivered: these messages were never sent before either.
  • ReadStreamCall: the request field is set to null right after sendMessage().

@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.97%. Comparing base (3d9dc0c) to head (e426ce5).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...n/java/tech/ydb/core/impl/call/ReadStreamCall.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #730      +/-   ##
============================================
+ Coverage     74.59%   74.97%   +0.38%     
- Complexity     3640     3671      +31     
============================================
  Files           392      393       +1     
  Lines         16554    16618      +64     
  Branches       1748     1757       +9     
============================================
+ Hits          12349    12460     +111     
+ Misses         3592     3532      -60     
- Partials        613      626      +13     

☔ View full report in Codecov by Harness.
📢 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.

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.

1 participant