sundapeng opened a new issue, #8860:
URL: https://github.com/apache/paimon/issues/8860

   ### Search before asking
   
   - [X] I searched in the [issues](https://github.com/apache/paimon/issues) 
and found nothing similar.
   
   ### Paimon version
   
   master (`45cf5ffb0`); every version that has Format Table support is 
affected.
   
   ### Compute Engine
   
   Any engine reading a Format Table through `paimon-core` (reported on Trino 
422 over OSS with the
   JindoOSS magic committer; Spark and Flink read through the same code path).
   
   ### Minimal reproduce step
   
   1. Create a partitioned Format Table (parquet) and write a partition 
normally.
   2. Leave a committer staging tree inside one partition — exactly what an 
S3A/JindoOSS *magic*
      committer or a `FileOutputCommitter` leaves behind while a job runs, or 
after it finishes with
      cleanup disabled:
   
   ```
   
<table>/p1=v004999/__magic-<uuid>/tasks/attempt_<id>/__base/part-00010-xxx.snappy.parquet
   (0 bytes)
   
<table>/p1=v004999/__magic-<uuid>/tasks/attempt_<id>/__base/part-00010-xxx.snappy.parquet.pending
   ```
   
   3. `SELECT count(*) FROM <table> WHERE p1 = 'v004999'`.
   
   ```
   Query ... failed: 
...__magic<uuid>/tasks/attempt_.../__base/part-00010-xxx.snappy.parquet
   is not a Parquet file (length is too low: 0)
   ```
   
   The whole query fails — not just that partition.
   
   ### What doesn't meet your expectations?
   
   Files under a hidden directory are not table data and must not be read, and 
must not be deleted by
   someone else's `INSERT OVERWRITE`. Two places treat them as data, for the 
same reason: a
   **recursive** listing is filtered by the **leaf file name only**.
   
   **1. Read** — `SplitEnumerator#createSplits`
   
(`paimon-core/src/main/java/org/apache/paimon/table/format/SplitEnumerator.java:112-118`):
   
   ```java
   FileStatus[] files = fileIO.listFiles(path, true);          // recursive
   for (FileStatus file : files) {
       if (FormatTableScan.isDataFileName(file.getPath().getName())) {   // 
leaf name only
   ```
   
   `FormatTableScan#isDataFileName`
   
(`paimon-core/src/main/java/org/apache/paimon/table/format/FormatTableScan.java:101-103`)
 is
   `!name.startsWith(".") && !name.startsWith("_")`. 
`part-00010-xxx.snappy.parquet` is a perfectly
   ordinary name — it is only the `__magic.../__base/` directories **above** it 
that say the file is
   uncommitted. So the 0-byte placeholder is opened as parquet and the query 
dies.
   
   **2. Write** — `FormatTableCommit#deletePreviousDataFile`
   
(`paimon-core/src/main/java/org/apache/paimon/table/format/FormatTableCommit.java:270-284`)
 does the
   same `listFiles(partitionPath, true)` + `isDataFileName(...)` and then 
deletes. An
   `INSERT OVERWRITE` therefore deletes the pending files of any writer that is 
still running in that
   partition — data loss for that job, not just a failed read.
   
   **3. Cleanup** — `RenamingTwoPhaseOutputStream.TempFileCommitter#clean`
   
(`paimon-common/src/main/java/org/apache/paimon/fs/RenamingTwoPhaseOutputStream.java:138-140`)
   deletes `tempPath.getParent()`, i.e. the whole `<partition>/_temporary` 
directory, after committing
   one file. `_temporary` is shared: two concurrent Paimon writers into the 
same partition delete each
   other's pending files, and any Hadoop `FileOutputCommitter` job staging 
there loses its output.
   
   **Why nothing else hits this.** Every other engine filters a *recursive* 
listing per path component.
   Hive says it explicitly in `HIDDEN_FILES_FULL_PATH_FILTER`
   
(`standalone-metastore/metastore-common/.../metastore/utils/FileUtils.java`), 
which it applies to the
   path made relative to the listing root in the S3A fast path of 
`listStatusRecursively` — the same
   `fs.listFiles(base, true)` API Paimon uses:
   
   > This works with RemoteIterator which (potentially) produces all files 
recursively so looking for
   > hidden folders must look at whole path, **not just the last part of it as 
would be appropriate
   > w/o recursive listing**.
   
   Hadoop MapReduce, Trino (`containsHiddenPathPartAfterIndex(path, 
rootPrefixLength)`) and Spark do the
   same. #6522 fixed *partition discovery* in Paimon 
(`PartitionPathUtils.searchPartSpecAndPaths`
   checks `isHiddenFile` at every level); the listing **inside** a partition 
never followed.
   
   ### Anything else?
   
   Two facts that make this more than a corner case:
   
   - **Leftover staging trees are a blessed steady state, not an anomaly.**
     `fs.s3a.committer.magic.cleanup.enabled=false` (HADOOP-18568, Hadoop 
3.4.2+) is the documented
     recommendation for large jobs, with the `__magic` tree removed by an 
object-store lifecycle rule
     instead; `MagicS3GuardCommitter.cleanupStagingDirs()` also swallows 
deletion failures. "It
     disappears when the job finishes" does not hold.
   - **Do not match on the literal `__magic`.** Since HADOOP-18797 (Hadoop 
3.4.0) the directory is
     named `__magic_job-${jobId}`. The generic `_` / `.` prefix rule is the one 
to implement; it also
     covers `_temporary`, `.hive-staging_*`, `_temporary_jindo`, and matches 
JindoOSS's own default
     (`fs.jfs.cache.oss.delete-marker.dirs` = 
`temporary,.staging,.hive-staging,__magic`).
   
   One correctness note for whoever fixes it: the hidden-component check **must 
be relative to the
   listed directory**. Walking up to the filesystem root would make every table 
under a warehouse path
   like `oss://bucket/_warehouse/db/t` read as empty — silently reading nothing 
is worse than the bug
   being fixed.
   
   Verified on a cluster (EMR Trino 422, OSS + JindoOSS): injecting a 0-byte
   `__magic-.../tasks/attempt_.../__base/part-00010-...snappy.parquet` into one 
partition of an existing
   table reproduces the error above verbatim; with the staging tree filtered 
out the same query returns
   its normal result and partition pruning is unchanged.
   
   ### Are you willing to submit a PR?
   
   - [X] I'm willing to submit a PR!
   


-- 
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]

Reply via email to