You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/09/21 11:58:59 UTC
[GitHub] [kafka] divijvaidya opened a new pull request, #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
divijvaidya opened a new pull request, #12669:
URL: https://github.com/apache/kafka/pull/12669
Test is failing intermittently because we do not wait for propagation of the altered config (LogRetentionTimeMillisProp) across all brokers before proceeding ahead with the test.
This PR makes the following changes:
1. Wait for propagation of altered configuration to propagate to all brokers.
2. Use the existing `killBroker` utility method which waits for shutdown using `awaitshutdown`.
3. Improve code readability by using `TestUtils.incrementalAlterConfigs` to send alter config requests.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dengziming commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
dengziming commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r981887516
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -159,22 +158,6 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForTopics(client, List(), topics)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
- @ValueSource(strings = Array("zk")) // KRaft mode will be supported in KAFKA-13910
Review Comment:
I saw the comments below and got 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dengziming commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
dengziming commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r985181862
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -159,22 +158,6 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForTopics(client, List(), topics)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
- @ValueSource(strings = Array("zk")) // KRaft mode will be supported in KAFKA-13910
Review Comment:
Yeah, metadata refresh is tested in other test cases so it's unnecessary to test it additionally.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] divijvaidya commented on pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
divijvaidya commented on PR #12669:
URL: https://github.com/apache/kafka/pull/12669#issuecomment-1253922789
@showuon please take a look whenever you get a chance.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
hachikuji commented on PR #12669:
URL: https://github.com/apache/kafka/pull/12669#issuecomment-1261291916
I started one more build since the last one had a bunch of failures due to a lingering thread.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji merged pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
hachikuji merged PR #12669:
URL: https://github.com/apache/kafka/pull/12669
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] divijvaidya commented on pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
divijvaidya commented on PR #12669:
URL: https://github.com/apache/kafka/pull/12669#issuecomment-1259136260
@hachikuji this is ready for your review. The test failures are unrelated:
```
Build / JDK 8 and Scala 2.12 / testTransactionAfterProducerIdExpires(String).quorum=zk – kafka.api.TransactionsExpirationTest
Build / JDK 8 and Scala 2.12 / testTransactionAfterProducerIdExpires(String).quorum=zk – kafka.api.TransactionsExpirationTest
Build / JDK 8 and Scala 2.12 / testTransactionAfterProducerIdExpires(String).quorum=kraft – kafka.api.TransactionsExpirationTest
Build / JDK 17 and Scala 2.13 / [1] tlsProtocol=TLSv1.2, useInlinePem=false – org.apache.kafka.common.network.SslTransportLayerTest
Build / JDK 17 and Scala 2.13 / shouldWorkWithRebalance – org.apache.kafka.streams.integration.SmokeTestDriverIntegrationTest
```
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] divijvaidya commented on pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
divijvaidya commented on PR #12669:
URL: https://github.com/apache/kafka/pull/12669#issuecomment-1262126497
Test failures are unrelated.
> Build / JDK 17 and Scala 2.13 / testTransactionAfterProducerIdExpires(String).quorum=zk - kafka.api.TransactionsExpirationTest
> Build / JDK 17 and Scala 2.13 / testTaskRequestWithOldStartMsGetsUpdated() – org.apache.kafka.trogdor.coordinator.CoordinatorTest
> Build / JDK 8 and Scala 2.12 / testTransactionAfterProducerIdExpires(String).quorum=zk – kafka.api.TransactionsExpirationTest
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r981539256
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -169,8 +168,7 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForTopics(client, expectedPresent = topics, expectedMissing = List())
val controller = brokers.find(_.config.brokerId == brokers.flatMap(_.metadataCache.getControllerId).head).get
- controller.shutdown()
- controller.awaitShutdown()
+ killBroker(controller.config.brokerId)
Review Comment:
The value of the test still seems questionable to me. The call `waitForTopics` ensures metadata propagation to at least one broker. The call to `describeTopics` below after we have killed the controller is very likely to hit the same broker because the admin client biases toward open connections. A better test of metadata durability would probably kill all brokers and restart them. Anyway, we don't need to fix that 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r978167386
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2523,19 +2522,29 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
def testCreateTopicsReturnsConfigs(quorum: String): Unit = {
client = Admin.create(super.createConfig)
- val alterMap = new util.HashMap[ConfigResource, util.Collection[AlterConfigOp]]
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER, ""), util.Arrays.asList(
- new AlterConfigOp(new ConfigEntry(KafkaConfig.LogRetentionTimeMillisProp, "10800000"), OpType.SET)))
- (brokers.map(_.config) ++ controllerServers.map(_.config)).foreach { case config =>
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER, config.nodeId.toString()),
- util.Arrays.asList(new AlterConfigOp(new ConfigEntry(
- KafkaConfig.LogCleanerDeleteRetentionMsProp, "34"), OpType.SET)))
+ val newLogRetentionProperties = new Properties
+ newLogRetentionProperties.put(KafkaConfig.LogRetentionTimeMillisProp, "10800000")
+ TestUtils.incrementalAlterConfigs(null, client, newLogRetentionProperties, perBrokerConfig = false)
+ .all().get(15, TimeUnit.SECONDS)
+
+ val newLogCleanerDeleteRetention = new Properties
+ newLogCleanerDeleteRetention.put(KafkaConfig.LogCleanerDeleteRetentionMsProp, "34")
+ TestUtils.incrementalAlterConfigs(brokers, client, newLogCleanerDeleteRetention, perBrokerConfig = true)
+ .all().get(15, TimeUnit.SECONDS)
+
+ if (isKRaftTest()) {
+ ensureConsistentKRaftMetadata()
+ } else {
+ waitUntilTrue(() => brokers.forall(_.config.originals.getOrDefault(
Review Comment:
Fair enough. The assertions below do verify the overridden properties in the CreateTopics response, which is the point of the test.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] divijvaidya commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
divijvaidya commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r977506540
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2523,19 +2522,29 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
def testCreateTopicsReturnsConfigs(quorum: String): Unit = {
client = Admin.create(super.createConfig)
- val alterMap = new util.HashMap[ConfigResource, util.Collection[AlterConfigOp]]
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER, ""), util.Arrays.asList(
- new AlterConfigOp(new ConfigEntry(KafkaConfig.LogRetentionTimeMillisProp, "10800000"), OpType.SET)))
- (brokers.map(_.config) ++ controllerServers.map(_.config)).foreach { case config =>
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER, config.nodeId.toString()),
- util.Arrays.asList(new AlterConfigOp(new ConfigEntry(
- KafkaConfig.LogCleanerDeleteRetentionMsProp, "34"), OpType.SET)))
+ val newLogRetentionProperties = new Properties
+ newLogRetentionProperties.put(KafkaConfig.LogRetentionTimeMillisProp, "10800000")
+ TestUtils.incrementalAlterConfigs(null, client, newLogRetentionProperties, perBrokerConfig = false)
+ .all().get(15, TimeUnit.SECONDS)
+
+ val newLogCleanerDeleteRetention = new Properties
+ newLogCleanerDeleteRetention.put(KafkaConfig.LogCleanerDeleteRetentionMsProp, "34")
+ TestUtils.incrementalAlterConfigs(brokers, client, newLogCleanerDeleteRetention, perBrokerConfig = true)
+ .all().get(15, TimeUnit.SECONDS)
+
+ if (isKRaftTest()) {
+ ensureConsistentKRaftMetadata()
+ } else {
+ waitUntilTrue(() => brokers.forall(_.config.originals.getOrDefault(
Review Comment:
Please correct me if I am wrong here.
My understanding is that `ensureConsistentKRaftMetadata` should ensure that the brokers have caught with the metadata available in kraft controllers. For alter config in this scenario, we are sure that one of the controller server contains the new configuration because we wait for the AlterConfig future to complete. Controller should have stored the new configuration in its metadata log before completing the future and `ensureConsistentKRaftMetadata` will ensure that all brokers are consistent with the controller.
Also note that other tests such as `testAppendConfig`, `testInvalidIncrementalAlterConfigs` and many more rely on `ensureConsistentKRaftMetadata` to ensure propagating before asserting in test.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] divijvaidya commented on pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
divijvaidya commented on PR #12669:
URL: https://github.com/apache/kafka/pull/12669#issuecomment-1253922098
Test failures are unrelated:
```
Build / JDK 8 and Scala 2.12 / testFailureToFenceEpoch(String).quorum=kraft – kafka.api.TransactionsTest
Build / JDK 11 and Scala 2.13 / shouldWorkWithUncleanShutdownWipeOutStateStore[exactly_once_v2] – org.apache.kafka.streams.integration.EOSUncleanShutdownIntegrationTest
```
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] divijvaidya commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
divijvaidya commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r978518711
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2523,19 +2522,29 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
def testCreateTopicsReturnsConfigs(quorum: String): Unit = {
client = Admin.create(super.createConfig)
- val alterMap = new util.HashMap[ConfigResource, util.Collection[AlterConfigOp]]
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER, ""), util.Arrays.asList(
- new AlterConfigOp(new ConfigEntry(KafkaConfig.LogRetentionTimeMillisProp, "10800000"), OpType.SET)))
- (brokers.map(_.config) ++ controllerServers.map(_.config)).foreach { case config =>
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER, config.nodeId.toString()),
- util.Arrays.asList(new AlterConfigOp(new ConfigEntry(
- KafkaConfig.LogCleanerDeleteRetentionMsProp, "34"), OpType.SET)))
+ val newLogRetentionProperties = new Properties
+ newLogRetentionProperties.put(KafkaConfig.LogRetentionTimeMillisProp, "10800000")
+ TestUtils.incrementalAlterConfigs(null, client, newLogRetentionProperties, perBrokerConfig = false)
+ .all().get(15, TimeUnit.SECONDS)
+
+ val newLogCleanerDeleteRetention = new Properties
+ newLogCleanerDeleteRetention.put(KafkaConfig.LogCleanerDeleteRetentionMsProp, "34")
+ TestUtils.incrementalAlterConfigs(brokers, client, newLogCleanerDeleteRetention, perBrokerConfig = true)
+ .all().get(15, TimeUnit.SECONDS)
+
+ if (isKRaftTest()) {
+ ensureConsistentKRaftMetadata()
+ } else {
+ waitUntilTrue(() => brokers.forall(_.config.originals.getOrDefault(
Review Comment:
I am resolving this comment. Please let me know if I missed changing something 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r985165490
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -159,22 +158,6 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForTopics(client, List(), topics)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
- @ValueSource(strings = Array("zk")) // KRaft mode will be supported in KAFKA-13910
Review Comment:
Since we deleted this code, should we close https://issues.apache.org/jira/browse/KAFKA-13910?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] divijvaidya commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
divijvaidya commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r978518387
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -169,8 +168,7 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForTopics(client, expectedPresent = topics, expectedMissing = List())
val controller = brokers.find(_.config.brokerId == brokers.flatMap(_.metadataCache.getControllerId).head).get
- controller.shutdown()
- controller.awaitShutdown()
+ killBroker(controller.config.brokerId)
Review Comment:
> We have already verified metadata propagation in the waitForTopics above
I think this assumption is not correct. `waitForTopics` sends a `metadata` API call to a broker (not necessarily the controller). In such a situation, it is not guaranteed that the broker will have the latest metadata in the metadata cache. (e.g. maybe the broker listener has not updated it's metadata cache from zk yet). Am I right in saying this?
> I feel like we could also just delete the test since it is probably covered by every integration test which kills the controller.
The test is meant to validate that the metadata is durable even when controller is replaced. Other tests in this class that use `killBroker` do not necessarily kill the controller, hence, I believe that there is value in keeping this test here.
I agree that we can change the name of the test. I have renamed it to `testMetadataDurabilityOnControllerFailure`.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r978171607
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -169,8 +168,7 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForTopics(client, expectedPresent = topics, expectedMissing = List())
val controller = brokers.find(_.config.brokerId == brokers.flatMap(_.metadataCache.getControllerId).head).get
- controller.shutdown()
- controller.awaitShutdown()
+ killBroker(controller.config.brokerId)
Review Comment:
The point of this test is a little unclear to me. We have already verified metadata propagation in the `waitForTopics` above. Not sure why killing the controller would change that. Anyway, perhaps a more accurate name for the test would be something like `testCreatedTopicStillVisibleAfterControllerChange`? I feel like we could also just delete the test since it is probably covered by every integration test which kills the controller.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r977071628
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2523,19 +2522,29 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
def testCreateTopicsReturnsConfigs(quorum: String): Unit = {
client = Admin.create(super.createConfig)
- val alterMap = new util.HashMap[ConfigResource, util.Collection[AlterConfigOp]]
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER, ""), util.Arrays.asList(
- new AlterConfigOp(new ConfigEntry(KafkaConfig.LogRetentionTimeMillisProp, "10800000"), OpType.SET)))
- (brokers.map(_.config) ++ controllerServers.map(_.config)).foreach { case config =>
- alterMap.put(new ConfigResource(ConfigResource.Type.BROKER, config.nodeId.toString()),
- util.Arrays.asList(new AlterConfigOp(new ConfigEntry(
- KafkaConfig.LogCleanerDeleteRetentionMsProp, "34"), OpType.SET)))
+ val newLogRetentionProperties = new Properties
+ newLogRetentionProperties.put(KafkaConfig.LogRetentionTimeMillisProp, "10800000")
+ TestUtils.incrementalAlterConfigs(null, client, newLogRetentionProperties, perBrokerConfig = false)
+ .all().get(15, TimeUnit.SECONDS)
+
+ val newLogCleanerDeleteRetention = new Properties
+ newLogCleanerDeleteRetention.put(KafkaConfig.LogCleanerDeleteRetentionMsProp, "34")
+ TestUtils.incrementalAlterConfigs(brokers, client, newLogCleanerDeleteRetention, perBrokerConfig = true)
+ .all().get(15, TimeUnit.SECONDS)
+
+ if (isKRaftTest()) {
+ ensureConsistentKRaftMetadata()
+ } else {
+ waitUntilTrue(() => brokers.forall(_.config.originals.getOrDefault(
Review Comment:
Do we not want to verify propagation for kraft as well? I think `ensureConsistentKRaftMetadata` only ensures brokers have caught up.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dengziming commented on a diff in pull request #12669: KAFKA-14248: Fix flaky test PlaintextAdminIntegrationTest.testCreateTopicsReturnsConfigs
Posted by GitBox <gi...@apache.org>.
dengziming commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r981881965
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -159,22 +158,6 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
waitForTopics(client, List(), topics)
}
- @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
- @ValueSource(strings = Array("zk")) // KRaft mode will be supported in KAFKA-13910
Review Comment:
Why do we remove this, Is this test meaningless?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org