[I] [Bug] Format table reads and deletes committer staging files inside partitions [paimon]

["sundapeng (via GitHub)" <gi...@apache.org>]

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: issues-unsubscribe@paimon.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug] Format table reads and deletes committer staging files inside partitions [paimon]

["sundapeng (via GitHub)" <gi...@apache.org>]

sundapeng commented on issue #8860:
URL: https://github.com/apache/paimon/issues/8860#issuecomment-5089604768

   Closing this: the two PRs describe the problem directly, so the issue only adds indirection.
   
   - #8861 — the recursive listing inside a partition (read, and `INSERT OVERWRITE` delete)
   - #8862 — `clean()` deleting the shared `_temporary` directory


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug] Format table reads and deletes committer staging files inside partitions [paimon]

["sundapeng (via GitHub)" <gi...@apache.org>]

sundapeng closed issue #8860: [Bug] Format table reads and deletes committer staging files inside partitions
URL: https://github.com/apache/paimon/issues/8860


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org