Support rewrite_manifests table maintenance#3631
Draft
moomindani wants to merge 6 commits into
Draft
Conversation
Add table.maintenance.rewrite_manifests(), which merges the current snapshot's data manifests into fewer manifests sized by commit.manifest.target-size-bytes. Entries are rewritten as EXISTING and keep their sequence numbers; delete manifests and manifests that need no merging are kept as-is. The result is committed as a replace snapshot whose totals carry over unchanged. V3 tables are rejected for now: rewriting must preserve the first-row-id of rewritten manifests, which needs the read side of row lineage (apache#3621). Closes apache#3629
Committing a replace snapshot that merely re-lists the same manifests has no value; return no updates instead, and assert in the no-op test that the current snapshot is unchanged. Document that rewrites carry live entries only, matching the reference implementation, and add the Spark interop integration test (data, snapshot history, file and manifest counts, and pre-rewrite time travel verified from Spark).
tests/table/test_rewrite_manifests.py and the integration test shared a module name, which fails mypy collection.
The target was exactly twice the first manifest's length, so a second manifest one byte larger started its own group and the test was flaky.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3629
Rationale for this change
Adds
table.maintenance.rewrite_manifests(), the explicit maintenance counterpart of Java'sTable.rewriteManifests()(Spark:rewrite_manifests). When a table accumulates many small manifests from repeated appends, scan planning slows down; this merges the current snapshot's data manifests into fewer manifests sized bycommit.manifest.target-size-bytes(default 8 MB).replacesnapshot; snapshot totals carry over unchanged (update_snapshot_summariesnow handlesOperation.REPLACE), and the summary reportsmanifests-created/manifests-kept/manifests-replaced/entries-processed, matching Java.first-row-idof rewritten manifests, which needs the read side of row lineage (Support row lineage assignment on commit #3621). The writer-side plumbing already exists in Support writing V3 manifests and manifest lists #3624.Related: #270 tracks automatic manifest merging on write; this PR covers only the explicit maintenance API.
Draft while the API shape settles: #3124 is introducing
table.maintenance.compact()on the sameMaintenanceTable, and I'd like this to land consistently with it.Are these changes tested?
Yes, five tests using an in-memory catalog with real appends: merging three manifests into one (entry counts, EXISTING status, data unchanged, replace summary with carried-over totals), sequence-number preservation including the merged manifest's
min_sequence_number, single-manifest no-op (kept as-is, not rewritten), target-size grouping, andrewrites_needed(). All fail without the implementation.Are there any user-facing changes?
Yes: a new
table.maintenance.rewrite_manifests()API.This pull request and its description were written by Claude Fable 5.