cadonna commented on code in PR #12337:
URL: https://github.com/apache/kafka/pull/12337#discussion_r911675917
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StoreChangelogReader.java:
##########
@@ -697,7 +693,7 @@ private Map<TopicPartition, Long>
committedOffsetForChangelogs(final Map<TaskId,
final
Set<TopicPartition> partitions) {
final Map<TopicPartition, Long> committedOffsets;
try {
- committedOffsets = fetchCommittedOffsets(partitions, mainConsumer);
Review Comment:
I think re-throwing the `InterruptedException` is cleaner. Otherwise we
defeat the purpose of interrupting a thread. Especially, in this shutdown case
were we use interruption specifically to interrupt any actions where possible.
The old code path should not be affected since we control the stream thread and
we never interrupt it.
The only drawback that I see is that we need to add the `throws` expression
to the calls in the stacktrace and handle (i.e., ignore) the
`InterruptedException` in the old code path in the stream thread.
As a middle ground, you could catch the `InterruptedException` and just
interrupt the current thread. That would cause the next method that checks the
interrupted state to throw the `InterruptedException`. I would prefer the
re-throwing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
