[GitHub] [kafka] vcrfxia commented on pull request #13264: KAFKA-14491: [12/N] Relax requirement that KTable stores must be TimestampedKVStores

2023-03-01 Thread via GitHub
vcrfxia commented on PR #13264: URL: https://github.com/apache/kafka/pull/13264#issuecomment-1451404037 > There is test failures. Could it be related to this PR? Good catch. These failures only appeared when I switched from the original try-catch approach for casting stores to either

[GitHub] [kafka] vcrfxia commented on a diff in pull request #13264: KAFKA-14491: [12/N] Relax requirement that KTable stores must be TimestampedKVStores

2023-03-01 Thread via GitHub
vcrfxia commented on code in PR #13264: URL: https://github.com/apache/kafka/pull/13264#discussion_r1122676573 ## streams/src/main/java/org/apache/kafka/streams/state/internals/KeyValueStoreWrapper.java: ## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation

[GitHub] [kafka] showuon merged pull request #13172: KAFKA-14590: Move DelegationTokenCommand to tools

2023-03-01 Thread via GitHub
showuon merged PR #13172: URL: https://github.com/apache/kafka/pull/13172 -- 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:

[GitHub] [kafka] showuon commented on pull request #13172: KAFKA-14590: Move DelegationTokenCommand to tools

2023-03-01 Thread via GitHub
showuon commented on PR #13172: URL: https://github.com/apache/kafka/pull/13172#issuecomment-1451363118 Failed tests are unrelated ``` Build / JDK 17 and Scala 2.13 / org.apache.kafka.controller.QuorumControllerTest.testUnregisterBroker() Build / JDK 8 and Scala 2.12 /

[jira] [Commented] (KAFKA-14768) proposal to reduce the first message's send time cost and max block time for safety

