Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-06 Thread via GitHub
chia7712 merged PR #15744: URL: https://github.com/apache/kafka/pull/15744 -- 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:

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-06 Thread via GitHub
mumrah commented on PR #15744: URL: https://github.com/apache/kafka/pull/15744#issuecomment-2096872249 [Latest test run](https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-15744/13/tests) looks pretty reasonable. The failures all look unrelated --

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-06 Thread via GitHub
mumrah commented on PR #15744: URL: https://github.com/apache/kafka/pull/15744#issuecomment-2096415182 Thanks for continued reviews @chia7712 , I've addressed your latest feedback. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-06 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1591129328 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +474,48 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-06 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1591129328 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +474,48 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-06 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1591110984 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -1037,24 +1104,35 @@ class ZkMigrationIntegrationTest {

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-05 Thread via GitHub
chia7712 commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1590547523 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -1073,4 +1151,13 @@ class ZkMigrationIntegrationTest { kraftCluster.close()

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-05 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1590396620 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +474,42 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-03 Thread via GitHub
chia7712 commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1589865773 ## core/src/main/scala/kafka/server/KafkaApis.scala: ## @@ -2992,6 +2992,11 @@ class KafkaApis(val requestChannel: RequestChannel, val preprocessingResponses =

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-03 Thread via GitHub
mumrah commented on PR #15744: URL: https://github.com/apache/kafka/pull/15744#issuecomment-2093513532 So many conflicts  -- 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.

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-05-01 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1586349388 ## core/src/test/java/kafka/test/junit/ZkClusterInvocationContext.java: ## @@ -106,6 +106,7 @@ public List getAdditionalExtensions() { },

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-29 Thread via GitHub
chia7712 commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1584100563 ## core/src/test/java/kafka/test/junit/ZkClusterInvocationContext.java: ## @@ -106,6 +106,7 @@ public List getAdditionalExtensions() { },

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-29 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1583263129 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -950,16 +980,47 @@ class ZkMigrationIntegrationTest {

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-26 Thread via GitHub
chia7712 commented on PR #15744: URL: https://github.com/apache/kafka/pull/15744#issuecomment-2080249371 @mumrah I'm sorry but could you please fix the conflicts again? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-25 Thread via GitHub
chia7712 commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1580175708 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -620,16 +620,19 @@ class ZkMigrationIntegrationTest { // Alter the metadata

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-25 Thread via GitHub
chia7712 commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1580174626 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -852,6 +855,33 @@ class ZkMigrationIntegrationTest { } } +

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-25 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1580150231 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -852,6 +855,33 @@ class ZkMigrationIntegrationTest { } } +

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-25 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1580149700 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -620,16 +620,19 @@ class ZkMigrationIntegrationTest { // Alter the metadata

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-25 Thread via GitHub
chia7712 commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1580130359 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -950,16 +980,47 @@ class ZkMigrationIntegrationTest {

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-24 Thread via GitHub
showuon commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1577340553 ## core/src/test/scala/unit/kafka/zk/AdminZkClientTest.scala: ## @@ -194,6 +199,8 @@ class AdminZkClientTest extends QuorumTestHarness with Logging with RackAwareTes

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-20 Thread via GitHub
soarez commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1573203822 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -950,16 +980,47 @@ class ZkMigrationIntegrationTest {

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
mumrah commented on PR #15744: URL: https://github.com/apache/kafka/pull/15744#issuecomment-2067204988 Updated to include a CheckOp on the `/controller` ZNode. We don't both using the controller epoch since it is not straightforward to consistently read the controller and controller epoch

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572801229 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572780325 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572777334 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572776248 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -950,16 +980,47 @@ class ZkMigrationIntegrationTest {

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
cmccabe commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572722514 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
cmccabe commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572717868 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
cmccabe commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572716567 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
cmccabe commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572716567 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
soarez commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572295590 ## core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala: ## @@ -950,16 +980,47 @@ class ZkMigrationIntegrationTest {

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-19 Thread via GitHub
showuon commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1572140166 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-17 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1569214270 ## core/src/main/scala/kafka/server/KafkaApis.scala: ## @@ -3003,9 +3008,10 @@ class KafkaApis(val requestChannel: RequestChannel, } } +// Forwarding

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-17 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1569208039 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-17 Thread via GitHub
akhileshchg commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1569190991 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-17 Thread via GitHub
mumrah commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1569103206 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo *

Re: [PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-17 Thread via GitHub
johnnychhsu commented on code in PR #15744: URL: https://github.com/apache/kafka/pull/15744#discussion_r1569005655 ## core/src/main/scala/kafka/zk/KafkaZkClient.scala: ## @@ -467,13 +470,33 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo

[PR] KAFKA-16539 Fix IncrementalAlterConfigs during ZK migration [kafka]

2024-04-17 Thread via GitHub
mumrah opened a new pull request, #15744: URL: https://github.com/apache/kafka/pull/15744 This patch fixes two issues with IncrementalAlterConfigs and the ZK migration. First, it changes the handling of IncrementalAlterConfigs to check if the controller is ZK vs KRaft and only forward for