Skip to content

Repair missing subscription brokers from consumer metadata - #19021

Merged
jt2594838 merged 4 commits into
masterfrom
fix/subscription-broker-self-healing
Oct 27, 2026
Merged

jt2594838 merged 4 commits into
masterfrom
fix/subscription-broker-self-healing

Conversation

@Caideyipi

Copy link
Copy Markdown
Collaborator

Summary

When a DataNode still has non-empty consumer-group metadata but its local subscription broker was removed by inactivity cleanup or a metadata race, the next handshake or metadata push now recreates the broker and repairs consensus subscriptions instead of leaving the provider at broker-missing status 1904.

Implementation

  • Added an idempotent SubscriptionConsumerAgent.ensureConsumerGroupBroker path serialized by the existing per-consumer-group lock.
  • Reconcile non-empty metadata during metadata updates and during handshake before activating the consumer.
  • Reuse createPipeBrokerIfNotExist and setupConsensusSubscriptions; duplicate brokers and consensus queues are prevented by the existing broker and queue guards.
  • Keep empty ConsumerGroupMeta empty. A group cleared before a later handshake is not guessed back into its previous topics; explicit subscription metadata remains authoritative.
  • Preserve existing brokers and queues during provider replacement. Provider replacement continues to close only the client session, with no subscription CLOSE request and no commit replay.

Tests

  • Added focused DataNode tests for broker recreation, empty metadata, handshake/metadata concurrency, canceled topics, and preservation of an existing broker queue.
  • mvn spotless:apply -P with-integration-tests -pl iotdb-core/datanode passed.
  • Focused Maven tests did not reach Surefire. The first run failed because the Windows JVM could not reserve the configured test heap; a bounded -DforkCount=0 retry failed in the Maven Checkstyle phase for the same system commit-memory limitation.
  • English and with-zh-locale incremental compiles reached javac but were blocked by pre-existing generated-source and dependency inconsistencies (IFill/IFillFilter/LinearFill, commons-collections4, and unrelated pipe/query API mismatches).

@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Self-review passed; no confirmed code issues were found.

The change adds serialized, idempotent broker and consensus-subscription recovery for non-empty consumer-group metadata during metadata reconciliation and handshake. Empty metadata remains non-recovering, canceled topics are not restored, existing queues are preserved, and provider replacement remains session-only without a subscription CLOSE or commit replay.

Verification:

  • mvn spotless:apply -P with-integration-tests -pl iotdb-core/datanode passed.
  • The focused DataNode tests were added but not executed because Maven failed before Surefire: the default run hit Windows commit-memory exhaustion, and the bounded no-fork retry hit the same limitation during Checkstyle.
  • English and with-zh-locale incremental compiles reached javac but failed on pre-existing generated-source and dependency/API mismatches outside this change.

@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Independent review of the full diff and the related subscription broker, consumer metadata, handshake, provider replacement, and ConfigNode procedure paths found and fixed these issues:

  1. [P1] Recovery could reinitialize a live new-region queue. A queue created by the new-DataRegion callback is initialized from the beginning of that region, but the later full-topic recovery path could run the tail-proposal initialization again. That could make the queue pending or change its startup frontier. Recovery now skips live region bindings and only repairs missing regions (SubscriptionConsumerAgent.java:286, ConsensusSubscriptionSetupHandler.java:630).

  2. [P1] A later setup failure could remove healthy queues and rewind concurrent commits. The original rollback unbound every attempted topic and restored the complete setup snapshot, including regions that were already active before recovery. It now records existing region bindings, removes only queues added by the failed setup, and preserves existing queue/state objects, current commit progress, initialization markers, and WAL retention (ConsensusSubscriptionSetupHandler.java:425, ConsensusSubscriptionCommitManager.java:283). Metadata reconciliation also performs this setup once instead of repeating it for newly subscribed topics.

  3. [P1] The new handshake test did not compile. SubscriptionReceiverV1Test#testHandshakeRepairsMissingBrokerWithoutChangingConsumerMetadata called a request builder that throws IOException without handling or declaring it. The test now declares the checked exception (SubscriptionReceiverV1Test.java:225).

  4. [P2] Two new pipe-queue tests lacked required topic metadata. The provider-replacement and canceled-topic fixtures used subscribed topics without registering topic metadata, so both failed before their queue assertions. The fixtures now register and clean up real topic metadata (SubscriptionConsumerAgentBrokerRecoveryTest.java:177, line 207).

