Skip to content

Handle compacting an empty delta - #607

Merged
yankevn merged 6 commits into
mainfrom
empty_delta
Apr 17, 2026
Merged

Handle compacting an empty delta#607
yankevn merged 6 commits into
mainfrom
empty_delta

Conversation

@yankevn

@yankevn yankevn commented Apr 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

This change handles the case where the delta results in no materialize results. In this case, committing the delta will be skipped in favor of directly writing back the RCF.

Rationale

Empty deltas may occur, which will result in an assertion error during the merge deltas step while processing the merge results.

Changes

List the major changes made in this pull request.

Impact

Discuss any potential impacts the changes may have on existing functionalities.

Testing

Describe how the changes have been tested, including both automated and manual testing strategies.
If this is a bugfix, explain how the fix has been tested to ensure the bug is resolved without introducing new issues.

Regression Risk

If this is a bugfix, assess the risk of regression caused by this fix and steps taken to mitigate it.

Checklist

  • Unit tests covering the changes have been added

    • If this is a bugfix, regression tests have been added
  • E2E testing has been performed

Additional Notes

Any additional information or context relevant to this PR.

@yankevn
yankevn marked this pull request as ready for review April 17, 2026 00:26

@Zyiqin-Miranda Zyiqin-Miranda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a high-level, any particular reason (I assume just for minimal changes) to commit an empty delta with empty manifests?

Comment thread deltacat/compute/compactor_v2/private/compaction_utils.py Outdated
@yankevn

yankevn commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator Author

Yeah, committing an empty delta means that at least there's something committed vs an empty staged partition. When I initially tested by just skipping the committed delta, the follow-up job failed, which is why I added the empty delta.

@yankevn
yankevn requested a review from Zyiqin-Miranda April 17, 2026 18:49

@Zyiqin-Miranda Zyiqin-Miranda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, maybe good idea to add empty->incremental, incremental->empty->incremental test cases to guard the behavior, especially around stream_position/rcf fetching/deriving.

@yankevn
yankevn merged commit b570f2f into main Apr 17, 2026
3 checks passed
@yankevn
yankevn deleted the empty_delta branch April 17, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants