[Iceberg CDC] Add Changelog readers and update resolver#38837
[Iceberg CDC] Add Changelog readers and update resolver#38837ahmedabu98 wants to merge 11 commits into
Conversation
…erg_changelog_readers
…erg_changelog_readers
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the core infrastructure for reading and resolving Iceberg CDC changelogs within Apache Beam. It implements a routing mechanism that distinguishes between unidirectional and bidirectional changelog tasks, ensuring that records are either emitted directly or routed for update resolution based on primary key overlap. By centralizing the reconciliation logic, this change enables consistent handling of CDC data, including the identification of update pairs and the filtering of redundant operations. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for Iceberg Change Data Capture (CDC) processing in Apache Beam, adding classes like CdcResolver, CdcRowDescriptor, LocalResolveDoFn, OverlapRange, and ReadFromChangelogs to handle bi-directional changelog resolution, Copy-on-Write deduplication, and primary key overlap detection. The review feedback highlights several critical issues: a potential ClassCastException in IcebergUtils when casting nested StructLike objects directly to Record, and multiple schema mismatches in OverlapRange, ReadFromChangelogs, and LocalResolveDoFn where projectors are constructed without accounting for metadata columns, which would cause position mismatches during record processing.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
R: @talatuyarer |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
…erg_changelog_readers
| protected int nonPkHash(Record rec) { | ||
| int hash = 1; | ||
| for (Types.NestedField field : nonPkFields) { | ||
| hash = 31 * hash + Objects.hashCode(rec.getField(field.name())); |
There was a problem hiding this comment.
On a table with a non-PK fixed column, a copy-on-write rewrite's delete/insert pair never matches in the hash index, so unchanged rows are emitted as spurious UPDATE_BEFORE/UPDATE_AFTER pairs.
Can we derive both hash and equals from one mechanism
There was a problem hiding this comment.
Can we derive both hash and equals from one mechanism
We could build a single content-based key per overlapping record and do a equals/hashCode for each lookup, but that would be less performant
Correct that the old code was problematic for fixed types cuz Objects.hashCode() would be identity based for byte[]. I covered this by making the hash logic array-aware. This way, we only check deep-equality on a hash collision. I also added tests that a CoW pair with a non-PK fixed column is recognized and dropped.
I'd rather keep it this way cuz it's more performant, but let me know what you think.
|
|
||
| // {PK: (inserts | deletes)} for in-overlap records that need resolution. | ||
| // Records outside the overlap are emitted directly | ||
| StructLikeMap<PkGroup> pkGroups = StructLikeMap.create(ovl.recordIdSchema().asStruct()); |
There was a problem hiding this comment.
Consider noting that while records are buffered based on the 128MB compressed SPLIT_SIZE, the current implementation lacks an inflation factor for decoded data. With no safety check for the actual memory footprint in LocalResolveDoFn, a single process() call can exceed 500MB of heap usage. Furthermore, since the overlap bounds can be null when metrics are missing, the scanner defaults to buffering everything, which risks OOM errors.
There was a problem hiding this comment.
I added a scaling factor in ChangelogScanner to make a rough estimate of the decoded bytes. This reduces the byte size threshold of batches getting routed to LocalResolveDoFn, so we won't see big batches making it there. Should help us avoid OOMs, and we can adjust the factor when we see how it's handled with real usage
…hash logic array-aware
…erg_changelog_readers
…erg_changelog_readers
Adds read transforms that consume planned Iceberg changelog tasks and turn them into Beam CDC rows.
ReadFromChangelogsreads task batches produced byChangelogScanner:INSERTorDELETErowsLocalResolveDoFnreads and resolves small bidirectional task batches in memory.CdcResolvercentralizes the logic for reconciling deletes and inserts for the same primary key. It emits changed pairs asUPDATE_BEFORE/UPDATE_AFTER, and leaves unmatched rows asDELETEorINSERT. Duplicate no-op pairs with identical non-PK fields are dropped.Part of #38831
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.