zhuxiangyi opened a new pull request, #9662:
URL: https://github.com/apache/paimon/pull/9662
### Purpose
A compaction task writes its output files before its `CompactResult` reaches
the writer, and that
result is the only thing that knows their names. Two paths throw it away and
leave the files
behind:
- **The task is cancelled.** `CompactFutureManager#cancelCompaction`
interrupts the thread and the
future drops the result, so nothing deletes what was already written. This
is the case the TODO
in that method described:
```java
// TODO this method may leave behind orphan files if compaction is
actually finished
// but some CPU work still needs to be done
```
- **A part of a merge tree compaction fails.** `MergeTreeCompactTask` calls
the rewriter more than
once per task: a large, non overlapping file sitting between two groups of
overlapping sections
flushes the accumulated candidates and is then upgraded on its own. The
groups that already
finished have closed their writers, so their files can only be reached
through the result of the
whole task, which is never returned now.
`RollingFileWriterImpl` already aborts the file it is writing when the write
itself fails, so what
leaks is the output of *earlier completed* rewrite calls, not the in flight
one. Those files stay
in the bucket directory, referenced by nothing, until an orphan file clean
runs.
`CompactResult` now carries the `FileWriterAbortExecutor`s of the files it
describes, and
`CompactTask` takes them over as each part of it finishes. A task deletes
them itself when it fails
or when it notices it was cancelled; `cancelCompaction` deletes them for a
task that never gets far
enough to observe the interrupt.
Two related leaks are out of scope here: the remote lookup files created by
`LookupMergeTreeCompactRewriter#notifyRewriteCompactAfter`, and the unaware
bucket dedicated
compaction job, which has its own lifecycle and never goes through
`CompactFutureManager`.
Internal signature changes: `RollingFileWriter` gains
`drainAbortExecutors()`, and
`BucketedAppendCompactManager.CompactRewriter#rewrite` and
`BaseAppendFileStoreWrite#compactRewrite`
return `CompactResult` instead of `List<DataFileMeta>`. No public API or
format change.
### Tests
`CompactOrphanFileTest` reproduces the leak end to end on a real merge tree,
with real files in a
real bucket directory. Five level 0 files are laid out so that a single
`MergeTreeCompactTask` makes
two separate rewrite calls:
| level 0 file | key range | role |
|---|---|---|
| 1 | `[1, 10]` | small, overlaps the next one |
| 2 | `[5, 15]` | small, closes the first section (2 sorted runs) |
| 3 | `[100, 700]` | large, its own section, flushes the accumulated
candidates and is then upgraded |
| 4 | `[2000, 2010]` | small, overlaps the next one |
| 5 | `[2005, 2015]` | small, closes the last section (2 sorted runs) |
The second rewrite call is then broken, once by cancelling the compaction
and once by throwing, and
the test asserts that the bucket directory holds nothing beyond the five
input files.
**Before this change** (the same test run against the parent commit):
```
CompactOrphanFileTest.testCancelledCompactionLeavesNoOrphanFile <<< FAILURE!
Expecting empty but was:
["data-853a7215-e4de-492c-907c-3600d783fc99-6.parquet"]
CompactOrphanFileTest.testFailedCompactionLeavesNoOrphanFile <<< FAILURE!
Expecting empty but was:
["data-8a2661d2-a0b1-4f00-a313-01b3d64da1e4-6.parquet"]
Tests run: 3, Failures: 2, Errors: 0, Skipped: 0
```
Each leaked file is the real output of the first rewrite call, left in the
bucket directory with
nothing referencing it.
**After this change:**
```
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
```
The third case is the control: a compaction that runs to the end must keep
its files, so the test
does not pass simply by deleting everything.
`CompactTaskCancelTest` covers the `CompactTask` bookkeeping directly,
including that a task which
succeeds keeps its files.
Existing suites: 291 tests across the compaction, append, merge tree, lookup
and clustering
packages of `paimon-core` pass, and `paimon-flink-common`,
`paimon-flink-cdc` and
`paimon-spark-common` still compile.
--
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]
