luoyuxia opened a new pull request, #3519:
URL: https://github.com/apache/fluss/pull/3519

   <!--
   Generated-by: Claude Opus 4.8 (Claude Code) following the guidelines 
(https://github.com/apache/fluss/blob/main/AGENTS.md)
   -->
   
   ### Purpose
   
   Linked issue: close #xxx <!-- TODO: replace with the GitHub issue number -->
   
   For Paimon deletion-vector (DV) enabled tables, 
`DvTableReadableSnapshotRetriever.getReadableSnapshotAndOffsets` computes, for 
a just-committed APPEND (tiered) snapshot, the readable snapshot + per-bucket 
readable offsets, and an `earliestSnapshotIdToKeep` telling Fluss which older 
lake snapshots are safe to delete. This PR fixes two correctness bugs in that 
path.
   
   **Bug 1 - scanning the wrong snapshot (ineffective options).**
   `getBucketsWithoutL0AndWithL0(snapshot)` decided which buckets have L0 by 
scanning via `fileStoreTable.copy(scanOptions).store().newScan()`, passing 
`SCAN_SNAPSHOT_ID` and `BATCH_SCAN_MODE` as table options. Those options are 
only consumed by table-level scans (`DataTableBatchScan`), not by store-level 
scans (`AbstractFileStoreScan`), so the scan always hit the *latest* snapshot 
instead of the requested compacted snapshot (`ManifestsReader` falls back to 
`snapshotManager.latestSnapshot()` when no snapshot is specified). Fixed by 
using the direct `.withSnapshot(snapshot)` API, consistent with 
`getBucketsWithFlushedL0` / `PaimonDvTableUtils`.
   
   **Bug 2 - premature snapshot deletion (over-aggressive retention).**
   When the latest compacted snapshot had no L0 in *any* bucket, the code 
shortcut `earliestSnapshotIdToKeep` to that compacted snapshot's previous 
APPEND, assuming nothing earlier was needed. This is unsound: a bucket can be 
clean (no L0) in the current compacted snapshot yet still be *anchored* to an 
older snapshot - it was flushed earlier and has not been flushed since, so its 
base anchor (the previous APPEND of the latest snapshot that exactly holds its 
most recently flushed L0) can be older than the compacted snapshot's previous 
APPEND. Once such a bucket later receives new L0, recomputation traces back to 
that older anchor; if it was already deleted, the retrieval returns `null` and 
readable offsets can no longer advance.
   
   Bug 2 was masked by Bug 1: scanning the latest (appended) snapshot made 
freshly-appended buckets look like "with L0", so they went through the 
per-bucket flush traversal that retained the old anchors. Fixing the scan 
exposed the retention bug.
   
   ### Brief change log
   
   - Scan the requested compacted snapshot via 
`FileStoreScan.withSnapshot(snapshot)` in `getBucketsWithoutL0AndWithL0` 
instead of relying on table options that the store-level scan ignores.
   - Compute the base anchor for buckets **without** L0 as well, and lower 
`earliestSnapshotIdToKeep` to the minimum anchor across all buckets (a bucket's 
anchor only moves forward when it is flushed again, so until then it must be 
retained).
     - Best-effort: if a bucket's flush history has expired and its anchor 
cannot be determined, fall back to `KEEP_ALL_PREVIOUS` (keep everything) rather 
than risk deleting a still-needed snapshot. This never blocks the 
readable-offset advance, since these buckets' offsets are already resolved from 
the latest tiered snapshot.
   - Extract the shared anchor computation into `findBaseAnchorAppendSnapshot`, 
reused by both the with-L0 offset traversal and the new no-L0 retention pass.
   
   ### Tests
   
   - `DvTableReadableSnapshotRetrieverTest#testGetReadableSnapshotAndOffsets` 
(non-partitioned) and `#testGetReadableSnapshotAndOffsetsForPartitionedTable` 
(partitioned) - both run against real Paimon tables and now pass with the 
corrected scan and retention logic.
   - The partitioned test's `earliestSnapshotIdToKeep` expectation, which 
encoded the old over-aggressive value, is updated to the correct minimum-anchor 
value (with an explanatory comment).
   
   ### API and Format
   
   No public API or storage format change.
   
   ### Documentation
   
   No documentation change; behavior-only fix.
   
   ---
   
   **Generative AI disclosure:** Yes - authored with Claude Code (Claude Opus 
4.8). Reviewed by a human developer.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


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