raghav-reglobe opened a new pull request, #2756:
URL: https://github.com/apache/iceberg-rust/pull/2756

   ## Summary
   
   Adds `Transaction::rewrite_manifests()` — a `RewriteManifestsAction` 
mirroring Java's `BaseRewriteManifests`: consolidate the current snapshot's 
data manifests (default partition spec) into fewer, target-sized manifests, 
committed as an `Operation::Replace` snapshot without changing any data. Delete 
manifests and older-spec data manifests are carried forward unchanged (Java 
parity — cross-spec rewriting needs per-spec grouping, deliberately out of 
scope here).
   
   Together with the existing `expire_snapshots()`, this continues filling out 
the table-maintenance family (`rewrite data files → expire snapshots → rewrite 
manifests → remove orphan files`). Sibling PRs: #2754 (`FileIO::list_prefix`) 
and #2755 (`RemoveOrphanFilesAction`) — this one is independent of both.
   
   ## Correctness properties (each covered by a test)
   
   - **ALIVE entries** (`Added`/`Existing`) are carried into the new manifests 
as `Existing`, preserving their original `snapshot_id`, data sequence number 
and file sequence number.
   - **Already-DELETED entries are dropped, never resurrected.** A long-lived 
table accumulates DELETED entries in rewritten manifests; carrying one forward 
as `Existing` restores a removed file — for V3 deletion vectors that produces 
multiple "live" DVs per data file and readers fail with `Can't index multiple 
DVs`. We hit exactly this in production with another engine's rewrite path, so 
the regression test plants a DELETED entry and asserts the removed file never 
reappears.
   - **Unresolved sequence numbers fail loud** — silently rewriting such an 
entry would make it adopt the new snapshot's sequence number, so files would 
appear newer than they are and deletes would stop applying.
   - **Live-file preservation check**: every alive entry collected at plan time 
must land in the new snapshot, or the commit errors.
   - **`min_input_manifests`** (default 2): fewer candidates → 
`PreconditionFailed`, so callers can treat "nothing to consolidate" as a no-op.
   
   Planning happens inside `commit()`, so a transaction commit-retry against a 
refreshed table re-plans against the new current snapshot instead of 
re-applying a stale plan.
   
   ## Supporting changes (kept minimal)
   
   - `SnapshotProducer::new_manifest_writer` becomes `pub(crate)` so the action 
writes its consolidated manifests through the producer's naming/counters — the 
`SnapshotProduceOperation` trait itself is untouched (the operation just 
returns the precomputed manifest list).
   - `update_snapshot_summaries` accepts `Operation::Replace`.
   - Snapshot summary carries `manifests-kept` / `manifests-created` / 
`manifests-replaced` / `entries-processed`, matching Java.
   
   ## Validation
   
   5 tests: consolidation with lineage preservation asserted per entry, the 
DELETED-entry resurrection guard, delete-manifest carry-forward, the 
`PreconditionFailed` no-op path, and stale-snapshot validation. Fixtures that 
need Existing/Deleted entries or delete manifests are built manually with 
`ManifestWriterBuilder`/`ManifestListWriter` (same pattern as `append.rs`'s 
delete-only-manifest fixture). Full `iceberg` suite green (1380 passed); `cargo 
fmt` clean; `public-api.txt` regenerated.
   


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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to