Skip to content

feat(load): reject managed TsFile directories - #18271

Open
luoluoyuyu wants to merge 9 commits into
apache:masterfrom
luoluoyuyu:reject-load-managed-tsfiles
Open

feat(load): reject managed TsFile directories#18271
luoluoyuyu wants to merge 9 commits into
apache:masterfrom
luoluoyuyu:reject-load-managed-tsfiles

Conversation

@luoluoyuyu

Copy link
Copy Markdown
Member

Description

as the title said


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

private static void validateNotLoadingInternalTsFile(final File file)
throws FileNotFoundException {
final Path sourcePath = canonicalPath(file);
for (final String dataDir : IoTDBDescriptor.getInstance().getConfig().getLocalDataDirs()) {

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.

[P2] Hoist managed-directory canonicalization out of the recursive traversal

findAllTsFile calls this method for every file and directory it encounters, and every call re-canonicalizes both sequence and unsequence under every local data directory. Directory loading therefore performs 1 + 2 * dataDirCount canonical-path resolutions per entry, even though the managed roots are invariant during one traversal.

On Windows/JDK 17, traversing 5,000 TsFiles with this PR took about 3.24 s with 1 local data directory, 9.68 s with 4, and 37.31 s with 16, before LOAD analysis or execution began. This makes large directory LOADs increasingly expensive for the supported multi-directory configuration.

Please compute the canonical managed roots once at the start of processTsFile and pass that snapshot through the recursion, or cache them alongside the data-directory configuration.

@luoluoyuyu
luoluoyuyu force-pushed the reject-load-managed-tsfiles branch from 0141c40 to 8baaef0 Compare July 23, 2026 02:24
@luoluoyuyu
luoluoyuyu force-pushed the reject-load-managed-tsfiles branch from 8baaef0 to e777e04 Compare July 23, 2026 02:31
Comment on lines +113 to +115
? org.apache.iotdb.db.queryengine.plan.statement.crud.LoadTsFileStatement
.processTsFile(new File(filePath), validateSourcePath)
: org.apache.iotdb.db.queryengine.plan.statement.crud.LoadTsFileStatement

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mind this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the reminder. processTsFileForPipe intentionally bypasses the managed-directory check because Pipe receiver/hardlink sources can be placed under IoTDB-managed paths. We have restricted this branch to createForPipe() call sites guarded by an existing isGeneratedByPipe context; normal table-model LOAD still uses processTsFile(...) and keeps the managed-directory validation.

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.

3 participants