sundapeng opened a new pull request, #8861:
URL: https://github.com/apache/paimon/pull/8861
### Purpose
Closes #8860.
A Format Table lists the files of a partition **recursively**
(`fileIO.listFiles(path, true)`) but decides what is data from the **leaf
file name only**
(`FormatTableScan#isDataFileName`: `!name.startsWith(".") &&
!name.startsWith("_")`). Files staged by
a committer inside the partition —
`__magic_job-<id>/tasks/attempt_*/__base/part-*.parquet`,
`_temporary/0/_temporary/attempt_*/part-*`, `.hive-staging_*/-ext-10000/*` —
carry perfectly ordinary
data file names; only the directories above them say they are uncommitted.
So they are picked up as
data, and a 0-byte magic-committer placeholder fails the whole query with
`... is not a Parquet file (length is too low: 0)`.
The same listing feeds `FormatTableCommit#deletePreviousDataFile`, so
`INSERT OVERWRITE` deletes the
pending files of writers that are still running in that partition.
Every other engine filters a recursive listing per path component — Hive's
`HIDDEN_FILES_FULL_PATH_FILTER` (applied to the path made relative to the
listing root, in the same
`fs.listFiles(base, true)` fast path Paimon uses), Hadoop MapReduce, Trino
(`containsHiddenPathPartAfterIndex`), Spark. #6522 fixed this for partition
*discovery*
(`searchPartSpecAndPaths` checks `isHiddenFile` at every level); the listing
inside a partition never
followed.
**This PR**
1. `PartitionPathUtils#containsHiddenPathPart(base, path)`: is any path
component **below `base`**
hidden (`_` or `.` prefixed)? Used by the read path
(`SplitEnumerator#createSplits`) and the write
path (`FormatTableCommit#deletePreviousDataFile`) through one shared
`FormatTableScan#isDataFile(listedRoot, file)`.
Two deliberate choices:
- **Relative to the listed directory, never absolute.** Walking up to the
filesystem root would
make every table under a warehouse path such as
`oss://bucket/_warehouse/db/t` read as empty.
Silently reading nothing is worse than the bug being fixed, so there is
a test for it.
- **The generic `_` / `.` rule, not a `__magic` literal.** Since
HADOOP-18797 (Hadoop 3.4.0) the
directory is `__magic_job-${jobId}`; the prefix rule also covers
`_temporary`,
`.hive-staging_*` and `_temporary_jindo`, and matches JindoOSS's own
default
(`fs.jfs.cache.oss.delete-marker.dirs`).
2. `RenamingTwoPhaseOutputStream.TempFileCommitter#clean` deleted
`tempPath.getParent()` — the whole
`<partition>/_temporary` directory — after committing a single file. That
directory is shared
with every other writer of the same directory: two concurrent Paimon
writers into one partition
wiped each other's pending files, and any Hadoop `FileOutputCommitter`
job staging there lost its
output. It now deletes only its own temporary file (a no-op after a
successful rename), leaving
at most an empty `_temporary` directory that readers already skip.
Worth knowing for reviewers: leftover staging trees are a supported steady
state, not an anomaly.
`fs.s3a.committer.magic.cleanup.enabled=false` (HADOOP-18568) is the
documented recommendation for
large jobs, with the `__magic` tree removed by an object-store lifecycle
rule, and
`MagicS3GuardCommitter.cleanupStagingDirs()` swallows deletion failures
anyway.
Not in this PR: skipping 0-byte files. It is a different question (harmless
for text formats, always
fatal for parquet/orc) and deserves its own discussion rather than riding
along with a path filter.
### Tests
`PartitionPathUtilsTest`
- `testCommitterStagingTreesAreHidden`: `_temporary/...`, `__magic/...`,
`__magic_job-<id>/...`,
`.hive-staging_*/...`, `_temporary_jindo/...`, and a hidden leaf.
- `testCommittedFilesAreKept`: plain and nested-but-visible files.
- `testHiddenComponentsAboveTheListedRootAreIgnored`: a table under
`_warehouse/.db/` still reads.
- `testPathOutsideTheListedRootIsLeftToTheCaller`.
`FormatTableScanTest`
- `testCreateSplitsSkipsCommitterStagingFiles` (unpartitioned) and
`testCreateSplitsSkipsStagingFilesInsidePartitions` (partitioned, both
partition layouts): only
the committed file becomes a split.
- `testCreateSplitsKeepsFilesUnderAStagingLikeTableLocation`: regression
guard for a table location
with a leading `_`.
`FormatTableCommitTest`
- `testOverwriteKeepsFilesOfConcurrentWritersStagingTrees`: `INSERT
OVERWRITE` removes the previous
data file, commits the new one, and leaves both staging trees untouched.
`RenamingTwoPhaseOutputStreamTest`
- `testCleanKeepsTheSharedStagingDirectory`.
All four new scan/commit tests fail on master without the change. `mvn -pl
paimon-common,paimon-core
test` is green.
### API and Format
No format change. `PartitionPathUtils` gains two public helpers
(`isHiddenName`,
`containsHiddenPathPart`); `FormatTableScan#isDataFileName` keeps its
signature and behaviour.
`RenamingTwoPhaseOutputStream` cleanup now leaves an empty `_temporary`
directory behind, which
readers already ignore.
### Documentation
None — this restores the behaviour the `_`/`.` convention already implies.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
