Skip to content

[Subscription] Release uncommitted tablet payloads with WAL replay - #18866

Merged
jt2594838 merged 4 commits into
masterfrom
fix/subscription-wal-backed-inflight
Oct 16, 2026
Merged

jt2594838 merged 4 commits into
masterfrom
fix/subscription-wal-backed-inflight

Conversation

@Caideyipi

@Caideyipi Caideyipi commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Delayed manual ACKs currently keep every delivered consensus tablet batch materialized, so a consumer can exhaust the broker's tablet budget before it finishes a transaction, batch, or checkpoint. This change releases a delivered batch's tablets and response cache after serialization, while retaining its commit context and a compact delivery record. Releasing the payload does not advance committed progress.

Delivery and replay

  • Record exact batch members using delta/zigzag varints, together with the original batch shape, memory charge, and frozen column projection. Redelivery preserves sparse membership and replicated writer records without a local search index.
  • Release delivered payloads without an extra WAL validation scan, including batches containing pending entries. Check WAL availability and fragment completeness only when redelivery actually needs to rebuild the payload.
  • On NACK, timeout, or disconnect, reserve the original materialization budget and rebuild the batch through an independent WAL iterator. Unavailable, incomplete, or unreadable WAL keeps the delivery record and returns the reserved memory for a later retry, without advancing committed progress.
  • Fence serialization, payload replacement, cleanup, and late delivery callbacks with the event monitor and a response identity for each poll attempt. Seek and close clear the delivery indexes.
  • Protect WAL needed by live delivery records, remove those records after ACK, and refresh the deletion boundary. Restart recovery continues from committed writer positions.

Resource tradeoffs

Payload reclamation adds no WAL reads. Redelivery adds disk reads and materialization work. Delivery metadata remains proportional to the number of outstanding batches; member coordinates are compressed and records retain neither tablets nor serialized payloads. The existing single-entry size limit still applies.

Validation

  • DataNode and dependency reactor main/test-source compilation passes. Local validation excludes the existing, unrelated PipeMemoryManagerResizeTest compilation issue; no build-file change is included in this PR.
  • Focused subscription/WAL regression: 151 tests, 0 failures, 0 errors, 1 existing skipped test, covering delivery, conversion, ACK state, initialization, seek, memory admission, WAL backpressure, the receiver, and the WAL iterator.
  • All 12 delivery tests pass, including consuming ten batches with a two-batch memory budget before committing, out-of-order/late ACKs, redelivery after reclamation, deferred WAL visibility and incomplete fragments, sparse/replicated members, restart recovery, seek reset, and serialization/cleanup fencing. Pending and mixed-batch regressions verify that reclamation performs no extra WAL scan and failed redelivery releases its memory reservation before retrying.
  • Spotless, Checkstyle, and git diff --check pass.

This PR has:

  • been self-reviewed for concurrent read, concurrent write, and concurrent read/write.
  • added comments explaining the lifecycle and WAL retention decisions.
  • added unit tests covering new delivery and replay paths.
Key changed classes

ConsensusPrefetchingQueue, ConsensusLogToTabletConverter, SubscriptionEvent, SubscriptionEventSingleResponse, CachedSubscriptionPollResponse, and SubscriptionReceiverV1.

@codecov

codecov Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 277 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.93%. Comparing base (0a79a49) to head (35c92bd).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
...on/broker/consensus/ConsensusPrefetchingQueue.java 0.00% 228 Missing ⚠️
...iotdb/db/subscription/event/SubscriptionEvent.java 0.00% 27 Missing ⚠️
...roker/consensus/ConsensusLogToTabletConverter.java 0.00% 9 Missing ⚠️
...vent/response/SubscriptionEventSingleResponse.java 0.00% 9 Missing ⚠️
.../subscription/receiver/SubscriptionReceiverV1.java 0.00% 3 Missing ⚠️
...on/event/cache/CachedSubscriptionPollResponse.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18866      +/-   ##
============================================
- Coverage     45.94%   45.93%   -0.01%     
  Complexity      712      712              
============================================
  Files          5502     5502              
  Lines        399066   399420     +354     
  Branches      52019    52101      +82     
============================================
+ Hits         183346   183479     +133     
- Misses       215720   215941     +221     

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

Comment on lines +2800 to +2809
private void reclaimDeliveredPayload(
final SubscriptionEvent event, final DeliveryBatchRecord record) {
if (event.isPayloadDetached() || !record.delivered) {
return;
}
// Pending entries can precede durable WAL visibility. Verify every member and fragment before
// releasing the only payload copy. This read does not change replay or committed progress.
if (!record.replayable && Objects.isNull(readDeliveryBatch(event, record, false))) {
return;
}

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.

Will readDeliveryBatch(event, record, false) be too costly here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes—the previous path repeated a WAL scan/deserialization in the poll delivery callback. Changed it to reuse the initial complete WAL read (including all fragments and the seal, with no incomplete scan), propagated across every batch member; WAL-backed delivery now opens no extra iterator.

Pending/partial batches keep their payload and are validated by the prefetch worker after delivery. Successful validation remains cached across redeliveries. This preserves the WAL-readability guard while moving pending validation off the RPC path.

Added three regressions for reader reuse, deferred validation, and mixed-batch safety. Focused subscription/WAL validation: 151 tests, 0 failures/errors, 1 existing skip.

@jt2594838 jt2594838 left a comment

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.

Most of the entries should be pending ones, and checking them is still too costly in most scenarios.

@jt2594838
jt2594838 merged commit c6f86b5 into master Oct 16, 2026
36 of 44 checks passed
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