Skip to content

[Subscription] Fix pending consensus request memory leak - #18429

Open
Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/subscription-request-memory-leak
Open

[Subscription] Fix pending consensus request memory leak#18429
Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/subscription-request-memory-leak

Conversation

@Caideyipi

Copy link
Copy Markdown
Collaborator

Description

Bound pending consensus request memory

In a multi-replica region, LogDispatcher serializes each IndexedConsensusRequest before the same object is offered to subscription pending queues. A slow or paused subscription could therefore retain both the raw InsertNode and serialized ByteBuffer without participating in the IoTConsensus memory budget, causing heap growth and eventually DataNode fencing.

This PR:

  • makes request serialization idempotent and exposes the retained size as raw request memory plus serialized buffer capacity;
  • reuses the existing ref-counted IoTConsensusMemoryManager, so dispatchers and subscription queues sharing one request reserve its physical memory only once;
  • rejects subscription fast-path admission when the shared queue budget is exhausted, allowing the existing WAL replay path to recover the entry;
  • releases queue ownership after materialization in a finally block, and on offer failure, clear, deactivation, and close;
  • keeps batch accounting unchanged because replication batches no longer own the raw request objects.

Remove unnecessary retention

Remove IoTConsensusServerImpl.lastConsensusRequest, which kept the latest request, including a potentially large raw/serialized payload, reachable indefinitely.

Diagnostics

Expose retainedRequestBytes in the subscription queue core report to make pending-request retention observable.

Tests

  • IoTConsensusMemoryManagerTest: 4 tests passed.
  • ConsensusPrefetchingQueueTest, ConsensusPrefetchingQueueDataNodeMemoryTest, and ConsensusPrefetchingQueueWalBackpressureTest: 29 tests passed.
  • Spotless, Checkstyle, and git diff --check passed.

This PR has:

  • been self-reviewed.
    • concurrent write
  • added Javadocs for non-trivial memory accounting behavior.
  • added comments explaining ownership and release intent.
  • added or updated unit tests to cover new code paths.

Key changed/added classes
  • IndexedConsensusRequest
  • IoTConsensusMemoryManager
  • IoTConsensusServerImpl
  • ConsensusPrefetchingQueue

Comment on lines -160 to 162
private IndexedConsensusRequest lastConsensusRequest;

// Subscription queues receive IndexedConsensusRequest in real-time from write(),
// similar to LogDispatcher, enabling in-memory data delivery without waiting for WAL flush.

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.

This is for debugging purposes, where the request is stuck or fails to be found in the WAL.

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.

2 participants