lianetm commented on code in PR #16673:
URL: https://github.com/apache/kafka/pull/16673#discussion_r1707389504
##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumerTest.java:
##########
@@ -1748,8 +1767,7 @@ public void testMultipleBackgroundErrors() {
final KafkaException expectedException2 = new KafkaException("Spam,
Spam, Spam");
final ErrorEvent errorEvent2 = new ErrorEvent(expectedException2);
backgroundEventQueue.add(errorEvent2);
- consumer.assign(singletonList(new TopicPartition("topic", 0)));
- final KafkaException exception = assertThrows(KafkaException.class, ()
-> consumer.poll(Duration.ZERO));
+ final KafkaException exception = assertThrows(KafkaException.class, ()
-> consumer.assign(singletonList(new TopicPartition("topic", 0))));
Review Comment:
oh I hadn't realized this undesired side effect of processing background
events on assign, but of course, I missed that. I would say we don't want this
change in behaviour.
We only need to ensure that the assign does not return until the background
event completes, so not really need to process background events, we just need
`applicationEventHandler.addAndGet(assignmentChangeEvent)` (and I would ensure
we create the event using the `defaultApiTimeout` for the deadline. Makes
sense? we would keep the same behaviour we had on this test (assign does not
throw any background event, poll does).
Just for the record, on the unsubscribe we do need to process background
events because of the rebalanceCallbacks that run in the app thread, but that's
not the case here.
--
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