The fixes are pushed in commit 0d2ef238dd6.

Validation performed locally on JDK 17 with bounded heap, -DforkCount=0, and clean source dependency rebuilds with the build cache disabled:

  • mvn spotless:apply -P with-integration-tests -pl iotdb-core/datanode: passed.
  • English focused reactor run: 44 tests passed, 0 failures/errors/skips.
  • with-zh-locale focused reactor run: 44 tests passed, 0 failures/errors/skips.
  • The run included SubscriptionConsumerAgentBrokerRecoveryTest, all SubscriptionReceiverV1Test methods, SubscriptionConsumerAgentTest, ConsensusSubscriptionSetupHandlerTest, and the two setup-rollback cases in ConsensusSubscriptionCommitStateTest.
  • An isolated comparison reproduced the queue-loss regression with the original setup handler (one expected failure) and passed with the fixed handler.

The remote attempt did not reach test execution; the reported results are from the local fallback. Integration and full-distribution tests were not run.

@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 7.65027% with 169 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.78%. Comparing base (495f04f) to head (10ad106).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...r/consensus/ConsensusSubscriptionSetupHandler.java 0.00% 80 Missing ⚠️
...bscription/broker/ConsensusSubscriptionBroker.java 0.00% 31 Missing ⚠️
.../subscription/agent/SubscriptionConsumerAgent.java 0.00% 30 Missing ⚠️
...db/subscription/agent/SubscriptionBrokerAgent.java 0.00% 14 Missing ⚠️
.../consensus/ConsensusSubscriptionCommitManager.java 0.00% 13 Missing ⚠️
.../subscription/receiver/SubscriptionReceiverV1.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19021      +/-   ##
============================================
- Coverage     47.78%   47.78%   -0.01%     
  Complexity      604      604              
============================================
  Files          5559     5559              
  Lines        407605   407714     +109     
  Branches      53583    53602      +19     
============================================
+ Hits         194783   194812      +29     
- Misses       212822   212902      +80     

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

@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Independent re-review of the complete diff and the related broker, consumer metadata, handshake, provider replacement, and ConfigNode procedure paths found a remaining P1 concurrency gap between broker repair and new-region callbacks.

At the previous head, the pre-setup queue snapshot could miss a queue created by a concurrent callback. Repair could then apply tail-proposal initialization to that queue, or its rollback could remove the callback's queue and progress after another topic failed. Repair could also bind an already-visible new peer before its callback initialized it from the beginning, risking skipped initial writes.

Fixed and pushed in 50008fb:

Actual verification this round:

  • Local JDK 17: mvn spotless:apply -P with-integration-tests -pl 'iotdb-core/consensus,iotdb-core/datanode' passed.
  • Remote Linux/JDK 17, separate pristine source trees and fresh isolated Maven repositories, build cache disabled, -DforkCount=0: 68 tests passed per locale, 0 failures/errors/skips. The six selected suites were IoTConsensusRecoveryTest, SubscriptionConsumerAgentTest, SubscriptionConsumerAgentBrokerRecoveryTest, SubscriptionReceiverV1Test, ConsensusSubscriptionSetupHandlerTest, and ConsensusSubscriptionCommitStateTest.
  • The English whole-reactor clean test command ran those tests successfully but ended with Maven exit 1 at distribution, which requires server/CLI/ConfigNode ZIPs that the test lifecycle does not produce. It is not a passing whole-reactor build.
  • The Chinese run used clean test -pl '!distribution' -am -P with-zh-locale with the same Surefire selection: BUILD SUCCESS, 49 modules.

No integration tests or distribution packaging were run. This round's newly added tests were executed remotely, not locally.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Comment on lines +227 to +228
SubscriptionAgent.topic()
.getTopicConfigs(java.util.Collections.singleton(topicName), isTableModel);

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.

Mind the import

@jt2594838
jt2594838 merged commit 389e374 into master Oct 27, 2026
42 of 46 checks passed
@jt2594838
jt2594838 deleted the fix/subscription-broker-self-healing branch October 27, 2026 02:36
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