[Caideyipi]

Description

A Region migration can snapshot LOAD staging files while a single LOAD is still applying its pieces. Keep the snapshot's staged bytes and progress logs independent from the writable source files, and keep restored tasks independent from the snapshot. Later piece appends must not change a completed snapshot, and repairing a copied progress log must not truncate the live log through a shared hard link.

Repair incomplete progress-file headers and reinitialize empty progress files on the next append. Bound tail scans by the captured file length so truncated entries larger than the read buffer are detected as well.

Also copy staged files created between the data-file and progress-log directory scans. Previously, catch-up skipped a missing snapshot data file and could leave a progress log without the data it describes.

Stabilize the LOAD cluster integration tests by isolating the second LOAD input directory, waiting until every DataNode has materialized the first LOAD's local TsFiles before stopping a node, and measuring replica sizes from local TsFile metadata instead of lagging show regions TSFILE_SIZE results. Use the standard session credentials for the JDBC checks.

Add deterministic regression coverage for header and entry truncation, large entries, files appearing between directory scans, snapshot isolation from later pieces, and repairs that preserve the source log. Coordinate the concurrent snapshot test with futures so writer errors propagate and file growth is observed independently of thread scheduling.

Validation

• mvn spotless:apply -P with-integration-tests passed.
• A clean JDK 17 DataNode dependency reactor with an initially empty isolated Maven repository and build-output caches disabled rebuilt all 28 modules from source; all 77 selected LOAD tests passed across LoadTsFileProgressTest, LoadTsFileSnapshotTest, LoadTsFileManagerTest, LoadTsFileResumeTest, LoadTsFileCleanerTest, and LoadTsFileManagerSplitCoverageTest.
• The three new isolation regressions each failed with the preceding hard-link implementation and passed with independent copies, using the dependencies and test classes from that clean build.
• Final-head IoTDBLoadTsFileClusterIT#testRestartedDataNodeIsSyncedWithTheLoadedFile passed with tests=1, failures=0, errors=0.
• Final-head IoTDBLoadTsFileClusterIT#testLoadAfterOneDataNodeDown passed with tests=1, failures=0, errors=0.
• The successful builds and tests ran on 192.168.130.6. Earlier Windows and remote attempts that stopped before completing verification are not counted as passing checks.

[Caideyipi]

Initial self-review found and fixed one actionable issue in d5766d7.

publishAtomicCopy preferred hard links for writable staging files and progress logs. A snapshot's torn-header or torn-entry repair could therefore truncate the live source log, and later LOAD pieces could change an already completed snapshot. Snapshot/restore publication now uses independent copies. Three deterministic regressions cover both source-log preservation cases and snapshot stability after a subsequent piece.

The concurrency is between the Region migration snapshot and LOAD application: the snapshot path does not acquire the consensus write lock, even when the pieces of one LOAD are applied sequentially.

Verification: local Spotless passed. A clean JDK 17 dependency-reactor build on 192.168.130.6, with a fresh Maven repository and build-output caching disabled, rebuilt all 28 modules and passed all 77 selected LOAD tests. All three new isolation regressions failed with the preceding hard-link implementation and passed with the fix. The local Windows reactor attempt was blocked by its commit limit, and an earlier remote attempt was blocked by dependency-repository connectivity; neither is reported as passing.

[Caideyipi]

Re-review of PR #19031 at head 2577a56 found one actionable recovery bug and fixed it in that commit.

The defect predates this PR's initial draft. The original LOAD snapshot implementation in #18698 (LoadTsFileSnapshot.java:143) copied staged files and progress logs in separate directory scans (the progress scan starts at line 152). A staged TsFile could be copied in the first scan while its progress log was still absent in the second. On restore, TsFileWriterManager.java:709 reconstructs writers only from progress files, so replay encounters the unrecorded staged file instead of resuming it. This is separate from the later hard-link isolation fix.

Snapshot finalization now drops staged TsFile copies with no complete progress entry and removes their sidecars while preserving the source. The new restore/replay regression is in LoadTsFileSnapshotTest.java:457.

Verification

• Local Spotless and git show --check passed.
• The previously reported clean remote build passed 77 selected tests.
• This round's focused remote run did not start tests: Maven Central reset the connection while resolving org.apache:apache:pom:31. No tests from that run passed or failed.
• The 10 newly added Progress/Snapshot tests have not run locally.

The PR remains draft. A fresh complete independent review is required at the updated head.

[Caideyipi]

Independent re-review of PR #19031 at head 2577a568196312f83544206125eb691f1eed65a3 found no actionable issues; no fixes were needed.

I inspected the complete five-file diff and the related LOAD snapshot, progress-log recovery, writer reconstruction, restore, and copy-isolation call chains. The new snapshot finalization removes only copied staged TsFiles whose repaired progress log has no complete chunk record, removes their copied sidecars, and leaves the source task files untouched. The earlier independent-copy fix remains effective, and the directory-scan catch-up path now supplies a copied data file when the progress log appears between scans. No regression was found in the interaction among recovery, torn-log repair, snapshot cleanup, or later piece appends.

Verification at the reviewed head:

