feat(compaction)!: support max_source_rows and max_source_bytes limits - #8235
feat(compaction)!: support max_source_rows and max_source_bytes limits#8235zhangyue19921010 wants to merge 6 commits into
Conversation
… for a single compaction run
github-actions
Bot
added
A-python
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The new budgets need to bound every Lance-owned source the compaction task rewrites and remain usable during rolling Java upgrades. Count Blob v2 sidecars in the byte budget, and make the distributed Java task representation explicitly versioned and backward-readable.
Please mark this PR with the breaking-change label.
zhangyue19921010
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
The implementation matches the stated manifest data/overlay byte contract, so Blob v2 payloads are a scoped limitation rather than a correctness failure. Cross-version Java task deserialization is likewise an existing same-version limitation, not a new stable wire promise.
Operators should not treat this setting as a total compaction-I/O cap; explicitly documenting the Blob v2 exclusion would reduce that risk. Versioning all serialized Java compaction objects together is the right follow-up.
There was a problem hiding this comment.
The Rust, Python, and Java documentation now makes the Blob v2 exclusion explicit and accurately distinguishes this metric from a total compaction-I/O cap. The implementation and public contract are aligned on this head.
Cross-version Java compaction-task serialization remains a non-blocking same-version limitation; versioning all serialized compaction objects together remains the appropriate follow-up.
|
Hi @yanghua would u mind to take a look? Thanks! |
| * href="https://docs.rs/lance/latest/src/lance/dataset/optimize.rs.html#118">rust code</a> for all | ||
| * default values. | ||
| */ | ||
| public class CompactionOptions implements Serializable { |
There was a problem hiding this comment.
This class would be serialized and transferred across the network. The old‑version serialization stream ends after maxSourceFragments, whereas the new readObject unconditionally continues to read maxSourceRows / maxSourceBytes, which may cause deserialization failures during rolling upgrades or replaying old tasks.
Can we assign a compatible fixed CompactionOptions serialVersionUID, and add compatibility handling for missing newly‑added trailing fields in readObject, setting them to Optional.empty() when absent?
There was a problem hiding this comment.
Done. Thanks for your review. :)
Xuanwo
added
K-approved
…maxRows-maxBytes-new # Conflicts: # java/lance-jni/src/optimize.rs
…er writers Pin serialVersionUID to the previously generated UID and treat the maxSourceRows/maxSourceBytes tail fields as unset when absent from the stream, so CompactionTask objects queued by older workers still deserialize during a rolling upgrade. Backed by a checked-in fixture stream produced by the pre-change class.
lance-gatekeeper
Bot
removed
K-approved
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The new revision resolves the remaining Java rolling-upgrade risk by preserving the historical serialization identity and tolerating streams written before the source-budget fields. Together with the documented Blob v2 scope, the implementation now matches the accepted row/byte-budget contract across Rust, Python, and Java.
lance-gatekeeper
Bot
added
the
K-approved
lance-gatekeeper
Bot
removed
the
K-approved
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The Java compatibility implementation remains correct, but this head removes the only checked-in historical-stream regression proving that old queued tasks remain readable. Same-version round trips cannot protect that rolling-upgrade contract, and repository policy requires bug fixes to retain corresponding tests.
Restore an equivalent pre-change CompactionOptions stream fixture—inline or as a checked-in test resource—and assert that old fields survive while maxSourceRows and maxSourceBytes remain empty.
lance-gatekeeper
Bot
added
the
K-changes
3 participants
Closes:#8234