Skip to content

feat(load): reject managed TsFile directories#18271

Open
luoluoyuyu wants to merge 1 commit into
apache:masterfrom
luoluoyuyu:reject-load-managed-tsfiles
Open

feat(load): reject managed TsFile directories#18271
luoluoyuyu wants to merge 1 commit 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 +204 to +214
private static Path[] getInternalTsFileDirCanonicalPaths() throws FileNotFoundException {
final String[] localDataDirs = IoTDBDescriptor.getInstance().getConfig().getLocalDataDirs();
final Path[] internalTsFileDirCanonicalPaths = new Path[localDataDirs.length * 2];
for (int i = 0; i < localDataDirs.length; i++) {
internalTsFileDirCanonicalPaths[i * 2] =
canonicalPath(new File(localDataDirs[i], SEQUENCE_FOLDER_NAME));
internalTsFileDirCanonicalPaths[i * 2 + 1] =
canonicalPath(new File(localDataDirs[i], UNSEQUENCE_FOLDER_NAME));
}
return internalTsFileDirCanonicalPaths;
}

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.

This does not need to be initialized for each load.
You may cache it in IoTDBConfig.

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.

Pipe hardlink dir, region snapshot dir, etc., should also be excluded.

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