Skip to content

[IoTConsensus] Preserve WAL needed by reconstructed peers - #19268

Open
hongzhi-gao wants to merge 1 commit into
apache:masterfrom
hongzhi-gao:fix/reconstruct-wal-retention
Open

hongzhi-gao wants to merge 1 commit into
apache:masterfrom
hongzhi-gao:fix/reconstruct-wal-retention

Conversation

@hongzhi-gao

Copy link
Copy Markdown
Contributor

Description

Reconstructing a replica in a two-replica IoTConsensus region temporarily leaves one replica. That transition permits WAL reclamation, but registering the replacement channel does not immediately restore its retention boundary. Cleanup can also keep using the file boundary calculated before registration. Incremental WAL needed after the snapshot can therefore disappear. The dispatcher currently skips missing indexes, allowing replication progress to advance across the hole.

For example, the new regression scenario pauses snapshot transmission, acknowledges 12,000 more writes, and runs WAL cleanup. With the original production code at ed9504f3ae, isolating the reconstructed replica returns only the snapshot's 1 row instead of the expected 12,001 rows. With this fix, each isolated replica returns all 12,001 timestamps and values.

Changes

  • Hold the existing write lock while capturing the initial replication index, registering the channel, and publishing WAL retention. Serialize retention calculation/publication so a stale calculation cannot overwrite a newly lowered boundary.
  • Coordinate retention publication with each WAL file's eligibility check and actual deletion. Cache the binary-search result while the boundary remains unchanged, and refresh it whenever the boundary or the cleanup file list changes.
  • Notify the LOAD retention listener only when the boundary advances, matching its contract. Lowering a boundary cannot make additional directories reclaimable; task registration and recovery already evaluate retention explicitly.
  • Fail the replication channel on a missing required WAL index rather than sending later entries across the gap. After activation, wait for the coordinator's outbound channel to acknowledge a fixed local frontier before reporting add-peer success, with failure propagation and a 10-minute timeout.
  • Release the dispatcher monitor before retention callbacks and provide matching English/Chinese exception templates.

No on-disk or RPC format changes are introduced. Validation targets two data replicas; the catch-up wait checks the coordinator's outbound channel.

Validation

  • IoTConsensusServerImplTest: 5 tests passed, including immediate retention publication, catch-up timeout, and failure on a WAL gap. Re-run after the localization change.
  • Focused WALNodeTest: 3 tests passed, including a deterministic cleanup-initialization / boundary-lowering / deletion sequence and listener notifications.
  • LoadTsFileManagerTest: 30 tests passed.
  • IoTDBRegionReconstructWALIT: passed with 1 ConfigNode and 3 DataNodes, data RF=2. Uses the existing debugger fixture to pause only snapshot transmission, exceeds the dispatcher queue capacity, invokes real WAL cleanup, and verifies both replicas separately by stopping the other replica. The old-code regression run failed with 1/12,001 rows.
  • mvn test-compile -o -DskipTests -Ddevelocity.off=true and the same full-reactor command with -Pwith-zh-locale: passed after localization. Compiled Chinese constants and the missing-WAL exception were inspected.
  • Spotless, locale key/placeholder parity, raw-literal checks for the five added failures, and git diff --check: passed.

The WAL/LOAD tests and IT ran before the final message-only localization change. Process-kill crash tests, I/O fault injection, and cross-host performance tests were not run.

Focused test commands
mvn test -o -pl iotdb-core/consensus -am -Dtest=IoTConsensusServerImplTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false -Ddevelocity.off=true

mvn verify -o '-Dtest=WALNodeTest#testRetentionListenerOnlyRunsWhenBoundaryAdvances+testCleanupRechecksLoweredReplicationBoundary+testDeleteOutdatedFiles,LoadTsFileManagerTest' -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false -Dfailsafe.failIfNoSpecifiedTests=false -Dit.test=IoTDBRegionReconstructWALIT -pl integration-test -am -Pwith-integration-tests,ClusterIT -Ddevelocity.off=true

Performance observations

Compared the previous correctness-fix implementation (per-file binary search / unconditional listener notification) with the optimized implementation, not with unfixed upstream. Three alternating repetitions per version used a shared HDD host, colocated 1C2D with RF=2, ASYNC WAL, 8 clients, and 10-row tablets with 3 INT64 measurements per row. Each run used fresh directories, a 10-second warmup, a 30-second steady window, and a 60-second reconstruction window. All six runs checked acknowledged write counts and confirmed reconstruction success before the window ended.

Median Before optimization After optimization
Steady rows/s 339,149 337,837
Steady tablet-write P99 0.397 ms 0.410 ms
Reconstruction-window rows/s 199,884 198,832
Reconstruction-window tablet-write P99 0.392 ms 0.384 ms

Throughput medians changed by -0.39% and -0.53%; steady P99 increased by 3.27%. These measurements do not establish zero regression. Existing approximately 30-second remove-channel stalls occurred in five runs and are outside this change.

A separate synthetic-metadata benchmark of the real eligibility predicate reduced the median 100,000-file scan at a stable finite boundary from 307.45 ms to 0.391 ms; the MAX_VALUE fast branch remained about 1.65 ms versus 1.62 ms. This excludes file deletion and lock contention and is not an end-to-end speedup claim.

This PR has:

  • Been self-reviewed and independently reviewed for lock ordering and lifecycle behavior.
  • Added unit/component regression coverage.
  • Added and executed a targeted cluster integration test.
  • Provided English and Chinese messages.

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