Skip to content

Set default redistribute key limit for KafkaIO read. - #36124

Merged
Abacn merged 4 commits into
apache:masterfrom
tomstepp:kafka-default
Sep 17, 2025
Merged

Set default redistribute key limit for KafkaIO read.#36124
Abacn merged 4 commits into
apache:masterfrom
tomstepp:kafka-default

Conversation

@tomstepp

@tomstepp tomstepp commented Sep 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Set default redistribute key limit for KafkaIO read.

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

@tomstepp tomstepp changed the title Draft: Set default redistribute keys for KafkaIO read. Set default redistribute key limit for KafkaIO read. Sep 13, 2025
@tomstepp
tomstepp marked this pull request as ready for review September 13, 2025 19:02
@tomstepp

tomstepp commented Sep 13, 2025

Copy link
Copy Markdown
Contributor Author

R: @Abacn

@github-actions

github-actions Bot commented Sep 13, 2025

Copy link
Copy Markdown
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

Abacn
Abacn reviewed Sep 16, 2025
View reviewed changes

@Abacn Abacn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would this change make this branch no longer accessible?

beam/sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIO.java

Line 1848 in 6ef60bb

if (kafkaRead.getRedistributeNumKeys() == 0) {

@tomstepp

tomstepp commented Sep 16, 2025

Copy link
Copy Markdown
Contributor Author

Would this change make this branch no longer accessible?

beam/sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIO.java

Line 1848 in 6ef60bb

if (kafkaRead.getRedistributeNumKeys() == 0) {

I think users could override to zero to get no bucketing. Do you think that's okay? We can leave for now as a way to achieve the original behavior in case there are any edge cases?

@Abacn

Abacn commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

Would this change make this branch no longer accessible?

beam/sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIO.java

Line 1848 in 6ef60bb

if (kafkaRead.getRedistributeNumKeys() == 0) {

I think users could override to zero to get no bucketing. Do you think that's okay? We can leave for now as a way to achieve the original behavior in case there are any edge cases?

I see, maybe add a javadoc in (withRedistributeNumKeys) noting the default number of buckets (2^15) and set to 0 to disable bucketing?

https://github.com/apache/beam/pull/36124/files#diff-ee81e93a5689a74087a1451bfaac1ef921b0b0254830b9e5450a0a5b6cf2c227R2687

@tomstepp

tomstepp commented Sep 16, 2025

Copy link
Copy Markdown
Contributor Author

Updated javadoc, thanks!

@tomstepp

tomstepp commented Sep 16, 2025

Copy link
Copy Markdown
Contributor Author

Assuming merge was awaiting for checks to go green, looks good now

@Abacn
Abacn merged commit 575df4e into apache:master Sep 17, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Abacn Abacn Abacn approved these changes

Assignees

No one assigned

Labels

io java kafka

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tomstepp @Abacn