Re: [PR] KAFKA-15853: Move quota configs into server-common package [kafka]

2024-04-23 Thread via GitHub
chia7712 merged PR #15774: URL: https://github.com/apache/kafka/pull/15774 -- 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-15853: Move quota configs into server-common package [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on PR #15774: URL: https://github.com/apache/kafka/pull/15774#issuecomment-2074035809 ``` ./gradlew cleanTest :streams:test --tests EOSUncleanShutdownIntegrationTest.shouldWorkWithUncleanShutdownWipeOutStateStore :tools:test --tests

[PR] KAFKA-16605: Fix the flaky LogCleanerParameterizedIntegrationTest [kafka]

2024-04-23 Thread via GitHub
kamalcph opened a new pull request, #15793: URL: https://github.com/apache/kafka/pull/15793 ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade

Re: [PR] KAFKA-15853: Move quota configs into server-common package [kafka]

2024-04-23 Thread via GitHub
OmniaGM commented on code in PR #15774: URL: https://github.com/apache/kafka/pull/15774#discussion_r1576158733 ## core/src/main/scala/kafka/server/DynamicConfig.scala: ## @@ -36,30 +35,12 @@ import scala.jdk.CollectionConverters._ object DynamicConfig { Review Comment: It

Re: [PR] KAFKA-16560: Refactor/cleanup BrokerNode/ControllerNode/ClusterConfig [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on code in PR #15761: URL: https://github.com/apache/kafka/pull/15761#discussion_r1576155040 ## core/src/test/java/kafka/testkit/TestKitNodes.java: ## @@ -167,11 +186,11 @@ private TestKitNodes( NavigableMap controllerNodes, NavigableMap

Re: [PR] KAFKA-16588: broker shutdown hangs when log.segment.delete.delay.ms is zero [kafka]

2024-04-23 Thread via GitHub
FrankYang0529 commented on PR #15773: URL: https://github.com/apache/kafka/pull/15773#issuecomment-2072172997 > > Yeah, I tried to use unit test. However, I didn't find a good way to check whether kafka-delete-logs task has run or not. I also tried to check whether scheduler can be

Re: [PR] KAFKA-16073: Increment the local-log-start-offset before deleting segments in memory table [kafka]

2024-04-23 Thread via GitHub
kamalcph commented on code in PR #15748: URL: https://github.com/apache/kafka/pull/15748#discussion_r1576175394 ## core/src/test/scala/unit/kafka/log/LogCleanerParameterizedIntegrationTest.scala: ## @@ -121,9 +121,9 @@ class LogCleanerParameterizedIntegrationTest extends

Re: [PR] KAFKA-16073: Increment the local-log-start-offset before deleting segments in memory table [kafka]

2024-04-23 Thread via GitHub
kamalcph commented on code in PR #15748: URL: https://github.com/apache/kafka/pull/15748#discussion_r1576175394 ## core/src/test/scala/unit/kafka/log/LogCleanerParameterizedIntegrationTest.scala: ## @@ -121,9 +121,9 @@ class LogCleanerParameterizedIntegrationTest extends

[jira] [Updated] (KAFKA-16605) Fix the flaky LogCleanerParameterizedIntegrationTest

2024-04-23 Thread Kamal Chandraprakash (Jira)
[ https://issues.apache.org/jira/browse/KAFKA-16605?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kamal Chandraprakash updated KAFKA-16605: - Description:

[jira] [Created] (KAFKA-16605) Fix the flaky LogCleanerParameterizedIntegrationTest

2024-04-23 Thread Kamal Chandraprakash (Jira)
Kamal Chandraprakash created KAFKA-16605: Summary: Fix the flaky LogCleanerParameterizedIntegrationTest Key: KAFKA-16605 URL: https://issues.apache.org/jira/browse/KAFKA-16605 Project: Kafka

Re: [PR] KAFKA-15853: Move quota configs into server-common package [kafka]

2024-04-23 Thread via GitHub
OmniaGM commented on code in PR #15774: URL: https://github.com/apache/kafka/pull/15774#discussion_r1576188830 ## server-common/src/main/java/org/apache/kafka/server/config/QuotaConfigs.java: ## @@ -0,0 +1,154 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under

Re: [PR] KAFKA-16073: Increment the local-log-start-offset before deleting segments in memory table [kafka]

2024-04-23 Thread via GitHub
kamalcph commented on code in PR #15748: URL: https://github.com/apache/kafka/pull/15748#discussion_r1576190364 ## core/src/test/scala/unit/kafka/log/LogCleanerParameterizedIntegrationTest.scala: ## @@ -121,9 +121,9 @@ class LogCleanerParameterizedIntegrationTest extends

[PR] KAFKA-16605: Fix the flaky LogCleanerParameterizedIntegrationTest [kafka]

2024-04-23 Thread via GitHub
kamalcph opened a new pull request, #15787: URL: https://github.com/apache/kafka/pull/15787 ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including

Re: [PR] MINOR: Various cleanups in core [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on code in PR #15786: URL: https://github.com/apache/kafka/pull/15786#discussion_r1576193128 ## core/src/test/scala/unit/kafka/coordinator/group/GroupCoordinatorConcurrencyTest.scala: ## @@ -209,7 +209,7 @@ class GroupCoordinatorConcurrencyTest extends

Re: [PR] KAFKA-16483: migrate DeleteOffsetsConsumerGroupCommandIntegrationTest to use ClusterTestExtensions [kafka]

2024-04-23 Thread via GitHub
FrankYang0529 commented on PR #15679: URL: https://github.com/apache/kafka/pull/15679#issuecomment-2072251579 > > I think we can add new test case in next PR. We can more focus on migrate to ClusterTestExtensions in this PR. > > Please take a look at [#15766

Re: [PR] KAFKA-16465: Fix consumer sys test revocation validation [kafka]

2024-04-23 Thread via GitHub
lucasbru commented on code in PR #15778: URL: https://github.com/apache/kafka/pull/15778#discussion_r1575801343 ## tests/kafkatest/tests/client/consumer_test.py: ## @@ -242,16 +242,15 @@ def test_static_consumer_bounce(self, clean_shutdown, static_membership, bounce_

Re: [PR] KAFKA-15853: Move quota configs into server-common package [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on code in PR #15774: URL: https://github.com/apache/kafka/pull/15774#discussion_r1575855253 ## server-common/src/main/java/org/apache/kafka/server/config/QuotaConfigs.java: ## @@ -0,0 +1,154 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under

Re: [PR] KAFKA-15853: Move socket configs into org.apache.kafka.network.SocketServerConfigs [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on PR #15772: URL: https://github.com/apache/kafka/pull/15772#issuecomment-2071861237 Those failed tests pass on my local. will merge 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

Re: [PR] KAFKA-16483: migrate DeleteOffsetsConsumerGroupCommandIntegrationTest to use ClusterTestExtensions [kafka]

2024-04-23 Thread via GitHub
FrankYang0529 commented on code in PR #15679: URL: https://github.com/apache/kafka/pull/15679#discussion_r1576082586 ## tools/src/test/java/org/apache/kafka/tools/consumer/group/DeleteOffsetsConsumerGroupCommandIntegrationTest.java: ## @@ -202,7 +238,7 @@ private KafkaProducer

Re: [PR] KAFKA-16568: JMH Benchmarks for Server Side Rebalances [kafka]

2024-04-23 Thread via GitHub
dajac commented on code in PR #15717: URL: https://github.com/apache/kafka/pull/15717#discussion_r1576085044 ## jmh-benchmarks/src/main/java/org/apache/kafka/jmh/assignor/ServerSideAssignorBenchmark.java: ## @@ -0,0 +1,281 @@ +/* + * Licensed to the Apache Software Foundation

Re: [PR] KAFKA-16483: migrate DeleteOffsetsConsumerGroupCommandIntegrationTest to use ClusterTestExtensions [kafka]

2024-04-23 Thread via GitHub
FrankYang0529 commented on PR #15679: URL: https://github.com/apache/kafka/pull/15679#issuecomment-2072051627 > @FrankYang0529 sorry that I check the PR again, and more comments are left. PTAL Hi @chia7712, I addressed last comments. I think we can add new test case in next PR. We

Re: [PR] KAFKA-16554: Online downgrade triggering and group type conversion [kafka]

2024-04-23 Thread via GitHub
dajac commented on code in PR #15721: URL: https://github.com/apache/kafka/pull/15721#discussion_r1576113492 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java: ## @@ -1422,10 +1499,16 @@ private CoordinatorResult consumerGr )

Re: [PR] KAFKA-16560: Refactor/cleanup BrokerNode/ControllerNode/ClusterConfig [kafka]

2024-04-23 Thread via GitHub
brandboat commented on PR #15761: URL: https://github.com/apache/kafka/pull/15761#issuecomment-2072088191 > @brandboat Sorry that I leave more comments below. please take a look. thanks Thank you for your patience! I appreciate your input. Already addressed all comments as above.

Re: [PR] MINOR: Modified System.getProperty("line.separator") to java.lang.System.lineSeparator() [kafka]

2024-04-23 Thread via GitHub
TaiJuWu commented on PR #15782: URL: https://github.com/apache/kafka/pull/15782#issuecomment-2072107254 > Thank you for the changes. > > Is there a reason to prefer the `java.lang.` prefix? There is no special reason, Removing prefix is ok. Following other code, Removing

[jira] [Commented] (KAFKA-16604) Deprecate ConfigDef.ConfigKey constructor from public APIs

2024-04-23 Thread Chia-Ping Tsai (Jira)
[ https://issues.apache.org/jira/browse/KAFKA-16604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17840075#comment-17840075 ] Chia-Ping Tsai commented on KAFKA-16604: [~sagarrao] Thanks for opening this jira. Do you have

Re: [PR] KAFKA-16483: migrate DeleteOffsetsConsumerGroupCommandIntegrationTest to use ClusterTestExtensions [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on PR #15679: URL: https://github.com/apache/kafka/pull/15679#issuecomment-2072132702 > I think we can add new test case in next PR. We can more focus on migrate to ClusterTestExtensions in this PR. Please take a look at

Re: [PR] KAFKA-16560: Refactor/cleanup BrokerNode/ControllerNode/ClusterConfig [kafka]

2024-04-23 Thread via GitHub
brandboat commented on code in PR #15761: URL: https://github.com/apache/kafka/pull/15761#discussion_r1576056664 ## core/src/test/java/kafka/testkit/BrokerNode.java: ## @@ -66,11 +74,27 @@ public Builder setNumLogDirectories(int numLogDirectories) { return this;

[PR] MINOR: Various cleanups in core [kafka]

2024-04-23 Thread via GitHub
mimaison opened a new pull request, #15786: URL: https://github.com/apache/kafka/pull/15786 ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including

[jira] [Created] (KAFKA-16604) Deprecate ConfigDef.ConfigKey constructor from public APIs

2024-04-23 Thread Sagar Rao (Jira)
Sagar Rao created KAFKA-16604: - Summary: Deprecate ConfigDef.ConfigKey constructor from public APIs Key: KAFKA-16604 URL: https://issues.apache.org/jira/browse/KAFKA-16604 Project: Kafka Issue

Re: [PR] KAFKA-16592: Add a new constructor which invokes the existing constructor with default value for alternativeString [kafka]

2024-04-23 Thread via GitHub
vamossagar12 commented on PR #15762: URL: https://github.com/apache/kafka/pull/15762#issuecomment-2072021300 @chia7712 , I created this ticket: [KAFKA-16604](https://issues.apache.org/jira/browse/KAFKA-16604) for tracking the KIP -- This is an automated message from the Apache Git

Re: [PR] KAFKA-16461: New consumer fails to consume records in security_test.py system test [kafka]

2024-04-23 Thread via GitHub
lucasbru merged PR #15746: URL: https://github.com/apache/kafka/pull/15746 -- 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-16225: Set `metadata.log.dir` to broker in KRAFT mode in integration test [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on PR #15354: URL: https://github.com/apache/kafka/pull/15354#issuecomment-2071806320 @jolshan Do you plan to backport this to 3.7? I recently try to backport other PRs to 3.7 and encounter the same flaky -- This is an automated message from the Apache Git Service. To

Re: [PR] KAFKA-16082 Avoid resuming future replica if current replica is in the same directory [kafka]

2024-04-23 Thread via GitHub
chia7712 merged PR #15777: URL: https://github.com/apache/kafka/pull/15777 -- 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-15853: Move socket configs into org.apache.kafka.network.SocketServerConfigs [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on PR #15772: URL: https://github.com/apache/kafka/pull/15772#issuecomment-2071831061 > controller.listener.names oh, it will be moved to `KRaftConfigs`. please ignore my previous comments. -- This is an automated message from the Apache Git Service. To respond

Re: [PR] KAFKA-16082 Avoid resuming future replica if current replica is in the same directory [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on PR #15777: URL: https://github.com/apache/kafka/pull/15777#issuecomment-2071783047 The failed tests is fixed by https://github.com/chia7712/kafka/commit/6e998cffdd33e343945877ccee1fec8337c7d57d -- This is an automated message from the Apache Git Service. To respond

Re: [PR] MINOR: Add junit properties to display parameterized test names [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on PR #14983: URL: https://github.com/apache/kafka/pull/14983#issuecomment-2071780823 I have backport this PR to 3.7 see https://github.com/apache/kafka/commit/6e998cffdd33e343945877ccee1fec8337c7d57d -- This is an automated message from the Apache Git Service. To

Re: [PR] KAFKA-16593: Rewrite DeleteConsumerGroupsTest by ClusterTestExtensions [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on code in PR #15766: URL: https://github.com/apache/kafka/pull/15766#discussion_r1576009920 ## tools/src/test/java/org/apache/kafka/tools/consumer/group/DeleteConsumerGroupsTest.java: ## @@ -17,279 +17,448 @@ package org.apache.kafka.tools.consumer.group;

Re: [PR] MINOR: Fix logging of KafkaStreams close [kafka]

2024-04-23 Thread via GitHub
soarez merged PR #15783: URL: https://github.com/apache/kafka/pull/15783 -- 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-16592: Add a new constructor which invokes the existing constructor with default value for alternativeString [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on PR #15762: URL: https://github.com/apache/kafka/pull/15762#issuecomment-2071864618 retrigger QA due to incomplete build. I will merge it after QA get completed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

Re: [PR] KAFKA-15853: Move socket configs into org.apache.kafka.network.SocketServerConfigs [kafka]

2024-04-23 Thread via GitHub
chia7712 merged PR #15772: URL: https://github.com/apache/kafka/pull/15772 -- 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-15265: Add Remote Log Manager quota manager [kafka]

2024-04-23 Thread via GitHub
abhijeetk88 commented on code in PR #15625: URL: https://github.com/apache/kafka/pull/15625#discussion_r1576005665 ## core/src/main/java/kafka/log/remote/quota/RLMQuotaManager.java: ## @@ -0,0 +1,119 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more

Re: [PR] KAFKA-15265: Add Remote Log Manager quota manager [kafka]

2024-04-23 Thread via GitHub
abhijeetk88 commented on code in PR #15625: URL: https://github.com/apache/kafka/pull/15625#discussion_r1576007574 ## core/src/main/java/kafka/log/remote/quota/RLMQuotaManager.java: ## Review Comment: Yes, the integration of the quota manager will come in the follow-up

Re: [PR] KAFKA-16483: migrate DeleteOffsetsConsumerGroupCommandIntegrationTest to use ClusterTestExtensions [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on code in PR #15679: URL: https://github.com/apache/kafka/pull/15679#discussion_r1576013151 ## tools/src/test/java/org/apache/kafka/tools/consumer/group/DeleteOffsetsConsumerGroupCommandIntegrationTest.java: ## @@ -171,29 +209,27 @@ private void

Re: [PR] MINOR: Various cleanups in core [kafka]

2024-04-23 Thread via GitHub
mimaison commented on code in PR #15786: URL: https://github.com/apache/kafka/pull/15786#discussion_r1576233041 ## core/src/test/scala/unit/kafka/coordinator/group/GroupCoordinatorConcurrencyTest.scala: ## @@ -209,7 +209,7 @@ class GroupCoordinatorConcurrencyTest extends

Re: [PR] KAFKA-16483: migrate DeleteOffsetsConsumerGroupCommandIntegrationTest to use ClusterTestExtensions [kafka]

2024-04-23 Thread via GitHub
chia7712 commented on code in PR #15679: URL: https://github.com/apache/kafka/pull/15679#discussion_r1576262016 ## tools/src/test/java/org/apache/kafka/tools/consumer/group/DeleteOffsetsConsumerGroupCommandIntegrationTest.java: ## @@ -170,30 +227,23 @@ private void

Re: [PR] KAFKA-15853: Move quota configs into server-common package [kafka]

2024-04-23 Thread via GitHub
mimaison commented on code in PR #15774: URL: https://github.com/apache/kafka/pull/15774#discussion_r1576247775 ## tools/src/main/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommand.java: ## @@ -98,23 +99,18 @@ public class ReassignPartitionsCommand { static

Re: [PR] KAFKA-10496: Removed relying on external DNS servers in tests [kafka]

2024-04-23 Thread via GitHub
ijuma commented on PR #9315: URL: https://github.com/apache/kafka/pull/9315#issuecomment-2072363189 Fyi, Java 18+ do have a SPI for this: https://bugs.openjdk.org/browse/JDK-8263693 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

Re: [PR] KAFKA-15853: Move quota configs into server-common package [kafka]

2024-04-23 Thread via GitHub
OmniaGM commented on code in PR #15774: URL: https://github.com/apache/kafka/pull/15774#discussion_r1576308101 ## tools/src/main/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommand.java: ## @@ -98,23 +99,18 @@ public class ReassignPartitionsCommand { static

Re: [PR] KAFKA-15853: Move quota configs into server-common package [kafka]

2024-04-23 Thread via GitHub
OmniaGM commented on code in PR #15774: URL: https://github.com/apache/kafka/pull/15774#discussion_r1576308464 ## checkstyle/import-control-metadata.xml: ## @@ -123,6 +123,7 @@ + Review Comment: Fixed -- This is an automated message

Re: [PR] KAFKA-16587: Add subscription model information to group state [kafka]

2024-04-23 Thread via GitHub
dajac commented on code in PR #15785: URL: https://github.com/apache/kafka/pull/15785#discussion_r1576317772 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/consumer/ConsumerGroup.java: ## @@ -918,40 +935,53 @@ private void

Re: [PR] MINOR: Various cleanups in core [kafka]

2024-04-23 Thread via GitHub
OmniaGM commented on code in PR #15786: URL: https://github.com/apache/kafka/pull/15786#discussion_r1576329465 ## core/src/main/scala/kafka/server/ZkAdminManager.scala: ## @@ -959,7 +960,7 @@ class ZkAdminManager(val config: KafkaConfig, } else if

Re: [PR] MINOR: Various cleanups in core [kafka]

2024-04-23 Thread via GitHub
OmniaGM commented on code in PR #15786: URL: https://github.com/apache/kafka/pull/15786#discussion_r1576332250 ## core/src/main/scala/kafka/server/ZkAdminManager.scala: ## @@ -871,7 +872,7 @@ class ZkAdminManager(val config: KafkaConfig,

Re: [PR] MINOR: Various cleanups in core [kafka]

2024-04-23 Thread via GitHub
OmniaGM commented on code in PR #15786: URL: https://github.com/apache/kafka/pull/15786#discussion_r1576333034 ## core/src/main/scala/kafka/server/ZkAdminManager.scala: ## @@ -871,7 +872,7 @@ class ZkAdminManager(val config: KafkaConfig,

Re: [PR] KAFKA-16483: migrate DeleteOffsetsConsumerGroupCommandIntegrationTest to use ClusterTestExtensions [kafka]

2024-04-23 Thread via GitHub
FrankYang0529 commented on PR #15679: URL: https://github.com/apache/kafka/pull/15679#issuecomment-2072421039 > @FrankYang0529 thanks for updated PR. please take a look at two comments. Hi @chia7712, thanks for the review. Updated it. -- This is an automated message from the Apache

Re: [PR] MINOR: Various cleanups in core [kafka]

2024-04-23 Thread via GitHub
OmniaGM commented on code in PR #15786: URL: https://github.com/apache/kafka/pull/15786#discussion_r1576338028 ## core/src/main/scala/kafka/server/ReplicaManager.scala: ## @@ -144,7 +144,7 @@ case class LogReadResult(info: FetchDataInfo, def withEmptyFetchInfo: LogReadResult

Re: [PR] MINOR: Various cleanups in core [kafka]

2024-04-23 Thread via GitHub
OmniaGM commented on code in PR #15786: URL: https://github.com/apache/kafka/pull/15786#discussion_r1576343801 ## core/src/main/scala/kafka/server/ReplicaManager.scala: ## @@ -300,7 +300,7 @@ class ReplicaManager(val config: KafkaConfig, protected val allPartitions = new

<    1   2