Conversation
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
R: @tvalentyn would you mind taking a review? Thanks! |
sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaExactlyOnceSink.java
Show resolved
Hide resolved
sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaUnboundedSource.java
Show resolved
Hide resolved
| callback); | ||
|
|
||
| elementsWritten.inc(); | ||
| if (!topicName.equals(reportedLineage)) { |
There was a problem hiding this comment.
Why are we adding this check do we expect to see more than one topic name or it is just to not redo this branch. If we expect more than one are they guaranteed to be appear in together?
There was a problem hiding this comment.
Yes it is possible multiple topics appears in a KafkaWriter. This check reduces some overhead when there is single topic (e.g. set by spec), or there was a GBK upstream that led elements in same topic grouped together and will be processed together. This is the same pattern I used for PubSubIO sink Lineage in #32037 (See diff of PubsubUnboundedSink.java)
If we expect more than one are they guaranteed to be appear in together?
not necessary, depend on whether/how elements were grouped upstream, see above.
* Add Lineage metrics to KafkaIO * Add asserts in tests
3 participants
This PR depends on #32090
Please add a meaningful description for your change here
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.