Skip to content

Fence IoTConsensus applies before storage shutdown - #18841

Merged
jt2594838 merged 4 commits into
masterfrom
fix/pipe-cluster-replica-restart-barrier
Oct 15, 2026
Merged

jt2594838 merged 4 commits into
masterfrom
fix/pipe-cluster-replica-restart-barrier

Conversation

@Caideyipi

@Caideyipi Caideyipi commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Description

An IoTConsensus SyncLog request can pass the read-only check, remain queued or pause before apply, and then write after StorageEngine and WALManager have stopped. In the supplied testPipeAfterDataRegionLeaderStop logs, the sender replica used by p2 has already lost t200: its TsFile starts at 201, while the other replica's starts at 200, and the compaction totals differ by the row's eight fields.

The late successful write recreates a WAL node in its existing directory. The new CheckpointManager appends another header to _0.checkpoint where recovery expects records, leaving the late memtable absent from checkpoint recovery. Actual WAL/TsFile recovery then produces an empty TsFile even when the late WAL was fully drained and normally closed. A late write before WAL shutdown does not recreate the node and remains recoverable, explaining why a flush-only reproducer did not demonstrate the loss.

Fence complete IoTConsensus applies against shutdown with a per-DataRegion lifecycle lock. Each apply attempt holds the shared side through the data mutation and WAL separator, checks read-only/stopping/closed admission under that lock, and releases it before any retry delay. Shutdown takes the exclusive side to drain admitted applies and permanently close admission before the final WAL drain and TsFile close. StorageEngine stop and forced shutdown also close admission. This covers both local writes and remote applies; a request that passed an earlier check is either included in the final flush or rejected without reopening WAL/TsFile processors.

The lifecycle lock is separate from the insertion lock so memory backpressure can still be relieved by ordinary flushing and snapshot closing does not introduce an inverse lock order. The consensus-layer check remains an early rejection for queued batches. Existing retry handling rejects the remaining batch and does not advance the applied frontier. The generic DataRegionStateMachine apply path retains its existing behavior for other consensus protocols.

The cluster tests check every replica's outgoing replication lag before flushing or restarting. The table-model test also checks sender data after restart, before creating p2, to distinguish source-side loss from pipe transfer failure.

Regression coverage

  • Pause SyncLog after its outer read-only check, complete storage/WAL shutdown, then resume: reject the batch without advancing progress or recreating storage. This test fails on the previous PR head with success (200) instead of read-only (600).
  • Pause an apply after lifecycle admission, switch to read-only, and start shutdown: shutdown waits for the apply and both rows are present in the sealed TsFile. Closed admission remains effective if the node status later changes.
  • Stop during a state-machine retry delay: shutdown completes while the retry waits, and the next attempt is rejected before visiting storage.
  • Verify a normal read-only-to-running transition still permits writes before shutdown.
  • Retain the queued-request regression and both actual WAL/TsFile recovery controls. The controls deliberately invoke the real unguarded visitor to model the legacy apply path.

Validation

  • Targeted UTs: 7 shutdown/recovery tests, 8 DataRegionStateMachine tests, and the queued IoTConsensus test passed (16 total).
  • Chinese locale: full-reactor mvn -nsu test-compile -P with-zh-locale -DskipTests passed.
  • English locale and fresh node templates: reactor mvn -nsu package -pl integration-test -am -P with-integration-tests -DskipTests passed (39 modules).
  • Original table-model IoTDBPipeClusterIT#testPipeAfterDataRegionLeaderStop passed in 138.5 seconds with MultiClusterIT2DualTableManualEnhanced and HighPerformanceMode (two data replicas). Logs confirm the post-restart sender check, p2 creation/history extraction, and normal completion through the final receiver assertions.
  • Spotless applied to changed Java modules; git diff --check passed.

Normal-write performance

Compared with the pre-fix baseline (9b29f1cf009), the fixed version (d878e7be265) was measured on 192.168.130.19 with JDK 17.0.9 using JMH. The benchmark kept the node in the normal Running state and used ASYNC WAL; ReadOnly and shutdown paths were excluded.

The lifecycle admission path adds about 14.6 ns per apply in a single-thread microbenchmark. Under artificial contention on one DataRegion, the added cost was about 638 ns/op with four threads and 1.96 us/op with eight threads. In the real DataRegion state-machine write benchmark, single-thread throughput changed by +1.7% for single-row writes and -1.4% for batches of 100 rows, both within normal benchmark variance. Four threads directly competing on one DataRegion showed roughly 2%-4% variation, while distributing eight threads across eight DataRegions showed no consistent regression. The direct same-DataRegion contention test is an upper-bound stress case because normal IoTConsensusServerImpl.write() already serializes state-machine access.

The expected normal-write cost is therefore a tens-of-nanoseconds admission check, generally below 1% for single-row writes and lower for larger batches.

@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.98%. Comparing base (17b95c8) to head (d878e7b).
⚠️ Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
.../apache/iotdb/db/service/DataNodeShutdownHook.java 0.00% 1 Missing ⚠️
...g/apache/iotdb/db/storageengine/StorageEngine.java 88.88% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18841      +/-   ##
============================================
+ Coverage     45.91%   45.98%   +0.07%     
  Complexity      712      712              
============================================
  Files          5502     5502              
  Lines        398778   399176     +398     
  Branches      51968    52047      +79     
============================================
+ Hits         183094   183575     +481     
+ Misses       215684   215601      -83     

☔ 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 Caideyipi changed the title Fix queued IoTConsensus writes during shutdown Fence IoTConsensus applies before storage shutdown Oct 14, 2026
@jt2594838
jt2594838 merged commit 532d6c1 into master Oct 15, 2026
39 of 40 checks passed
@jt2594838
jt2594838 deleted the fix/pipe-cluster-replica-restart-barrier branch October 15, 2026 08:13
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