• git diff --check passed.
• A clean JDK 17 serial dependency reactor on 192.168.130.6 used fresh source and Maven-repository directories with -Ddevelocity.off=true. LoadTsFileProgressTest ran 10 tests with 0 failures/errors, and LoadTsFileSnapshotTest ran 14 tests with 0 failures/errors; all 24 passed, including the 10 newly added tests.
• The targeted ClusterIT reactor ran IoTDBLoadTsFileClusterIT#testRestartedDataNodeIsSyncedWithTheLoadedFile and #testLoadAfterOneDataNodeDown: 2 tests, 0 failures, 0 errors.
• An earlier parallel reactor attempt stopped before tests because the Thrift generator reported Text file busy; it is not counted as a passing or failing test result.

The PR was still draft during this review, and the remote head matched the reviewed revision before this comment.

[Caideyipi]

Self-review follow-up for PR #19031 at head 0927eac90ad68ca78248d2c8d8b93e959f5e855b found one actionable test-logic issue in the concurrent snapshot regression and fixed it in that commit.

The previous test only observed snapshot growth across independently completed writes; it did not force a snapshot to overlap bytes written before the corresponding progress record. The test now pauses setChunkPayloadRefs after the staged bytes are written, takes the snapshot in that window, restores it, and replays the paused chunk. It asserts that the restored progress log advances from one record to two and keeps writer, latch, and configuration cleanup bounded.

Production review of the final five-file PR diff and the related LOAD snapshot, progress-log recovery, writer reconstruction, restore, and copy-isolation paths found no additional actionable issues.

Verification at the final local commit:

• mvn spotless:apply -P with-integration-tests -pl iotdb-core/datanode passed.
• A clean offline JDK 17 dependency reactor with a dedicated Maven repository and build-output caches disabled rebuilt all 28 modules from source. LoadTsFileSnapshotTest#testSnapshotTakenWhilePiecesAreAppliedStaysRestorable ran with 1 test, 0 failures, and 0 errors.
• git diff --check passed.
• The remote head was verified as 0927eac90ad68ca78248d2c8d8b93e959f5e855b; the PR remains open and draft.

The PR remains draft. A fresh independent review is required at the updated head.

[Caideyipi]

Independent re-review of the test changes in commit 0927eac90ad68ca78248d2c8d8b93e959f5e855b found no actionable issues; no fixes were needed. The latch pauses after the staged bytes are written and before their progress record is flushed, so the test deterministically snapshots the intended in-flight state, restores it, and replays the paused piece. Cleanup releases the writer and restores the modified configuration. git show --check passed. Tests were not rerun in this review round; the focused test result for this head is recorded in the preceding self-review comment.

[luoluoyuyu]

Read the diff against TsFileWriterManager (fsync before records) and recoverFromDisk (truncate to resumeOffset), so the core fix looks sound. Issues introduced by this change:

1. Misleading WARN on empty progress logs. findTornTailOffset() returns 0 for any file shorter than 4 bytes, and readAllRecordsRepairingTornTail() warns "dropped the trailing entry ... from offset 0" whenever the offset is >= 0. A normal snapshot that copies a staged file before its log gets its header, and recovery of a crashed task, now both report dropping an entry that never existed. Only warn when the offset is > 0.

2. One unreadable progress log now fails the whole region snapshot. discardUnrecordedStagedFiles() lets the IOException from an invalid magic escape, so snapshot() returns false and the migration keeps retrying against a log the node cannot replay anyway. recordedLengthOf() previously swallowed it. Is failing the migration the intended blast radius, or should the unreadable task be left out of the snapshot?

3. The "log ⊆ data" invariant is only checked in tests. copyPrefixBytes() breaks silently when transferTo() returns 0, and catchUpStagedFiles() gives up after MAX_STAGED_FILE_REPAIRS without signaling anything, so snapshot() can return true with a log that describes bytes the copied file does not hold. Verifying max physicalEnd <= copied file length at the end of copyTask and returning false would make that fail loudly instead.

4. An empty .progress file is left behind when a staged file is discarded, which is why assertEveryCopiedLogFitsItsCopiedFile() had to be relaxed to allow a log without data. Deleting the log too keeps the invariant strict. The deletion is also silent, which makes "why did this task replay from scratch after migration" hard to debug.

5. Behavior change outside the LOAD snapshot path. readAllRecords() now returns empty for a 0-byte file, so LoadTsFileCleaner.lastRecordedSearchIndex() no longer hits the "read failed, leave it alone" branch and proceeds to delete/dropTask. A 0-byte log currently only exists inside the window in ensureActivated(); please confirm the cleaner is expected to release such a directory.

6. Cost: hard link -> byte copy doubles staging disk usage during migration and makes snapshot time linear in pending LOAD size. Hard-linking files that can no longer be appended (terminal record present / resource NORMAL) and copying only the live ones may be worth it.

Minor test notes: blocking inside setChunkPayloadRefs() keeps the writer thread waiting up to 30s under the manager lock; config.setLoadTsFileWriteBufferSize(1024) mutates the global IoTDBConfig; and nothing asserts that replaying after restore() leaves the snapshot bytes unchanged, which is the other half of the fix.