Repository navigation
Fix subscription commit recovery after RPC timeouts - #19023
Conversation
|
Self-review passed at 72494ee3e40 after reviewing the complete diff and the related commit batching, provider recovery, server ACK handling, and consumer lifecycle paths. All confirmed review findings are fixed. Fixed [P2] Whole-group retries replay completed RPC batches in Validation:
Focused test selection (with the isolated repository and sanitized settings also supplied): ./mvnw -T 2 clean test -pl iotdb-client/subscription -am -P with-integration-tests \
-Dtest=SubscriptionConsumerMultiProviderPollTest,SubscriptionConsumerLifecycleTest,SubscriptionProviderConnectionTimeoutTest,SubscriptionProviderSlicingTest \
-Dsurefire.failIfNoSpecifiedTests=falseThe original head's SonarCloud quality gate was failing; the 50 GitHub annotations inspected were outside this PR's diff and were left unchanged. |
|
Re-review completed after checking the full PR diff and the related commit batching, server ACK handling, provider recovery, asynchronous commit, and consumer lifecycle paths. Fixed one additional issue in 53b3b02ad4b. [P2] Retry completion could roll back a newer committed checkpoint — Added Validation:
Focused test command, with the isolated repository, sanitized settings, and cache disabling also supplied: mvn -T 2 clean test -pl iotdb-client/subscription -am -P with-integration-tests \
-Dtest=SubscriptionConsumerMultiProviderPollTest,SubscriptionConsumerLifecycleTest,SubscriptionProviderConnectionTimeoutTest,SubscriptionProviderSlicingTest \
-Dsurefire.failIfNoSpecifiedTests=false |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19023 +/- ##
============================================
- Coverage 47.83% 47.82% -0.01%
Complexity 604 604
============================================
Files 5562 5562
Lines 408030 408227 +197
Branches 53644 53685 +41
============================================
+ Hits 195172 195245 +73
- Misses 212858 212982 +124 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Re-review fixes are in 4bbeedd2952.
Validation:
|
|
Re-review of the complete diff at
Validation:
|
|




Transient subscription commit connection failures currently escape from
commitSyncafter provider recovery is scheduled. A consumer that stops on that exception can stop making progress even though the provider can recover.Explicit synchronous and asynchronous ACKs now retry only unconfirmed contexts: the initial attempt plus at most three retries, five seconds apart. Connection failures and partial acceptance share that budget. If a response accepts 8 of 10 contexts, those exact 8 leave the pending buffer immediately; only the other 2 are retried. Successful DataNode groups and RPC batches are preserved. Each response's checkpoint is applied once, so completing an older retry cannot reapply its checkpoint over a newer concurrent commit. Waiting happens outside the provider read lock; server errors, close, fencing, and interruption stop retries.
Automatic/close-time commits and redirected ACK flushes also preserve completed-batch results when a later batch loses its connection, removing accepted contexts before their existing subsequent flushes. File/tablet continuation-poll cleanup retains the original recoverable poll failure if its NACK encounters a connection failure. NACKs are sent once because replaying them can increment the server's poison-message counter again; fencing and server errors still propagate.
Validation on a remote JDK 17 host:
commonandtsfileartifacts were rebuilt from source commiteb733133b9eee0b1eb74f7671d809ecd730cedc3; only Maven coordinates were adapted to2.5.1-261023-SNAPSHOT. Installed JAR hashes matched the source build. The third-party dependency download cache was reused.All newly added test methods are in
SubscriptionConsumerMultiProviderPollTest. Localmvn spotless:apply -P with-integration-tests -pl iotdb-client/subscriptionandgit diff --checkpassed; local unit tests and the original long-running workload were not run.