Consolidate duplicate ShardedKey implementations in core module#38145
Consolidate duplicate ShardedKey implementations in core module#38145Eliaaazzz wants to merge 2 commits into
Conversation
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 streamlines the Apache Beam Java SDK by consolidating duplicate ShardedKey implementations. By unifying these classes, the change reduces technical debt, eliminates potential confusion regarding shard ID types, and simplifies the auto-sharded write paths in various IO components. The refactoring ensures that existing consumers can continue to use integer-based shard identifiers seamlessly through new helper methods, while improving the overall maintainability of the codebase. Highlights
🧠 New Feature in Public Preview: You can now enable Memory 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 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 counter productive. 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
|
cf8f823 to
3ea385d
Compare
Remove the deprecated org.apache.beam.sdk.values.ShardedKey and the internal (@VisibleForTesting) org.apache.beam.sdk.coders.ShardedKeyCoder, consolidating into the single org.apache.beam.sdk.util.ShardedKey. - Add of(K, int) factory and getShardNumber() accessor to util.ShardedKey so callers using integer shard numbers migrate without logic changes. The int is stored as a 4-byte big-endian byte array and round-trips through the Coder. - toString() displays integer shard number for 4-byte shard IDs instead of raw byte array, preserving log readability. - Migrate all consumers across core SDK, BigQuery IO, WriteFiles, ShardingFunction, Flink runner, and Spark runner. - Simplify auto-sharded write paths in WriteFiles and BatchLoads that previously required awkward conversion between the two ShardedKey types. - Add tests for int-based shard methods and coder round-trip behavior. - Update CHANGES.md with Breaking Changes entry. Wire format note: the removed ShardedKeyCoder (sdk.coders) had a different wire format from ShardedKey.Coder (sdk.util) — reversed field order and VarInt vs ByteArray shard encoding. ShardedKeyCoder was NOT a Beam standard/model coder and was only used transiently within pipeline constructions (GroupByKey, sharding), never for persistent state or checkpoints. The standard model coder (ShardedKey.Coder) is unchanged.
Consolidate the two duplicate
ShardedKeyclasses in the Java SDK core module into a single unified implementation. The deprecatedorg.apache.beam.sdk.values.ShardedKey(already@Deprecatedsince its introduction) and its internal companionorg.apache.beam.sdk.coders.ShardedKeyCoder(@VisibleForTesting) are removed in favor of the singleorg.apache.beam.sdk.util.ShardedKey.What changed:
org.apache.beam.sdk.values.ShardedKey(deprecated, int-based shard) andorg.apache.beam.sdk.coders.ShardedKeyCoder(internal,@VisibleForTesting)of(K, int)factory method andgetShardNumber()accessor toorg.apache.beam.sdk.util.ShardedKey— the int is stored as a 4-byte big-endian byte array and survives coder round-tripstoString()to display integer shard number (not raw byte array) for 4-byte shard IDs, preserving log readabilityWriteFilesandBatchLoadsthat previously had awkward conversion code between the twoShardedKeytypesCHANGES.mdwith Breaking Changes entryWire format note: The removed
ShardedKeyCoderhad a different wire format fromShardedKey.Coder(reversed field order, VarInt vs ByteArray shard encoding).ShardedKeyCoderwas NOT a Beam standard/model coder — it was@VisibleForTestingand only used transiently within pipeline constructions (GroupByKey, sharding), never for persistent state or checkpoints. The standard model coder (ShardedKey.Coderinsdk.util, registered inModelCoderRegistrar) is unchanged.Fixes #37990
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.