lucasbru commented on code in PR #20123:
URL: https://github.com/apache/kafka/pull/20123#discussion_r2191800588
##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/JoinWithIncompleteMetadataIntegrationTest.java:
##########
@@ -87,10 +90,16 @@ public void cleanup() throws InterruptedException,
IOException {
IntegrationTestUtils.purgeLocalStreamsState(STREAMS_CONFIG);
}
- @Test
- public void testShouldAutoShutdownOnJoinWithIncompleteMetadata() throws
InterruptedException {
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void testShouldAutoShutdownOnJoinWithIncompleteMetadata(final
boolean useNewProtocol) throws InterruptedException {
STREAMS_CONFIG.put(StreamsConfig.APPLICATION_ID_CONFIG, APP_ID);
STREAMS_CONFIG.put(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG,
CLUSTER.bootstrapServers());
+
+ // set group protocol according to parameter
Review Comment:
nit: I'd omit inline comments that don't add any new information
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamThreadTest.java:
##########
@@ -3873,6 +3874,15 @@ public void
testStreamsProtocolRunOnceWithoutProcessingThreads() {
));
thread.runOnceWithoutProcessingThreads();
verify(shutdownErrorHook).run();
+
+ // Test MISSING_SOURCE_TOPICS status
Review Comment:
Can you please add a separate unit test for this?
##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/HandlingSourceTopicDeletionIntegrationTest.java:
##########
@@ -91,6 +95,11 @@ public void shouldThrowErrorAfterSourceTopicDeleted(final
TestInfo testName) thr
streamsConfiguration.put(StreamsConfig.DEFAULT_VALUE_SERDE_CLASS_CONFIG,
Serdes.String().getClass());
streamsConfiguration.put(StreamsConfig.NUM_STREAM_THREADS_CONFIG,
NUM_THREADS);
streamsConfiguration.put(StreamsConfig.METADATA_MAX_AGE_CONFIG, 2000);
+
+ // set group protocol according to parameter
Review Comment:
nit: I'd omit inline comments that don't add any new information
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamThreadTest.java:
##########
@@ -3932,6 +3942,15 @@ public void
testStreamsProtocolRunOnceWithProcessingThreads() {
));
thread.runOnceWithProcessingThreads();
verify(shutdownErrorHook).run();
+
+ // Test MISSING_SOURCE_TOPICS status
Review Comment:
Can you please add a separate unit test for this?
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java:
##########
@@ -1536,6 +1537,10 @@ public void handleStreamsRebalanceData() {
for (final StreamsGroupHeartbeatResponseData.Status status :
streamsRebalanceData.get().statuses()) {
if (status.statusCode() ==
StreamsGroupHeartbeatResponse.Status.SHUTDOWN_APPLICATION.code()) {
shutdownErrorHook.run();
+ } else if (status.statusCode() ==
StreamsGroupHeartbeatResponse.Status.MISSING_SOURCE_TOPICS.code()) {
+ final String errorMsg = "Missing source topics";
+ log.error(errorMsg);
+ throw new MissingSourceTopicException(errorMsg);
Review Comment:
We definitely need a better error message here. The
`StreamsGroupHeartbeatResponse` contains a `statusDetail`. Could we bring it to
this exception?
--
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
