-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[bug30870]: make consumer polling timeout configurable for KafkaIO.Read #30877
Conversation
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @xianhualiu - the API looks good!
I added some comments on maybe increasing the default + some additional logging to inform users of this timeout. This issue being so obscure and difficult to debug, I worry that users will likely not realize that they should increase the transform's poll timeout.
Additionally, this issue also affects the Python SDK. Are you intending to include a fix to the Python API in this PR or in a follow up PR?
sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIO.java
Outdated
Show resolved
Hide resolved
sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/ReadFromKafkaDoFn.java
Show resolved
Hide resolved
sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/ReadFromKafkaDoFn.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
updated changes.md with changes to make consumer polling timeout configurable for KafkaIO.Read
sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/ReadFromKafkaDoFn.java
Show resolved
Hide resolved
Assigning reviewers. If you would like to opt out of this review, comment R: @bvolpato for label java. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
added break changes
It appears this breaks Java IOs PreCommit: #30941 please fix |
Also as followup |
…fkaIO.Read (apache#30877)" This reverts commit 3f4b256.
This PR has merged into master and the revert change by Yi Hu never merged since the test issues have been fixed. The PR containing the revert is cloded now: #31001. |
addresses #30870. The changes in this PR make the consumer polling timeout configurable for KafkaIO.Read with following new command:
KafkaIO.read().withConsumerPollingTimeout(Duration duration)
The duration must be greater than zero. If not specified, the default will be 1 second.