Skip to content

Fix subscription replay after queue deactivation - #19034

Closed
Caideyipi wants to merge 2 commits into
apache:masterfrom
Caideyipi:fix/subscription-requeue-inflight-on-deactivation
Closed

Caideyipi wants to merge 2 commits into
apache:masterfrom
Caideyipi:fix/subscription-requeue-inflight-on-deactivation

Conversation

@Caideyipi

Copy link
Copy Markdown
Collaborator

Problem

When a consensus subscription queue was deactivated during leader changes or queue removal, it discarded prefetched and in-flight events. The next poll used client-reported progress, which advances when an event is observed and can be ahead of its ACK. Those events were then skipped without a WAL-gap signal.

Fix

  • Initialize replay from the subscription commit manager's committed progress after a queue reset, rather than the poll progress hint.
  • Add a regression test proving that an unacknowledged event is replayed after deactivation and reactivation.

This preserves at-least-once delivery across queue lifecycle resets while retaining the existing ACK-based commit frontier.

Validation

  • mvn clean test -T 2 -Dtest=ConsensusPrefetchingQueueDeliveryTest,ConsensusPrefetchingQueueTest -Dsurefire.failIfNoSpecifiedTests=false -pl iotdb-core/datanode -am -P with-integration-tests
  • 67 tests passed, 0 failures, 0 errors.

@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Initial self-review passed with no actionable findings; no fixes were needed. Verified with clean-source reactor tests: 67 tests passed, with 0 failures and 0 errors, including the new deactivation replay regression test.

@Caideyipi
Caideyipi marked this pull request as ready for review October 28, 2026 02:35
@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Independent re-review completed for the full current PR diff and relevant context. No source-behavior or test-logic issues were found.

One comment-only finding was fixed in e40e0f0: ConsensusPrefetchingQueue.java:315 still described initialization using a poll progress hint. The comment now describes successful initialization from committed progress (or its fallback), matching the implementation.

Validation: mvn spotless:apply -P with-integration-tests completed successfully, and git diff --check passed. The author-provided remote clean-reactor result is 67 tests passed, 0 failures and 0 errors. The new ConsensusPrefetchingQueueDeliveryTest#testDeactivationReplaysUncommittedEventInsteadOfUsingPollProgress has not been run locally; tests were not rerun for this comment-only fix.

The remote head was verified as e40e0f0. This PR remains open and non-draft under the author's explicit comment-only exception. This session performed one review round.

@Caideyipi
Caideyipi marked this pull request as draft October 29, 2026 03:37
@Caideyipi Caideyipi closed this Oct 29, 2026
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