Re: [PR] Image test improvements [kafka]
ahuang98 commented on code in PR #15373: URL: https://github.com/apache/kafka/pull/15373#discussion_r1544932474 ## metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java: ## @@ -88,7 +101,7 @@ public class ClusterImageTest { setId(2). setEpoch(123). setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")). -setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9093))). +setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9094))). Review Comment: Responded on https://issues.apache.org/jira/browse/KAFKA-16447, someone is volunteering to fix, will check in again tomorrow! -- 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
Re: [PR] Image test improvements [kafka]
ahuang98 commented on code in PR #15373: URL: https://github.com/apache/kafka/pull/15373#discussion_r1544930494 ## metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java: ## @@ -88,7 +101,7 @@ public class ClusterImageTest { setId(2). setEpoch(123). setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")). -setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9093))). +setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9094))). Review Comment: Thanks folks, will open up a fix soon -- 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
Re: [PR] Image test improvements [kafka]
jolshan commented on code in PR #15373: URL: https://github.com/apache/kafka/pull/15373#discussion_r1544929534 ## metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java: ## @@ -88,7 +101,7 @@ public class ClusterImageTest { setId(2). setEpoch(123). setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")). -setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9093))). +setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9094))). Review Comment: I just found this as well. I opened https://issues.apache.org/jira/browse/KAFKA-16451 but will close as a duplicate -- 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
Re: [PR] Image test improvements [kafka]
chia7712 commented on code in PR #15373: URL: https://github.com/apache/kafka/pull/15373#discussion_r1544339792 ## metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java: ## @@ -88,7 +101,7 @@ public class ClusterImageTest { setId(2). setEpoch(123). setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")). -setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9093))). +setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9094))). Review Comment: I have filed a ticket https://issues.apache.org/jira/browse/KAFKA-16447 please feel free to take over it -- 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
Re: [PR] Image test improvements [kafka]
chia7712 commented on code in PR #15373: URL: https://github.com/apache/kafka/pull/15373#discussion_r1544335647 ## metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java: ## @@ -88,7 +101,7 @@ public class ClusterImageTest { setId(2). setEpoch(123). setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")). -setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9093))). +setListeners(Arrays.asList(new Endpoint("PLAINTEXT", SecurityProtocol.PLAINTEXT, "localhost", 9094))). Review Comment: `ReplicaManagerTest` reuse the `IMAGE1` to test the listeners, hence this change break some test cases. Do you have free cycle to fix it? If not, I can file PR to fix it :) -- 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
Re: [PR] Image test improvements [kafka]
mimaison merged PR #15373: URL: https://github.com/apache/kafka/pull/15373 -- 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
Re: [PR] Image test improvements [kafka]
mimaison commented on code in PR #15373: URL: https://github.com/apache/kafka/pull/15373#discussion_r1539799687 ## metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java: ## @@ -186,6 +264,19 @@ public void testImage2RoundTrip() { testToImage(IMAGE2); } +public void testApplyDelta2() { Review Comment: Sorry I should have added that after enabling that test, it is failing. Expected: `brokers([...], 1(BrokerRegistration(id=1, epoch=1001, incarnationId=U52uRe20RsGI0RvpcTx33Q, listeners=[Endpoint(listenerName='PLAINTEXT', securityProtocol=PLAINTEXT, host='localhost', port=9093)], supportedFeatures={foo: 1-3}, rack=Optional.empty, fenced=true, inControlledShutdown=true, isMigratingZkBroker=false, directories=[])), [...]` Actual: `brokers([...], 1(BrokerRegistration(id=1, epoch=1001, incarnationId=U52uRe20RsGI0RvpcTx33Q, listeners=[Endpoint(listenerName='PLAINTEXT', securityProtocol=PLAINTEXT, host='localhost', port=9093)], supportedFeatures={foo: 1-3}, rack=Optional.empty, fenced=false, inControlledShutdown=false, isMigratingZkBroker=false, directories=[])), [...]` Broker 1 has `fenced=false` and `inControlledShutdown=false` while we expected both to be `true`. -- 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
Re: [PR] Image test improvements [kafka]
mimaison commented on code in PR #15373: URL: https://github.com/apache/kafka/pull/15373#discussion_r1539417806 ## metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java: ## @@ -186,6 +264,19 @@ public void testImage2RoundTrip() { testToImage(IMAGE2); } +public void testApplyDelta2() { Review Comment: Is this missing the `@Test` annotation? ## metadata/src/test/java/org/apache/kafka/image/ImageDowngradeTest.java: ## @@ -128,6 +128,39 @@ public void testPreControlledShutdownStateVersion() { TEST_RECORDS.get(1))); } +/** + * Test downgrading to a MetadataVersion that doesn't support ZK migration. + */ +@Test +public void testPreZkMigrationSupportVersion() throws Throwable { Review Comment: This does not throw so we can remove `throws Throwable` -- 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
Re: [PR] Image test improvements [kafka]
mimaison commented on PR #15373: URL: https://github.com/apache/kafka/pull/15373#issuecomment-2012481246 @ahuang98 The [last CI](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15373/3/#showFailuresLink) run for this PR had a lot of test failures. I think if you rebase on trunk that should clear most of them. -- 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