Skip to content

fix(load): validate TsFile before async move - #18531

Open
luoluoyuyu wants to merge 1 commit into
apache:masterfrom
luoluoyuyu:codex/fix-async-load-non-tsfile
Open

fix(load): validate TsFile before async move#18531
luoluoyuyu wants to merge 1 commit into
apache:masterfrom
luoluoyuyu:codex/fix-async-load-non-tsfile

Conversation

@luoluoyuyu

Copy link
Copy Markdown
Member

Description


This PR has:

  • been self-reviewed.
    • concurrent read
    • concurrent write
    • concurrent read and write
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods.
  • added or updated version, license, or notice information
  • added comments explaining the "why" and the intent of the code wherever would not be obvious
    for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold
    for code coverage.
  • added integration tests.
  • been tested in a test IoTDB cluster.

Key changed/added classes (or packages if there are too many classes) in this PR

@Caideyipi Caideyipi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The async move path still has two correctness gaps. I am requesting changes because both can make the source and active-load state diverge.

Please see the two inline findings below.

}

// Validate before moving the source so ordinary or malformed files remain in place.
if (!isValidTsFile(file)) {

@Caideyipi Caideyipi Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Validate the complete batch before moving any file

loadTsFileAsyncToActiveDir calls this method once per *.tsfile. If a directory contains a valid *.tsfile followed by a malformed *.tsfile, the valid file has already been copied/linked into the active-load directory before this call returns false for the malformed one. doAsyncLoad then falls back to normal analysis, but the first file remains queued; with on-success='delete', its source may already be deleted as well. This can cause partial ingestion and unsafe retries. Validate all files before starting the transfer, or roll back every transfer when a later validation fails.

return true;
}

private static boolean isValidTsFile(final File file) {

@Caideyipi Caideyipi Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Apply this validation to the Pipe async path too

IoTDBDataNodeReceiver.loadTsFileAsync calls LoadUtil.loadFilesToActiveDir, but isValidTsFile is only invoked from loadTsFilesToActiveDir. The Pipe seal path therefore still transfers and deletes the main TsFile even when its magic is invalid, then returns SUCCESS_STATUS; the active loader fails later, after the sender has already been acknowledged. Identify the main .tsfile entry and apply the same validation in loadFilesToActiveDir before transferFilesToActiveDir (while preserving valid .resource/.mods sidecars), and add coverage for this path.

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