2023-03-01 Thread Luke Chen (Jira)
[ https://issues.apache.org/jira/browse/KAFKA-14768?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17695451#comment-17695451 ] Luke Chen commented on KAFKA-14768: --- For (1), could you explain what "warmup" messages mean? How could

[GitHub] [kafka] satishd commented on a diff in pull request #13304: KAFKA-14726 Move/rewrite of LogReadInfo, LogOffsetSnapshot, LogStartOffsetIncrementReason to storage module

2023-03-01 Thread via GitHub
satishd commented on code in PR #13304: URL: https://github.com/apache/kafka/pull/13304#discussion_r1121995162 ## storage/src/main/java/org/apache/kafka/storage/internals/log/LogReadInfo.java: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under

[jira] [Commented] (KAFKA-14771) Include current thread ids in ConcurrentModificationException message

2023-03-01 Thread Luke Chen (Jira)
[ https://issues.apache.org/jira/browse/KAFKA-14771?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17695434#comment-17695434 ] Luke Chen commented on KAFKA-14771: --- [~pierDipi] , the suggestion makes sense to me. Welcome to submit

[jira] [Resolved] (KAFKA-14729) The kafakConsumer pollForFetches(timer) method takes up a lot of cpu due to the abnormal exit of the heartbeat thread

2023-03-01 Thread Luke Chen (Jira)
[ https://issues.apache.org/jira/browse/KAFKA-14729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Luke Chen resolved KAFKA-14729. --- Fix Version/s: 3.5.0 Resolution: Fixed > The kafakConsumer pollForFetches(timer) method

[GitHub] [kafka] showuon merged pull request #13270: KAFKA-14729: The kafakConsumer pollForFetches(timer) method takes up a lot of cpu due to the abnormal exit of the heartbeat thread

2023-03-01 Thread via GitHub
showuon merged PR #13270: URL: https://github.com/apache/kafka/pull/13270 -- 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:

[GitHub] [kafka] CalvinConfluent opened a new pull request, #13323: Add ReplicaState to FetchRequest.

2023-03-01 Thread via GitHub
CalvinConfluent opened a new pull request, #13323: URL: https://github.com/apache/kafka/pull/13323 As the first part of the [KIP-903](https://cwiki.apache.org/confluence/display/KAFKA/KIP-903%3A+Replicas+with+stale+broker+epoch+should+not+be+allowed+to+join+the+ISR), it updates the

[GitHub] [kafka] kirktrue commented on a diff in pull request #13301: KAFKA-14758: Extract inner classes from Fetcher for reuse in refactoring

2023-03-01 Thread via GitHub
kirktrue commented on code in PR #13301: URL: https://github.com/apache/kafka/pull/13301#discussion_r1122535670 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/FetchManagerMetrics.java: ## @@ -0,0 +1,212 @@ +/* + * Licensed to the Apache Software Foundation

[GitHub] [kafka] kirktrue commented on a diff in pull request #13301: KAFKA-14758: Extract inner classes from Fetcher for reuse in refactoring

2023-03-01 Thread via GitHub
kirktrue commented on code in PR #13301: URL: https://github.com/apache/kafka/pull/13301#discussion_r1122517751 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CompletedFetch.java: ## @@ -0,0 +1,356 @@ +/* + * Licensed to the Apache Software Foundation

[GitHub] [kafka] mjsax commented on pull request #13264: KAFKA-14491: [12/N] Relax requirement that KTable stores must be TimestampedKVStores

2023-03-01 Thread via GitHub
mjsax commented on PR #13264: URL: https://github.com/apache/kafka/pull/13264#issuecomment-1451176639 There is test failures. Could it be related to this PR? ``` org.apache.kafka.streams.kstream.internals.KTableKTableInnerJoinTest.shouldLogAndMeterSkippedRecordsDueToNullLeftKey

[GitHub] [kafka] mjsax commented on a diff in pull request #13264: KAFKA-14491: [12/N] Relax requirement that KTable stores must be TimestampedKVStores

2023-03-01 Thread via GitHub
mjsax commented on code in PR #13264: URL: https://github.com/apache/kafka/pull/13264#discussion_r1122525554 ## streams/src/main/java/org/apache/kafka/streams/state/internals/KeyValueStoreWrapper.java: ## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation

[GitHub] [kafka] mjsax commented on a diff in pull request #13264: KAFKA-14491: [12/N] Relax requirement that KTable stores must be TimestampedKVStores

2023-03-01 Thread via GitHub
mjsax commented on code in PR #13264: URL: https://github.com/apache/kafka/pull/13264#discussion_r1122527645 ## streams/src/main/java/org/apache/kafka/streams/state/internals/KeyValueStoreWrapper.java: ## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation

[GitHub] [kafka] mjsax commented on a diff in pull request #13264: KAFKA-14491: [12/N] Relax requirement that KTable stores must be TimestampedKVStores

2023-03-01 Thread via GitHub
mjsax commented on code in PR #13264: URL: https://github.com/apache/kafka/pull/13264#discussion_r1122527645 ## streams/src/main/java/org/apache/kafka/streams/state/internals/KeyValueStoreWrapper.java: ## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation

[GitHub] [kafka] mjsax commented on a diff in pull request #13264: KAFKA-14491: [12/N] Relax requirement that KTable stores must be TimestampedKVStores

2023-03-01 Thread via GitHub
mjsax commented on code in PR #13264: URL: https://github.com/apache/kafka/pull/13264#discussion_r1122525901 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/TimestampedKeyValueStoreMaterializer.java: ## @@ -33,7 +32,7 @@ public

[GitHub] [kafka] mjsax commented on a diff in pull request #13264: KAFKA-14491: [12/N] Relax requirement that KTable stores must be TimestampedKVStores

2023-03-01 Thread via GitHub
mjsax commented on code in PR #13264: URL: https://github.com/apache/kafka/pull/13264#discussion_r1122525901 ## streams/src/main/java/org/apache/kafka/streams/kstream/internals/TimestampedKeyValueStoreMaterializer.java: ## @@ -33,7 +32,7 @@ public

[GitHub] [kafka] mjsax commented on a diff in pull request #13264: KAFKA-14491: [12/N] Relax requirement that KTable stores must be TimestampedKVStores

2023-03-01 Thread via GitHub
mjsax commented on code in PR #13264: URL: https://github.com/apache/kafka/pull/13264#discussion_r1122525554 ## streams/src/main/java/org/apache/kafka/streams/state/internals/KeyValueStoreWrapper.java: ## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation

[GitHub] [kafka] guozhangwang commented on pull request #13319: MINOR: Fix flaky tests in DefaultStateUpdaterTest

2023-03-01 Thread via GitHub
guozhangwang commented on PR #13319: URL: https://github.com/apache/kafka/pull/13319#issuecomment-1451145318 Merged to trunk. -- 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

[GitHub] [kafka] guozhangwang merged pull request #13319: MINOR: Fix flaky tests in DefaultStateUpdaterTest

2023-03-01 Thread via GitHub
guozhangwang merged PR #13319: URL: https://github.com/apache/kafka/pull/13319 -- 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:

[GitHub] [kafka] junrao commented on a diff in pull request #13275: KAFKA-14522 Rewrite/Move of RemoteIndexCache to storage module.

2023-03-01 Thread via GitHub
junrao commented on code in PR #13275: URL: https://github.com/apache/kafka/pull/13275#discussion_r1119167627 ## storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteIndexCache.java: ## @@ -0,0 +1,407 @@ +/* + * Licensed to the Apache Software Foundation (ASF)

[GitHub] [kafka] guozhangwang merged pull request #13288: MINOR: fix rerun-tests for unit test

2023-03-01 Thread via GitHub
guozhangwang merged PR #13288: URL: https://github.com/apache/kafka/pull/13288 -- 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:

[GitHub] [kafka] guozhangwang commented on pull request #13288: MINOR: fix rerun-tests for unit test

2023-03-01 Thread via GitHub
guozhangwang commented on PR #13288: URL: https://github.com/apache/kafka/pull/13288#issuecomment-1451098190 Ah thanks @chia7712 , my bad missing that change in the PR. As for the solution I do not have a preference in either side and I agree that both should not involve recompilation.

[GitHub] [kafka] gharris1727 commented on a diff in pull request #13185: KAFKA-14670: (part 1) Wrap Connectors in IsolatedConnector objects

2023-03-01 Thread via GitHub
gharris1727 commented on code in PR #13185: URL: https://github.com/apache/kafka/pull/13185#discussion_r1122473180 ## connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerder.java: ## @@ -243,7 +248,11 @@ public synchronized void

[GitHub] [kafka] gharris1727 commented on a diff in pull request #13185: KAFKA-14670: (part 1) Wrap Connectors in IsolatedConnector objects

2023-03-01 Thread via GitHub
gharris1727 commented on code in PR #13185: URL: https://github.com/apache/kafka/pull/13185#discussion_r1122467555 ## connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerder.java: ## @@ -243,7 +248,11 @@ public synchronized void

[GitHub] [kafka] jolshan commented on a diff in pull request #13231: KAFKA-14402: Update AddPartitionsToTxn protocol to batch and handle verifyOnly requests

2023-03-01 Thread via GitHub
jolshan commented on code in PR #13231: URL: https://github.com/apache/kafka/pull/13231#discussion_r1122411269 ## core/src/main/scala/kafka/server/KafkaApis.scala: ## @@ -2384,68 +2385,116 @@ class KafkaApis(val requestChannel: RequestChannel, if

[GitHub] [kafka] OneCricketeer commented on a diff in pull request #12992: KIP-887: Add ConfigProvider to make use of environment variables

2023-03-01 Thread via GitHub
OneCricketeer commented on code in PR #12992: URL: https://github.com/apache/kafka/pull/12992#discussion_r1122385195 ## clients/src/main/java/org/apache/kafka/common/config/provider/EnvVarConfigProvider.java: ## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software

[GitHub] [kafka] jeffkbkim commented on a diff in pull request #13322: KAFKA-14462; [1/N] Add new server configurations (KIP-848)

2023-03-01 Thread via GitHub
jeffkbkim commented on code in PR #13322: URL: https://github.com/apache/kafka/pull/13322#discussion_r1122374811 ## core/src/main/scala/kafka/server/KafkaConfig.scala: ## @@ -163,6 +163,20 @@ object Defaults { val GroupInitialRebalanceDelayMs = 3000 val GroupMaxSize: Int

[GitHub] [kafka] jolshan merged pull request #13078: KAFKA-13999: Add ProducerCount metrics (KIP-847)

2023-03-01 Thread via GitHub
jolshan merged PR #13078: URL: https://github.com/apache/kafka/pull/13078 -- 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:

[GitHub] [kafka] jolshan commented on pull request #13078: KAFKA-13999: Add ProducerCount metrics (KIP-847)

2023-03-01 Thread via GitHub
jolshan commented on PR #13078: URL: https://github.com/apache/kafka/pull/13078#issuecomment-1450918075 Tests look unrelated. Going to merge. -- 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

[jira] [Resolved] (KAFKA-14451) Make range assignor rack-aware if consumer racks are configured

2023-03-01 Thread Rajini Sivaram (Jira)
[ https://issues.apache.org/jira/browse/KAFKA-14451?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Rajini Sivaram resolved KAFKA-14451. Fix Version/s: 3.5.0 Reviewer: David Jacot Resolution: Fixed > Make range

[GitHub] [kafka] cmccabe commented on pull request #13114: KAFKA-14084: SCRAM support in KRaft.

2023-03-01 Thread via GitHub
cmccabe commented on PR #13114: URL: https://github.com/apache/kafka/pull/13114#issuecomment-1450843896 > I'm confused by this comment. There is a ./metadata/src/test/java/org/apache/kafka/image/ScramImageTest.java in the review. It is the last file in the review. Sorry, my mistake.

[GitHub] [kafka] rajinisivaram merged pull request #12990: KAFKA-14451: Rack-aware consumer partition assignment for RangeAssignor (KIP-881)

2023-03-01 Thread via GitHub
rajinisivaram merged PR #12990: URL: https://github.com/apache/kafka/pull/12990 -- 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:

[GitHub] [kafka] pprovenzano commented on pull request #13114: KAFKA-14084: SCRAM support in KRaft.

2023-03-01 Thread via GitHub
pprovenzano commented on PR #13114: URL: https://github.com/apache/kafka/pull/13114#issuecomment-1450839352 > We need a test like `./metadata/src/test/java/org/apache/kafka/image/ScramImageTesst.java` , similar to the tests for the other Image classes. I'm confused by this comment.

[GitHub] [kafka] rajinisivaram commented on pull request #12990: KAFKA-14451: Rack-aware consumer partition assignment for RangeAssignor (KIP-881)

2023-03-01 Thread via GitHub
rajinisivaram commented on PR #12990: URL: https://github.com/apache/kafka/pull/12990#issuecomment-1450838774 @dajac Thanks for the reviews. Test failures not related (have verified them locally), merging to trunk. -- This is an automated message from the Apache Git Service. To respond

[GitHub] [kafka] Hangleton commented on a diff in pull request #13162: fix: replace an inefficient loop in kafka internals

2023-03-01 Thread via GitHub
Hangleton commented on code in PR #13162: URL: https://github.com/apache/kafka/pull/13162#discussion_r1122285463 ## clients/src/main/java/org/apache/kafka/common/utils/Utils.java: ## @@ -1225,13 +1226,11 @@ public static long tryWriteTo(TransferableChannel destChannel, *

[GitHub] [kafka] Hangleton commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
Hangleton commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1122271424 ## clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitResponse.java: ## @@ -119,43 +128,53 @@ public boolean shouldClientThrottle(short version) {

[GitHub] [kafka] Hangleton commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
Hangleton commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1122265994 ## core/src/test/scala/unit/kafka/server/OffsetCommitRequestTest.scala: ## @@ -0,0 +1,78 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or

[GitHub] [kafka] jsancio commented on pull request #13102: KAFKA-14371: Remove unused clusterId field from quorum-state file

2023-03-01 Thread via GitHub
jsancio commented on PR #13102: URL: https://github.com/apache/kafka/pull/13102#issuecomment-1450774546 > @jsancio @hachikuji Was there a reason to have this field or was it added accidentally? @ijuma I think this is just an artifact that `quorum-state` was implemented before

[GitHub] [kafka] jsancio commented on pull request #13102: KAFKA-14371: Remove unused clusterId field from quorum-state file

2023-03-01 Thread via GitHub
jsancio commented on PR #13102: URL: https://github.com/apache/kafka/pull/13102#issuecomment-1450767005 Thanks for the change and review. I think it is safe to remove the cluster id from `quorum-state`. The cluster id is stored and read from the file `meta.properties` in

[GitHub] [kafka] guozhangwang commented on pull request #13318: [DO NOT MERGE] KAFKA-14533: Re-enable state-updater in SmokeTestDriverIntegrationTest

2023-03-01 Thread via GitHub
guozhangwang commented on PR #13318: URL: https://github.com/apache/kafka/pull/13318#issuecomment-1450748832 Re-starting new checks as `SmokeTestDriverIntegrationTest` did not fail again. -- This is an automated message from the Apache Git Service. To respond to the message, please log

[GitHub] [kafka] Hangleton commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
Hangleton commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1122188628 ## clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitResponse.java: ## @@ -166,20 +187,27 @@ public Builder addPartitions( return

[GitHub] [kafka] Hangleton commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
Hangleton commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1122175488 ## clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitResponse.java: ## @@ -119,43 +130,62 @@ public boolean shouldClientThrottle(short version) {

[GitHub] [kafka] C0urante commented on a diff in pull request #12992: KIP-887: Add ConfigProvider to make use of environment variables

2023-03-01 Thread via GitHub
C0urante commented on code in PR #12992: URL: https://github.com/apache/kafka/pull/12992#discussion_r1122150546 ## clients/src/main/java/org/apache/kafka/common/config/provider/EnvVarConfigProvider.java: ## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation

[GitHub] [kafka] jolshan commented on pull request #13322: KAFKA-14462; [1/N] Add new server configurations (KIP-848)

2023-03-01 Thread via GitHub
jolshan commented on PR #13322: URL: https://github.com/apache/kafka/pull/13322#issuecomment-1450638812 Got it. So the distinction is "publishing" and that is done when the config is not internal. Thanks for clarifying. -- This is an automated message from the Apache Git Service. To

[GitHub] [kafka] dajac commented on pull request #13322: KAFKA-14462; [1/N] Add new server configurations (KIP-848)

2023-03-01 Thread via GitHub
dajac commented on PR #13322: URL: https://github.com/apache/kafka/pull/13322#issuecomment-1450613278 > Just a general question about configs -- do we have any guidelines for adding and removing configs between releases? Just curious about compatibility and typical guidelines around that.

[GitHub] [kafka] jolshan commented on pull request #13322: KAFKA-14462; [1/N] Add new server configurations (KIP-848)

2023-03-01 Thread via GitHub
jolshan commented on PR #13322: URL: https://github.com/apache/kafka/pull/13322#issuecomment-1450610551 Just a general question about configs -- do we have any guidelines for adding and removing configs between releases? Just curious about compatibility and typical guidelines around that.

[GitHub] [kafka] jolshan commented on a diff in pull request #13322: KAFKA-14462; [1/N] Add new server configurations (KIP-848)

2023-03-01 Thread via GitHub
jolshan commented on code in PR #13322: URL: https://github.com/apache/kafka/pull/13322#discussion_r1122123411 ## core/src/main/scala/kafka/server/KafkaConfig.scala: ## @@ -1267,6 +1314,24 @@ object KafkaConfig { .define(GroupInitialRebalanceDelayMsProp, INT,

[GitHub] [kafka] jolshan commented on a diff in pull request #13322: KAFKA-14462; [1/N] Add new server configurations (KIP-848)

2023-03-01 Thread via GitHub
jolshan commented on code in PR #13322: URL: https://github.com/apache/kafka/pull/13322#discussion_r1122121405 ## core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala: ## @@ -976,6 +976,20 @@ class KafkaConfigTest { case

[GitHub] [kafka] jolshan commented on a diff in pull request #13322: KAFKA-14462; [1/N] Add new server configurations (KIP-848)

2023-03-01 Thread via GitHub
jolshan commented on code in PR #13322: URL: https://github.com/apache/kafka/pull/13322#discussion_r1122120725 ## core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala: ## @@ -976,6 +976,20 @@ class KafkaConfigTest { case

[GitHub] [kafka] jolshan commented on a diff in pull request #13322: KAFKA-14462; [1/N] Add new server configurations (KIP-848)

2023-03-01 Thread via GitHub
jolshan commented on code in PR #13322: URL: https://github.com/apache/kafka/pull/13322#discussion_r1122119145 ## core/src/main/scala/kafka/server/KafkaConfig.scala: ## @@ -1267,6 +1314,24 @@ object KafkaConfig { .define(GroupInitialRebalanceDelayMsProp, INT,

[GitHub] [kafka] jolshan commented on pull request #13078: KAFKA-13999: Add ProducerCount metrics (KIP-847)

2023-03-01 Thread via GitHub
jolshan commented on PR #13078: URL: https://github.com/apache/kafka/pull/13078#issuecomment-1450588812 Seems like there was a build falilure related to `rat` I restarted the build. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [kafka] gharris1727 commented on a diff in pull request #13182: KAFKA-14649: Isolate failures during plugin path scanning to single plugin classes

2023-03-01 Thread via GitHub
gharris1727 commented on code in PR #13182: URL: https://github.com/apache/kafka/pull/13182#discussion_r1122112042 ## connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java: ## @@ -419,7 +423,14 @@ private Collection>

[GitHub] [kafka] gharris1727 commented on a diff in pull request #13182: KAFKA-14649: Isolate failures during plugin path scanning to single plugin classes

2023-03-01 Thread via GitHub
gharris1727 commented on code in PR #13182: URL: https://github.com/apache/kafka/pull/13182#discussion_r1122110705 ## connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java: ## @@ -439,6 +457,22 @@ public static String

[GitHub] [kafka] gharris1727 commented on a diff in pull request #13182: KAFKA-14649: Isolate failures during plugin path scanning to single plugin classes

2023-03-01 Thread via GitHub
gharris1727 commented on code in PR #13182: URL: https://github.com/apache/kafka/pull/13182#discussion_r1122107145 ## connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java: ## @@ -111,20 +115,62 @@ public enum TestPlugin { /**

[GitHub] [kafka] guozhangwang commented on pull request #13319: MINOR: Fix flaky tests in DefaultStateUpdaterTest

2023-03-01 Thread via GitHub
guozhangwang commented on PR #13319: URL: https://github.com/apache/kafka/pull/13319#issuecomment-1450565316 I checked the latest failure on `shouldPauseStandbyTaskAndNotTransit..` and found out that it's because of the test condition itself is timing dependent. What it wants to verify is

[GitHub] [kafka] Hangleton commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
Hangleton commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1122096890 ## clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitResponse.java: ## @@ -149,13 +161,15 @@ public Builder addPartition( return this;

[GitHub] [kafka] guozhangwang commented on pull request #13319: MINOR: Fix flaky tests in DefaultStateUpdaterTest

2023-03-01 Thread via GitHub
guozhangwang commented on PR #13319: URL: https://github.com/apache/kafka/pull/13319#issuecomment-1450512377 Honestly I do not quite understanding how mockito is messing here.. take `shouldPauseActiveTaskAndTransitToUpdateStandby` for example, after task1 (named topologyA) and task2 (named

[GitHub] [kafka] gharris1727 commented on pull request #13315: KAFKA-14767: Fix missing commitId build error after git gc

2023-03-01 Thread via GitHub
gharris1727 commented on PR #13315: URL: https://github.com/apache/kafka/pull/13315#issuecomment-1450445379 It appears that the mustRunAfter directive has fixed the build, and now it's just normal flakey tests. I won't be reverting the fixups and should be ready for review. -- This is

[GitHub] [kafka] satishd commented on a diff in pull request #13304: KAFKA-14726 Move/rewrite of LogReadInfo, LogOffsetSnapshot, LogStartOffsetIncrementReason to storage module

2023-03-01 Thread via GitHub
satishd commented on code in PR #13304: URL: https://github.com/apache/kafka/pull/13304#discussion_r1121995162 ## storage/src/main/java/org/apache/kafka/storage/internals/log/LogReadInfo.java: ## @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under

[GitHub] [kafka] satishd commented on pull request #13304: KAFKA-14726 Move/rewrite of LogReadInfo, LogOffsetSnapshot, LogStartOffsetIncrementReason to storage module

2023-03-01 Thread via GitHub
satishd commented on PR #13304: URL: https://github.com/apache/kafka/pull/13304#issuecomment-1450425363 Thanks @junrao @ijuma for your review. Addressed the review comments with the latest commit. -- This is an automated message from the Apache Git Service. To respond to the

[GitHub] [kafka] Hangleton commented on pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
Hangleton commented on PR #13240: URL: https://github.com/apache/kafka/pull/13240#issuecomment-1450362232 Please note that as mentioned by David above, in the current state, topic ids are not provided by the group coordinator when constructing the response, hence are not returned with the

[GitHub] [kafka] divijvaidya commented on pull request #13284: KAFKA-14718: Fix flaky DedicatedMirrorIntegrationTest

2023-03-01 Thread via GitHub
divijvaidya commented on PR #13284: URL: https://github.com/apache/kafka/pull/13284#issuecomment-1450247692 @C0urante ready for your review. I have increased the timeout to 2 min. and also added the wait for mirror maker to get ready. In a separate JIRA we should talk about how we can make

[jira] [Created] (KAFKA-14773) Make MirrorMaker startup synchronous

2023-03-01 Thread Divij Vaidya (Jira)
Divij Vaidya created KAFKA-14773: Summary: Make MirrorMaker startup synchronous Key: KAFKA-14773 URL: https://issues.apache.org/jira/browse/KAFKA-14773 Project: Kafka Issue Type: Improvement

[GitHub] [kafka] divijvaidya commented on a diff in pull request #13284: KAFKA-14718: Fix flaky DedicatedMirrorIntegrationTest

2023-03-01 Thread via GitHub
divijvaidya commented on code in PR #13284: URL: https://github.com/apache/kafka/pull/13284#discussion_r1121820892 ## connect/mirror/src/test/java/org/apache/kafka/connect/mirror/DedicatedMirrorIntegrationTest.java: ## @@ -14,14 +14,12 @@ * See the License for the specific

[GitHub] [kafka] dpcollins-google commented on a diff in pull request #13162: fix: replace an inefficient loop in kafka internals

2023-03-01 Thread via GitHub
dpcollins-google commented on code in PR #13162: URL: https://github.com/apache/kafka/pull/13162#discussion_r1121810375 ## clients/src/main/java/org/apache/kafka/common/utils/Utils.java: ## @@ -1225,13 +1226,11 @@ public static long tryWriteTo(TransferableChannel destChannel,

[jira] [Created] (KAFKA-14772) Add ConsumerGroupHeartbeat API to AuthorizerIntegrationTest

2023-03-01 Thread David Jacot (Jira)
David Jacot created KAFKA-14772: --- Summary: Add ConsumerGroupHeartbeat API to AuthorizerIntegrationTest Key: KAFKA-14772 URL: https://issues.apache.org/jira/browse/KAFKA-14772 Project: Kafka

[GitHub] [kafka] satishd commented on pull request #13067: KAFKA-14524: Rewrite KafkaMetricsGroup in Java

2023-03-01 Thread via GitHub
satishd commented on PR #13067: URL: https://github.com/apache/kafka/pull/13067#issuecomment-1450196726 > @satishd can you help review this? @ijuma Sure, I will have cycles after this week. I will review it by next Monday. -- This is an automated message from the

[GitHub] [kafka] dajac opened a new pull request, #13322: KAFKA-14462; [1/N] Add new server configurations (KIP-848)

2023-03-01 Thread via GitHub
dajac opened a new pull request, #13322: URL: https://github.com/apache/kafka/pull/13322 This patch adds the new server configurations introduced in KIP-848. All of them are kept as internal configurations for now. We will make them public when the KIP is ready. It also adds an internal

[GitHub] [kafka] rajinisivaram commented on pull request #12990: KAFKA-14451: Rack-aware consumer partition assignment for RangeAssignor (KIP-881)

2023-03-01 Thread via GitHub
rajinisivaram commented on PR #12990: URL: https://github.com/apache/kafka/pull/12990#issuecomment-1450159413 @dajac Thanks for the reviews, I have addressed your comments. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and

[GitHub] [kafka] rajinisivaram commented on a diff in pull request #12990: KAFKA-14451: Rack-aware consumer partition assignment for RangeAssignor (KIP-881)

2023-03-01 Thread via GitHub
rajinisivaram commented on code in PR #12990: URL: https://github.com/apache/kafka/pull/12990#discussion_r1121733442 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -1951,20 +1954,47 @@ class PlaintextConsumerTest extends BaseConsumerTest { }

[GitHub] [kafka] rajinisivaram commented on a diff in pull request #12990: KAFKA-14451: Rack-aware consumer partition assignment for RangeAssignor (KIP-881)

2023-03-01 Thread via GitHub
rajinisivaram commented on code in PR #12990: URL: https://github.com/apache/kafka/pull/12990#discussion_r1121729089 ## clients/src/main/java/org/apache/kafka/clients/consumer/RangeAssignor.java: ## @@ -74,45 +105,194 @@ public String name() { private Map>

[GitHub] [kafka] rajinisivaram commented on a diff in pull request #12990: KAFKA-14451: Rack-aware consumer partition assignment for RangeAssignor (KIP-881)

2023-03-01 Thread via GitHub
rajinisivaram commented on code in PR #12990: URL: https://github.com/apache/kafka/pull/12990#discussion_r1121728688 ## clients/src/main/java/org/apache/kafka/clients/consumer/RangeAssignor.java: ## @@ -74,45 +105,194 @@ public String name() { private Map>

[GitHub] [kafka] Hangleton commented on pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
Hangleton commented on PR #13240: URL: https://github.com/apache/kafka/pull/13240#issuecomment-1450155072 Thanks David, included all your comments in the PR. I now work on: - How to limit the highest version of the OffsetCommit API to 8 in the admin client; - Integration test

[GitHub] [kafka] Hangleton commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
Hangleton commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121724366 ## clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitResponse.java: ## @@ -50,14 +52,21 @@ public class OffsetCommitResponse extends

[GitHub] [kafka] chia7712 commented on pull request #13285: KAFKA-13874 Avoid synchronization in SocketServer metrics

2023-03-01 Thread via GitHub
chia7712 commented on PR #13285: URL: https://github.com/apache/kafka/pull/13285#issuecomment-1450145297 @divijvaidya thanks for great feedback! > we have also have to ensure that deadlock doesn't occur when trying to acquire SocketServer lock and Processors lock. that is

[GitHub] [kafka] divijvaidya commented on pull request #13284: KAFKA-14718: Make MirrorMaker startup synchronously depend on connector start

2023-03-01 Thread via GitHub
divijvaidya commented on PR #13284: URL: https://github.com/apache/kafka/pull/13284#issuecomment-1450105039 > This is less concerning because the MirrorMaker class isn't part of public API; It's not technically part of public API but it has a `main()` method which is executed by the

[GitHub] [kafka] divijvaidya commented on pull request #13285: KAFKA-13874 Avoid synchronization in SocketServer metrics

2023-03-01 Thread via GitHub
divijvaidya commented on PR #13285: URL: https://github.com/apache/kafka/pull/13285#issuecomment-1450075461 I am afraid the latest approach will also not work. This is because there is a possibility that `ArrayBuffer` is internally performing size expansion while we read at

[jira] [Comment Edited] (KAFKA-14771) Include current thread ids in ConcurrentModificationException message

2023-03-01 Thread Pierangelo Di Pilato (Jira)
[ https://issues.apache.org/jira/browse/KAFKA-14771?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17695040#comment-17695040 ] Pierangelo Di Pilato edited comment on KAFKA-14771 at 3/1/23 12:22 PM:

[jira] [Commented] (KAFKA-14771) Include current thread ids in ConcurrentModificationException message

2023-03-01 Thread Pierangelo Di Pilato (Jira)
[ https://issues.apache.org/jira/browse/KAFKA-14771?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17695040#comment-17695040 ] Pierangelo Di Pilato commented on KAFKA-14771: -- [~lukech] what do you think about this

[GitHub] [kafka] showuon commented on pull request #13319: MINOR: Fix flaky tests in DefaultStateUpdaterTest

2023-03-01 Thread via GitHub
showuon commented on PR #13319: URL: https://github.com/apache/kafka/pull/13319#issuecomment-1450060935 I can see the test result is better, but there are still failed tests in `DefaultStateUpdaterTest` ``` Build / JDK 17 and Scala 2.13 /

[jira] [Commented] (KAFKA-14771) Include current thread ids in ConcurrentModificationException message

2023-03-01 Thread Pierangelo Di Pilato (Jira)
[ https://issues.apache.org/jira/browse/KAFKA-14771?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17695039#comment-17695039 ] Pierangelo Di Pilato commented on KAFKA-14771: -- I can provide a patch if this improvement

[jira] [Created] (KAFKA-14771) Include current thread ids in ConcurrentModificationException message

2023-03-01 Thread Pierangelo Di Pilato (Jira)
Pierangelo Di Pilato created KAFKA-14771: Summary: Include current thread ids in ConcurrentModificationException message Key: KAFKA-14771 URL: https://issues.apache.org/jira/browse/KAFKA-14771

[GitHub] [kafka] dajac commented on pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on PR #13240: URL: https://github.com/apache/kafka/pull/13240#issuecomment-1449936916 Sorry, used the wrong button... -- 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

[GitHub] [kafka] dajac closed pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac closed pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9 URL: https://github.com/apache/kafka/pull/13240 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the

[GitHub] [kafka] dajac commented on pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on PR #13240: URL: https://github.com/apache/kafka/pull/13240#issuecomment-1449935049 > Hello David, thanks for the fast review. Apologies for being slow, I hadn't finished the previous revision. Will include your comments. Working on it right now. Thanks! No

[GitHub] [kafka] Hangleton commented on pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
Hangleton commented on PR #13240: URL: https://github.com/apache/kafka/pull/13240#issuecomment-1449920535 Hello David, thanks for the fast review. Apologies for being slow, I hadn't finished the previous revision. Will include your comments. Working on it right now. Thanks! -- This is

[jira] [Created] (KAFKA-14770) Allow dynamic keystore update for brokers if string representation of DN matches even if canonical DNs don't match

2023-03-01 Thread Rajini Sivaram (Jira)
Rajini Sivaram created KAFKA-14770: -- Summary: Allow dynamic keystore update for brokers if string representation of DN matches even if canonical DNs don't match Key: KAFKA-14770 URL:

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121510354 ## checkstyle/suppressions.xml: ## @@ -93,7 +93,7 @@

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121506569 ## clients/src/test/java/org/apache/kafka/common/requests/OffsetCommitResponseTest.java: ## @@ -17,37 +17,64 @@ package org.apache.kafka.common.requests; import

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121501980 ## clients/src/test/java/org/apache/kafka/common/requests/TxnOffsetCommitRequestTest.java: ## @@ -80,10 +81,10 @@ public void setUp() { ); } -@Test

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121499577 ## clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitResponse.java: ## @@ -188,12 +188,73 @@ public Builder merge( }

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121496085 ## clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java: ## @@ -101,18 +103,18 @@ public static List getErrorResponseTopics(

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121494379 ## clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java: ## @@ -57,8 +55,22 @@ public Builder(OffsetCommitRequestData data) { public

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121489116 ## core/src/main/scala/kafka/server/KafkaApis.scala: ## @@ -425,35 +425,63 @@ class KafkaApis(val requestChannel: RequestChannel,

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121488495 ## core/src/main/scala/kafka/server/KafkaApis.scala: ## @@ -425,35 +425,63 @@ class KafkaApis(val requestChannel: RequestChannel,

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121487731 ## core/src/main/scala/kafka/server/KafkaApis.scala: ## @@ -425,35 +425,63 @@ class KafkaApis(val requestChannel: RequestChannel,

[GitHub] [kafka] dajac commented on a diff in pull request #13240: KAFKA-14690: Add topic IDs to OffsetCommit API and propagate for request version >= 9

2023-03-01 Thread via GitHub
dajac commented on code in PR #13240: URL: https://github.com/apache/kafka/pull/13240#discussion_r1121486904 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -1352,8 +1361,42 @@ public void handle(OffsetCommitResponse

[GitHub] [kafka] dajac commented on a diff in pull request #12990: KAFKA-14451: Rack-aware consumer partition assignment for RangeAssignor (KIP-881)

2023-03-01 Thread via GitHub
dajac commented on code in PR #12990: URL: https://github.com/apache/kafka/pull/12990#discussion_r1121387449 ## clients/src/main/java/org/apache/kafka/clients/consumer/RangeAssignor.java: ## @@ -74,45 +105,194 @@ public String name() { private Map> consumersPerTopic(Map

  1   2   >