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

   ### Search before asking
   
   - [x] I searched in the [issues](https://github.com/apache/paimon/issues) 
and found nothing similar.
   
   ### Paimon version
   
   master (cd39bd705), also reproduced on 1.4.x
   
   ### Compute Engine
   
   Engine-independent (paimon-core). Observed in production via Spark `CALL 
sys.compact` on a DV-enabled primary-key table, reproduced with a pure 
paimon-core unit test.
   
   ### Minimal reproduce step
   
   Scenario (mirrors the common write-only + standalone compaction deployment):
   
   1. Create a primary-key table with `deletion-vectors.enabled=true`.
   2. Write some rows, run a **full compaction** → data files sit at max level.
   3. Write `-D` records for some keys (they stay in L0).
   4. Run a **minor compaction** → L0 is merged down, deletion vectors are 
generated for the untouched max-level files. `$table_indexes` now shows 
`DELETION_VECTORS` entries. So far so good.
   5. Run a **full compaction** → the max-level files that carry DVs are 
rewritten (deleted rows are physically dropped; row count is correct).
   
   **Expected**: after step 5 the consumed `DELETION_VECTORS` entries are 
removed from the index manifest (the rewritten files no longer exist, so their 
DVs are meaningless).
   
   **Actual**: the DV index entries remain in the index manifest, pointing to 
data files that are no longer referenced by any snapshot — orphan DV entries. 
`$table_indexes` keeps showing them forever.
   
   Reproducing unit test (paimon-core):
   
   ```java
   // 1. write 100 rows, full compact (data at max level)
   // 2. write 10 -D rows (L0)
   // 3. minor compact -> DV generated:
   //      dvRanges=[data-e6fe2b16-....parquet]   (alive, level 5)
   // 4. full compact -> alive file becomes data-59c812ad-....parquet
   //    but index manifest still contains:
   //      dvRanges=[data-e6fe2b16-....parquet]   (dead file) <- ORPHAN
   ```
   
   I can provide the complete runnable test class in a follow-up PR.
   
   ### What doesn't meet your expectations?
   
   Root cause analysis:
   
   When a full compaction finds the bucket contains **only max-level files**, 
`CompactStrategy.pickFullCompaction` picks the files that carry deletion 
vectors and builds a unit with `fileRewrite=true`:
   
   ```java
   // CompactStrategy.pickFullCompaction
   } else if (dvMaintainer != null
           && dvMaintainer.deletionVectorOf(file.fileName()).isPresent()) {
       filesToBeCompacted.add(file);
   }
   ...
   return Optional.of(CompactUnit.fromFiles(maxLevel, filesToBeCompacted, 
true));
   ```
   
   `MergeTreeCompactManager.submitCompaction` then dispatches to 
`FileRewriteCompactTask` instead of `MergeTreeCompactTask`:
   
   ```java
   if (unit.fileRewrite()) {
       task = new FileRewriteCompactTask(rewriter, unit, dropDelete, 
metricsReporter, bucketInfo);
   } else {
       task = new MergeTreeCompactTask(..., compactDfSupplier, ...);
   }
   ```
   
   Inside the task, the rewriter correctly calls `notifyRewriteCompactBefore` → 
`dvMaintainer.removeDeletionVectorOf(file)` for every rewritten file, so the 
maintainer's in-memory state is updated (`modified=true`).
   
   However, **`FileRewriteCompactTask` never receives `compactDfSupplier` and 
never calls `result.setDeletionFile(compactDfSupplier.get())`**, unlike 
`MergeTreeCompactTask` which does:
   
   ```java
   // MergeTreeCompactTask.doCompact
   result.setDeletionFile(compactDfSupplier.get());   // writes the updated 
(possibly empty) DV index
   ```
   
   As a result the updated DV index file is never written / attached to the 
`CompactResult`, the commit contains no index-manifest change for this bucket, 
and `IndexManifestFileHandler.BucketedCombiner` keeps the previous (now stale) 
DV entry because there is no new entry to replace it.
   
   This was introduced by #5751 ("Simplify force rewrite files in Compact 
Task") which extracted `FileRewriteCompactTask` from the old code path but 
dropped the `compactDfSupplier` handling.
   
   Impact:
   
   - Correctness of reads is **not** affected (DVs are matched by data file 
name; the dead file names never match any split).
   - But it is a metadata leak: orphan DV index entries accumulate on every 
such full compaction, `$table_indexes` becomes misleading, and the stale index 
files cannot be reclaimed properly.
   - Buckets that still have multiple sorted runs at full-compaction time go 
through `MergeTreeCompactTask` and are cleaned correctly — so within one `CALL 
sys.compact` some buckets end up clean and others leak, which is how we noticed 
it in production (8 buckets: 1 cleaned, 4 leaked).
   
   Proposed fix: pass `compactDfSupplier` into `FileRewriteCompactTask` and 
call `result.setDeletionFile(compactDfSupplier.get())` at the end of 
`doCompact()`, mirroring `MergeTreeCompactTask`. I'm happy to submit a PR with 
the fix and the reproducing test.
   
   ### Anything else?
   
   _No response_
   
   ### 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