Re: [PR] KAFKA-16481: Fixing flaky test kafka.server.ReplicaManagerTest#testRemoteLogReaderMetrics [kafka]

2024-04-07 Thread via GitHub


vamossagar12 commented on PR #15677:
URL: https://github.com/apache/kafka/pull/15677#issuecomment-2041849012

   Thanks for the review @showuon and thanks for the input. I agree to your 
proposal, we can first merge this and then I will look at the adding more 
resilience on the test for this case.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-15746: KRaft support in ControllerMutationQuotaTest [kafka]

2024-04-07 Thread via GitHub


github-actions[bot] commented on PR #15038:
URL: https://github.com/apache/kafka/pull/15038#issuecomment-2041800868

   This PR is being marked as stale since it has not had any activity in 90 
days. If you would like to keep this PR alive, please ask a committer for 
review. If the PR has  merge conflicts, please update it with the latest from 
trunk (or appropriate release branch)  If this PR is no longer valid or 
desired, please feel free to close it. If no activity occurs in the next 30 
days, it will be automatically closed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: Add retry mechanism to EOS example [kafka]

2024-04-07 Thread via GitHub


gaoran10 commented on PR #15561:
URL: https://github.com/apache/kafka/pull/15561#issuecomment-2041793310

   Sorry for the late response, great work!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[PR] KAFKA-16481: Fixing flaky test kafka.server.ReplicaManagerTest#testRemoteLogReaderMetrics [kafka]

2024-04-07 Thread via GitHub


vamossagar12 opened a new pull request, #15677:
URL: https://github.com/apache/kafka/pull/15677

   Noticed in the build: 
https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-15621/32/tests/
   
   There are a lot of NPEs of the form:
   
   ```
   [2024-04-05 21:08:58,617] WARN [RemoteLogManager=0 
partition=C_F_5O-eSYWmgQoron0dOQ:test-topic-0] Current task for topic-partition 
C_F_5O-eSYWmgQoron0dOQ:test-topic-0 received error but it will be scheduled. 
Reason: Cannot invoke "org.apache.kafka.common.TopicPartition.topic()" because 
the return value of "kafka.log.UnifiedLog.topicPartition()" is null 
(kafka.log.remote.RemoteLogManager$RLMTask:829)
   [2024-04-05 21:08:58,622] ERROR Error occurred while reading the remote data 
for test-topic-0 (kafka.log.remote.RemoteLogReader:71)
   java.lang.NullPointerException: Cannot invoke "scala.Option.isDefined()" 
because "leaderEpochCache" is null
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1339)
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1325)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:62)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:31)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
at 
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at 
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1583)
   [2024-04-05 21:08:58,623] ERROR Error occurred while reading the remote data 
for test-topic-0 (kafka.log.remote.RemoteLogReader:71)
   java.lang.NullPointerException: Cannot invoke "scala.Option.isDefined()" 
because "leaderEpochCache" is null
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1339)
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1325)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:62)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:31)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
at 
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at 
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1583)
   [2024-04-05 21:08:58,623] ERROR Error occurred while reading the remote data 
for test-topic-0 (kafka.log.remote.RemoteLogReader:71)
   java.lang.NullPointerException: Cannot invoke "scala.Option.isDefined()" 
because "leaderEpochCache" is null
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1339)
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1325)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:62)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:31)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
at 
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at 
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1583)
   [2024-04-05 21:08:58,624] ERROR Error occurred while reading the remote data 
for test-topic-0 (kafka.log.remote.RemoteLogReader:71)
   java.lang.NullPointerException: Cannot invoke "scala.Option.isDefined()" 
because "leaderEpochCache" is null
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1339)
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1325)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:62)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:31)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
at 
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at 
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1583)
   [2024-04-05 21:08:58,625] ERROR Error occurred while reading the remote data 
for test-topic-0 (kafka.log.remote.RemoteLogReader:71)
   java.lang.NullPointerException: Cannot invoke "scala.Option.isDefined()" 
because "leaderEpochCache" is null
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1339)
at kafka.log.remote.RemoteLogManager.read(RemoteLogManager.java:1325)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:62)
at kafka.log.remote.RemoteLogReader.call(RemoteLogReader.java:31)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
at 
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
 

[PR] KAFKA-16482: Eliminate the IDE warnings of accepting ClusterConfig in BeforeEach [kafka]

2024-04-07 Thread via GitHub


KevinZTW opened a new pull request, #15676:
URL: https://github.com/apache/kafka/pull/15676

   
   
   *More detailed description of your change,
   if necessary. The PR title and PR message become
   the squashed commit message, so use a separate
   comment to ping reviewers.*
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[jira] [Assigned] (KAFKA-16049) Can you please share tutorial how to run Latest Kafka (3.6.0) with SASL_SSL

2024-04-07 Thread Kelvin Lui (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-16049?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Kelvin Lui reassigned KAFKA-16049:
--

Assignee: (was: Kelvin Lui)

> Can you please share tutorial how to run Latest Kafka (3.6.0) with SASL_SSL
> ---
>
> Key: KAFKA-16049
> URL: https://issues.apache.org/jira/browse/KAFKA-16049
> Project: Kafka
>  Issue Type: Task
>  Components: config
>Affects Versions: 3.6.0
>Reporter: Petr Kostroun
>Priority: Major
>  Labels: newbie
>
> Can you please share tutorial how to use SASL_SSL with Kafka version 3.6.0?
>  
> I use this config for zookeeper.properties:
> authProvider.sasl=org.apache.zookeeper.server.auth.SASLAuthenticationProvider
>  
> I use this config for zookeeper.jaas.config:
> Server
> {        org.apache.zookeeper.server.auth.DigestLoginModule required        
> user_super="adminsecret"        user_admin="kafka123"; }
> ;
>  
> I use this config for server.properties:
>  
> sasl.enabled.mechanisms=SCRAM-SHA-256
> listeners=SASL_SSL://localhost:9092
> advertised.listeners=SASL_SSL://localhost:9092
> sasl.mechanism.inter.broker.protocol=SCRAM-SHA-256
> security.inter.broker.protocol=SASL_SSL
> ssl.keystore.location=C:/apps/certs/keystore.jks
> ssl.keystore.password=sepultura1
> ssl.key.password=sepultura1
> ssl.truststore.location=C:/apps/certs/truststore.jks
> ssl.truststore.password=sepultura1
> ssl.client.auth=required
> ssl.endpoint.identification.algorithm=
>  
> I use this as kafkaserver.jaas.config:
> KafkaServer
> {    org.apache.kafka.common.security.scram.ScramLoginModule required    
> username="user"    password="sepultura1"; }
> ;
> Client
> {     org.apache.zookeeper.server.auth.DigestLoginModule required     
> username="admin"     password="kafka123"; }
> ;
>  
> But in server log I see error:
>  
> [2023-12-25 19:36:58,233] INFO [Controller id=0, targetBrokerId=0] Node 0 
> disconnected. (org.apache.kafka.clients.NetworkClient)
> [2023-12-25 19:36:58,244] ERROR [Controller id=0, targetBrokerId=0] 
> Connection to node 0 (localhost/127.0.0.1:9092) failed authentication due to: 
> Authentication failed during authentication due to invalid credentials with 
> SASL mechanism SCRAM-SHA-256 (org.apache.kafka.clients.NetworkClient)
>  
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Assigned] (KAFKA-16049) Can you please share tutorial how to run Latest Kafka (3.6.0) with SASL_SSL

2024-04-07 Thread Kelvin Lui (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-16049?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Kelvin Lui reassigned KAFKA-16049:
--

Assignee: Kelvin Lui

> Can you please share tutorial how to run Latest Kafka (3.6.0) with SASL_SSL
> ---
>
> Key: KAFKA-16049
> URL: https://issues.apache.org/jira/browse/KAFKA-16049
> Project: Kafka
>  Issue Type: Task
>  Components: config
>Affects Versions: 3.6.0
>Reporter: Petr Kostroun
>Assignee: Kelvin Lui
>Priority: Major
>  Labels: newbie
>
> Can you please share tutorial how to use SASL_SSL with Kafka version 3.6.0?
>  
> I use this config for zookeeper.properties:
> authProvider.sasl=org.apache.zookeeper.server.auth.SASLAuthenticationProvider
>  
> I use this config for zookeeper.jaas.config:
> Server
> {        org.apache.zookeeper.server.auth.DigestLoginModule required        
> user_super="adminsecret"        user_admin="kafka123"; }
> ;
>  
> I use this config for server.properties:
>  
> sasl.enabled.mechanisms=SCRAM-SHA-256
> listeners=SASL_SSL://localhost:9092
> advertised.listeners=SASL_SSL://localhost:9092
> sasl.mechanism.inter.broker.protocol=SCRAM-SHA-256
> security.inter.broker.protocol=SASL_SSL
> ssl.keystore.location=C:/apps/certs/keystore.jks
> ssl.keystore.password=sepultura1
> ssl.key.password=sepultura1
> ssl.truststore.location=C:/apps/certs/truststore.jks
> ssl.truststore.password=sepultura1
> ssl.client.auth=required
> ssl.endpoint.identification.algorithm=
>  
> I use this as kafkaserver.jaas.config:
> KafkaServer
> {    org.apache.kafka.common.security.scram.ScramLoginModule required    
> username="user"    password="sepultura1"; }
> ;
> Client
> {     org.apache.zookeeper.server.auth.DigestLoginModule required     
> username="admin"     password="kafka123"; }
> ;
>  
> But in server log I see error:
>  
> [2023-12-25 19:36:58,233] INFO [Controller id=0, targetBrokerId=0] Node 0 
> disconnected. (org.apache.kafka.clients.NetworkClient)
> [2023-12-25 19:36:58,244] ERROR [Controller id=0, targetBrokerId=0] 
> Connection to node 0 (localhost/127.0.0.1:9092) failed authentication due to: 
> Authentication failed during authentication due to invalid credentials with 
> SASL mechanism SCRAM-SHA-256 (org.apache.kafka.clients.NetworkClient)
>  
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [PR] MINOR: Fix usage of none in javadoc [kafka]

2024-04-07 Thread via GitHub


chia7712 merged PR #15674:
URL: https://github.com/apache/kafka/pull/15674


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[jira] [Updated] (KAFKA-16482) Eliminate the IDE warnings of accepting ClusterConfig in BeforeEach

2024-04-07 Thread Chia-Ping Tsai (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-16482?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chia-Ping Tsai updated KAFKA-16482:
---
Summary: Eliminate the IDE warnings of accepting ClusterConfig in 
BeforeEach  (was: Avoid changing ClusterConfig in `BeforeEach` phase)

> Eliminate the IDE warnings of accepting ClusterConfig in BeforeEach
> ---
>
> Key: KAFKA-16482
> URL: https://issues.apache.org/jira/browse/KAFKA-16482
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Cheng-Kai, Zhang
>Priority: Major
>
> IDE does not like the code style, and we can leverage `ClusterConfigProperty` 
> to eliminate the false error from IDE
>  
> [https://github.com/apache/kafka/blob/trunk/core/src/test/scala/integration/kafka/coordinator/transaction/ProducerIdsIntegrationTest.scala#L42]
> [https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java#L75]
>  
> https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java#L68



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (KAFKA-16482) Avoid changing ClusterConfig in `BeforeEach` phase

2024-04-07 Thread Chia-Ping Tsai (Jira)


[ 
https://issues.apache.org/jira/browse/KAFKA-16482?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17834745#comment-17834745
 ] 

Chia-Ping Tsai commented on KAFKA-16482:


BTW, we can eliminate the IDE error by updating configs of non-started cluster 
instance. for example:  LeaderElectionCommandTest
{color:#b3ae60}@BeforeEach
{color}{color:#cf8e6d}void {color}{color:#56a8f5}setup{color}() {
TestUtils.verifyNoUnexpectedThreads({color:#6aab73}"@BeforeEach"{color});
{color:#c77dbb}cluster{color}.config().serverProperties().put({color:#6aab73}"auto.leader.rebalance.enable"{color},
 {color:#6aab73}"false"{color});
{color:#c77dbb}cluster{color}.config().serverProperties().put({color:#6aab73}"controlled.shutdown.enable"{color},
 {color:#6aab73}"true"{color});
{color:#c77dbb}cluster{color}.config().serverProperties().put({color:#6aab73}"controlled.shutdown.max.retries"{color},
 {color:#6aab73}"1"{color});
{color:#c77dbb}cluster{color}.config().serverProperties().put({color:#6aab73}"controlled.shutdown.retry.backoff.ms"{color},
 {color:#6aab73}"1000"{color});
{color:#c77dbb}cluster{color}.config().serverProperties().put({color:#6aab73}"offsets.topic.replication.factor"{color},
 {color:#6aab73}"2"{color});
}
 

> Avoid changing ClusterConfig in `BeforeEach` phase
> --
>
> Key: KAFKA-16482
> URL: https://issues.apache.org/jira/browse/KAFKA-16482
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Cheng-Kai, Zhang
>Priority: Major
>
> IDE does not like the code style, and we can leverage `ClusterConfigProperty` 
> to eliminate the false error from IDE
>  
> [https://github.com/apache/kafka/blob/trunk/core/src/test/scala/integration/kafka/coordinator/transaction/ProducerIdsIntegrationTest.scala#L42]
> [https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java#L75]
>  
> https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java#L68



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-07 Thread via GitHub


chia7712 commented on PR #15621:
URL: https://github.com/apache/kafka/pull/15621#issuecomment-2041650425

   > There are quite a few test failures on 
[kafka.server.ListOffsetsRequestTest.testResponseIncludesLeaderEpoch()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15621/35/testReport/junit/kafka.server/ListOffsetsRequestTest/Build___JDK_21_and_Scala_2_13___testResponseIncludesLeaderEpoch__/).
   
   yep, I have fixed it on my local. will update PR later. thanks for the 
reminder :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16310 ListOffsets doesn't report the offset with maxTimestamp a… [kafka]

2024-04-07 Thread via GitHub


junrao commented on PR #15621:
URL: https://github.com/apache/kafka/pull/15621#issuecomment-2041649750

   @chia7712 : There are quite a few test failures on 
[kafka.server.ListOffsetsRequestTest.testResponseIncludesLeaderEpoch()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15621/35/testReport/junit/kafka.server/ListOffsetsRequestTest/Build___JDK_21_and_Scala_2_13___testResponseIncludesLeaderEpoch__/).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16477: Detect thread leaked client-metrics-reaper in tests [kafka]

2024-04-07 Thread via GitHub


chia7712 commented on code in PR #15668:
URL: https://github.com/apache/kafka/pull/15668#discussion_r1555109656


##
server/src/main/java/org/apache/kafka/server/ClientMetricsManager.java:
##
@@ -112,7 +113,7 @@ public ClientMetricsManager(ClientMetricsReceiverPlugin 
receiverPlugin, int clie
 this.subscriptionMap = new ConcurrentHashMap<>();
 this.subscriptionUpdateVersion = new AtomicInteger(0);
 this.clientInstanceCache = new SynchronizedCache<>(new 
LRUCache<>(CACHE_MAX_SIZE));
-this.expirationTimer = new SystemTimerReaper("client-metrics-reaper", 
new SystemTimer("client-metrics"));
+this.expirationTimer = new 
SystemTimerReaper(CLIENT_METRICS_REAPER_THREAD_NAME, new 
SystemTimer("client-metrics"));

Review Comment:
   I just notice this response. Could you add `executor-`  to 
`unexpectedThreadNames` to trigger QA again?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16123: KStreamKStreamJoinProcessor does not drop late records. [kafka]

2024-04-07 Thread via GitHub


florin-akermann commented on PR #15189:
URL: https://github.com/apache/kafka/pull/15189#issuecomment-2041579829

   Hi @mjsax 
   It's now rebased.
   Plus, I addressed your reply below.
   
   > If find this test a little bit hard to follow...
   > 
   > We start to open a window `[5;15]` with the first input record. This 
window would close at `31` so why do we dump to `40`, and test dropping with 
`ts=24` -- both are totally unrelated to the left input record's window.
   > 
   > Also, it would be good to test that we can join out-of-order record 
successfully as long as the window is open, and that we don't drop re-mature 
before we hit the close time, thus I would suggest something like this:
   > 
   > ```
   > // prepare
   > - left input at 15 -> open window
   > // positive test
   > - bump time to 30 (different key) -> window still open
   > - right input at 5 -> joins (no need to test with 4, because 4 won't join 
because of window size anyway...)
   > - right input at 25 -> joins 
   > // negative test
   > - bump time to 31 (different key) -> window closes
   > - right input at 5 -> dropped (recorded with metric)
   > - right input at 25 -> does not join any longer; window closed (for this 
case we don't drop and don't record metric)
   > // test sharp lower drop bound
   >  - right input at 6 -> does not join any longer; window closed (for this 
case we don't drop though and don't record metric)
   > // cont. with additional sanity check:
   > - left input at 16 -> joins with both right input at 6 and right input at 
25 -- to verify both records did not get dropped
   > ```
   
   I would argue that the second 'right input at 25' still should join / emit a 
record because the other window is still active and 'ts=15' within its bounds. 
Other than that the expected behavior of the test cases in 
KStreamKStreamWindowCloseTest is in line with what you suggested.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16436: Online upgrade triggering and group type conversion [kafka]

2024-04-07 Thread via GitHub


dongnuo123 commented on code in PR #15662:
URL: https://github.com/apache/kafka/pull/15662#discussion_r1555022535


##
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/classic/ClassicGroup.java:
##
@@ -1300,6 +1341,68 @@ public Map groupAssignment() {
 ));
 }
 
+/**
+ * Convert the current classic group to a consumer group.
+ * Add the records for the conversion.
+ *
+ * @param consumerGroup The converted consumer group.
+ * @param records   The list to which the new records are added.
+ *
+ * @throws GroupIdNotFoundException if any of the group's member doesn't 
support the consumer protocol.
+ */
+public void convertToConsumerGroup(
+ConsumerGroup consumerGroup,
+List records,
+TopicsImage topicsImage
+) throws GroupIdNotFoundException {
+consumerGroup.setGroupEpoch(generationId);
+consumerGroup.setTargetAssignmentEpoch(generationId);
+
+records.add(RecordHelpers.newGroupEpochRecord(groupId(), 
generationId));
+// SubscriptionMetadata will be computed in the following 
consumerGroupHeartbeat
+
records.add(RecordHelpers.newGroupSubscriptionMetadataRecord(groupId(), 
Collections.emptyMap()));
+records.add(RecordHelpers.newTargetAssignmentEpochRecord(groupId(), 
generationId));
+
+members.forEach((memberId, member) -> {
+ConsumerPartitionAssignor.Assignment assignment = 
ConsumerProtocol.deserializeAssignment(ByteBuffer.wrap(member.assignment()));
+Map> partitions = 
topicPartitionMapFromList(assignment.partitions(), topicsImage);
+ConsumerPartitionAssignor.Subscription subscription = 
ConsumerProtocol.deserializeSubscription(ByteBuffer.wrap(member.metadata()));

Review Comment:
   >keep a reference to those in the member's state
   
   It was planned to be added in downgrade conversion but let me bring them 
into this pr because we will need it in implementing the apis



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16436: Online upgrade triggering and group type conversion [kafka]

2024-04-07 Thread via GitHub


dongnuo123 commented on code in PR #15662:
URL: https://github.com/apache/kafka/pull/15662#discussion_r1555022109


##
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/classic/ClassicGroup.java:
##
@@ -1300,6 +1341,68 @@ public Map groupAssignment() {
 ));
 }
 
+/**
+ * Convert the current classic group to a consumer group.
+ * Add the records for the conversion.
+ *
+ * @param consumerGroup The converted consumer group.
+ * @param records   The list to which the new records are added.
+ *
+ * @throws GroupIdNotFoundException if any of the group's member doesn't 
support the consumer protocol.
+ */
+public void convertToConsumerGroup(
+ConsumerGroup consumerGroup,
+List records,
+TopicsImage topicsImage
+) throws GroupIdNotFoundException {

Review Comment:
   `Group epoch` and `Target assignment epoch` only apply to the group. Should 
we add the 5 records for each member?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16436: Online upgrade triggering and group type conversion [kafka]

2024-04-07 Thread via GitHub


dongnuo123 commented on code in PR #15662:
URL: https://github.com/apache/kafka/pull/15662#discussion_r1555021412


##
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/classic/ClassicGroup.java:
##
@@ -1244,6 +1267,24 @@ public boolean completeSyncFuture(
 return false;
 }
 
+/**
+ * Complete all the awaiting sync future with the give error.
+ *
+ * @param error  the error to complete the future with.
+ */
+public void completeAllSyncFutures(
+Errors error
+) {
+members.forEach((__, member) -> completeSyncFuture(
+member,
+new SyncGroupResponseData()
+.setProtocolName(protocolName.orElse(null))
+.setProtocolType(protocolType.orElse(null))
+.setAssignment(member.assignment())

Review Comment:
   We did set them in `resetAndPropagateAssignmentWithError(group, 
Errors.REBALANCE_IN_PROGRESS);` but I don't think they are necessary



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: Add type check to classic group timeout operations [kafka]

2024-04-07 Thread via GitHub


dongnuo123 commented on code in PR #15587:
URL: https://github.com/apache/kafka/pull/15587#discussion_r1555019741


##
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##
@@ -2415,6 +2415,20 @@ private CoordinatorResult 
classicGroupJoinExistingMember(
 return EMPTY_RESULT;
 }
 
+/**
+ * An overload of {@link 
GroupMetadataManager#completeClassicGroupJoin(ClassicGroup)} used as
+ * timeout operation. It additionally looks up the group by the id and 
checks the group type.
+ * completeClassicGroupJoin will only be called if the group is CLASSIC.
+ */
+private CoordinatorResult completeClassicGroupJoin(String 
groupId) {
+if (containsClassicGroup(groupId)) {
+return 
completeClassicGroupJoin(getOrMaybeCreateClassicGroup(groupId, false));

Review Comment:
   Yes I agree. Let me use the try catch



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16297: Race condition while promoting future replica [kafka]

2024-04-07 Thread via GitHub


soarez commented on PR #15557:
URL: https://github.com/apache/kafka/pull/15557#issuecomment-2041535672

   I think I've addressed the issues. The failing tests on the most recent 
build are unrelated. PTAL @showuon 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16477: Detect thread leaked client-metrics-reaper in tests [kafka]

2024-04-07 Thread via GitHub


brandboat commented on code in PR #15668:
URL: https://github.com/apache/kafka/pull/15668#discussion_r1554997766


##
server-common/src/test/java/org/apache/kafka/server/util/timer/TimerTest.java:
##
@@ -77,7 +77,7 @@ public void setup() {
 @AfterEach
 public void teardown() throws Exception {
 timer.close();
-TestUtils.waitForCondition(() -> timer.isExecutorTerminated(), "timer 
excutor not terminated");
+TestUtils.waitForCondition(() -> timer.isTerminated(), "timer executor 
not terminated");

Review Comment:
   God... I definitly need to open the IDE check for this. Many thanks



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16477: Detect thread leaked client-metrics-reaper in tests [kafka]

2024-04-07 Thread via GitHub


chia7712 commented on code in PR #15668:
URL: https://github.com/apache/kafka/pull/15668#discussion_r1554997443


##
server-common/src/test/java/org/apache/kafka/server/util/timer/TimerTest.java:
##
@@ -77,7 +77,7 @@ public void setup() {
 @AfterEach
 public void teardown() throws Exception {
 timer.close();
-TestUtils.waitForCondition(() -> timer.isExecutorTerminated(), "timer 
excutor not terminated");
+TestUtils.waitForCondition(() -> timer.isTerminated(), "timer executor 
not terminated");

Review Comment:
   `timer::isTerminated`



##
server-common/src/test/java/org/apache/kafka/server/util/timer/SystemTimerReaperTest.java:
##
@@ -59,4 +60,13 @@ public void testReaper() throws Exception {
 timer.close();
 }
 }
+
+@Test
+public void testReaperClose() throws Exception {
+Timer timer = Mockito.mock(Timer.class);
+SystemTimerReaper timerReaper = new SystemTimerReaper("reaper", timer);
+timerReaper.close();
+Mockito.verify(timer, Mockito.times(1)).close();
+TestUtils.waitForCondition(() -> timerReaper.isShutdown(), "reaper not 
shutdown");

Review Comment:
   `timerReaper::isShutdown`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[jira] [Assigned] (KAFKA-16484) Support to define per broker/controller property by ClusterConfigProperty

2024-04-07 Thread Chia-Ping Tsai (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-16484?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chia-Ping Tsai reassigned KAFKA-16484:
--

Assignee: Kuan Po Tseng  (was: Chia-Ping Tsai)

> Support to define per broker/controller property by ClusterConfigProperty
> -
>
> Key: KAFKA-16484
> URL: https://issues.apache.org/jira/browse/KAFKA-16484
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Kuan Po Tseng
>Priority: Major
>
> the property set to `ClusterConfigProperty` gets applied to all brokers, and 
> hence we can't have individual props for each broker to test racks.
>  
> It seems to me we can add new field "id" to `ClusterConfigProperty` to 
> declare the property should be applied to specific broker (or controller). 
> the default value is -1 and it should be applied to all nodes.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (KAFKA-16484) Support to define per broker/controller property by ClusterConfigProperty

2024-04-07 Thread Chia-Ping Tsai (Jira)


[ 
https://issues.apache.org/jira/browse/KAFKA-16484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17834692#comment-17834692
 ] 

Chia-Ping Tsai commented on KAFKA-16484:


[~brandboat] I'm not working on this, so please feel free to take over it. 
thanks!

> Support to define per broker/controller property by ClusterConfigProperty
> -
>
> Key: KAFKA-16484
> URL: https://issues.apache.org/jira/browse/KAFKA-16484
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Kuan Po Tseng
>Priority: Major
>
> the property set to `ClusterConfigProperty` gets applied to all brokers, and 
> hence we can't have individual props for each broker to test racks.
>  
> It seems to me we can add new field "id" to `ClusterConfigProperty` to 
> declare the property should be applied to specific broker (or controller). 
> the default value is -1 and it should be applied to all nodes.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Comment Edited] (KAFKA-16484) Support to define per broker/controller property by ClusterConfigProperty

2024-04-07 Thread Kuan Po Tseng (Jira)


[ 
https://issues.apache.org/jira/browse/KAFKA-16484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17834690#comment-17834690
 ] 

Kuan Po Tseng edited comment on KAFKA-16484 at 4/7/24 3:14 PM:
---

Hello [~chia7712], Not sure if you're working on this issue, if not, I'm 
willing to take over this issue, many thanks !


was (Author: brandboat):
Hello [~chia7712], I'm willing to take over this issue, many thanks !

> Support to define per broker/controller property by ClusterConfigProperty
> -
>
> Key: KAFKA-16484
> URL: https://issues.apache.org/jira/browse/KAFKA-16484
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Chia-Ping Tsai
>Priority: Major
>
> the property set to `ClusterConfigProperty` gets applied to all brokers, and 
> hence we can't have individual props for each broker to test racks.
>  
> It seems to me we can add new field "id" to `ClusterConfigProperty` to 
> declare the property should be applied to specific broker (or controller). 
> the default value is -1 and it should be applied to all nodes.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (KAFKA-16484) Support to define per broker/controller property by ClusterConfigProperty

2024-04-07 Thread Kuan Po Tseng (Jira)


[ 
https://issues.apache.org/jira/browse/KAFKA-16484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17834690#comment-17834690
 ] 

Kuan Po Tseng commented on KAFKA-16484:
---

Hello [~chia7712], I'm willing to take over this issue, many thanks !

> Support to define per broker/controller property by ClusterConfigProperty
> -
>
> Key: KAFKA-16484
> URL: https://issues.apache.org/jira/browse/KAFKA-16484
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Chia-Ping Tsai
>Priority: Major
>
> the property set to `ClusterConfigProperty` gets applied to all brokers, and 
> hence we can't have individual props for each broker to test racks.
>  
> It seems to me we can add new field "id" to `ClusterConfigProperty` to 
> declare the property should be applied to specific broker (or controller). 
> the default value is -1 and it should be applied to all nodes.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Created] (KAFKA-16484) Support to define per broker property by ClusterConfigProperty

2024-04-07 Thread Chia-Ping Tsai (Jira)
Chia-Ping Tsai created KAFKA-16484:
--

 Summary: Support to define per broker property by 
ClusterConfigProperty
 Key: KAFKA-16484
 URL: https://issues.apache.org/jira/browse/KAFKA-16484
 Project: Kafka
  Issue Type: Test
Reporter: Chia-Ping Tsai
Assignee: Chia-Ping Tsai


the property set to `ClusterConfigProperty` gets applied to all brokers, and 
hence we can't have individual props for each broker to test racks.

 

It seems to me we can add new field "id" to `ClusterConfigProperty` to declare 
the property should be applied to specific broker (or controller). the default 
value is -1 and it should be applied to all nodes.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Updated] (KAFKA-16484) Support to define per broker/controller property by ClusterConfigProperty

2024-04-07 Thread Chia-Ping Tsai (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-16484?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chia-Ping Tsai updated KAFKA-16484:
---
Summary: Support to define per broker/controller property by 
ClusterConfigProperty  (was: Support to define per broker property by 
ClusterConfigProperty)

> Support to define per broker/controller property by ClusterConfigProperty
> -
>
> Key: KAFKA-16484
> URL: https://issues.apache.org/jira/browse/KAFKA-16484
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Chia-Ping Tsai
>Priority: Major
>
> the property set to `ClusterConfigProperty` gets applied to all brokers, and 
> hence we can't have individual props for each broker to test racks.
>  
> It seems to me we can add new field "id" to `ClusterConfigProperty` to 
> declare the property should be applied to specific broker (or controller). 
> the default value is -1 and it should be applied to all nodes.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [PR] MINOR: enable kraft test for ReassignPartitionsIntegrationTest [kafka]

2024-04-07 Thread via GitHub


FrankYang0529 commented on PR #15675:
URL: https://github.com/apache/kafka/pull/15675#issuecomment-2041482907

   > @FrankYang0529 Could we use `ClusterTestExtensions` to enable `kraft`? It 
seems to me that using the new test framework is the better way. Otherwise, we 
may rewrite it again in the future.
   
   Of course. I will dig into it tomorrow. Thanks for the suggestion.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: enable kraft test for ReassignPartitionsIntegrationTest [kafka]

2024-04-07 Thread via GitHub


chia7712 commented on PR #15675:
URL: https://github.com/apache/kafka/pull/15675#issuecomment-2041482458

   @FrankYang0529 Could we use `ClusterTestExtensions` to enable `kraft`? It 
seems to me that using the new test framework is the better way. Otherwise, we 
may rewrite it again in the future.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[PR] MINOR: enable kraft test for ReassignPartitionsIntegrationTest [kafka]

2024-04-07 Thread via GitHub


FrankYang0529 opened a new pull request, #15675:
URL: https://github.com/apache/kafka/pull/15675

   Enable Kraft test for `ReassignPartitionsIntegrationTest`.
   
   All test cases can pass on my laptop.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-13907: Fix hanging ServerShutdownTest.testCleanShutdownWithKRaftControllerUnavailable [kafka]

2024-04-07 Thread via GitHub


chia7712 commented on PR #12174:
URL: https://github.com/apache/kafka/pull/12174#issuecomment-2041478299

   @soarez Could you please fix the conflicts? thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: Default test name added to core [kafka]

2024-04-07 Thread via GitHub


chia7712 merged PR #15667:
URL: https://github.com/apache/kafka/pull/15667


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: Default test name added to tools [kafka]

2024-04-07 Thread via GitHub


chia7712 merged PR #15666:
URL: https://github.com/apache/kafka/pull/15666


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: Cleanup in MetadataShell [kafka]

2024-04-07 Thread via GitHub


chia7712 commented on code in PR #15672:
URL: https://github.com/apache/kafka/pull/15672#discussion_r1554976395


##
shell/src/main/java/org/apache/kafka/shell/MetadataShell.java:
##
@@ -281,13 +278,12 @@ public static void main(String[] args) throws Exception {
 }
 }
 
-void waitUntilCaughtUp() throws ExecutionException, InterruptedException {
+void waitUntilCaughtUp() throws InterruptedException {
 while (true) {
 if (loader.lastAppliedOffset() > 0) {
 return;
 }
 Thread.sleep(10);
 }
-//snapshotFileReader.caughtUpFuture().get();

Review Comment:
   not sure whether this wait is necessary or a exception?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-16477: Detect thread leaked client-metrics-reaper in tests [kafka]

2024-04-07 Thread via GitHub


chia7712 commented on code in PR #15668:
URL: https://github.com/apache/kafka/pull/15668#discussion_r1554964050


##
server-common/src/test/java/org/apache/kafka/server/util/timer/TimerTest.java:
##
@@ -76,6 +77,7 @@ public void setup() {
 @AfterEach
 public void teardown() throws Exception {
 timer.close();
+TestUtils.waitForCondition(() -> timer.isExecutorTerminated(), "timer 
excutor not terminated");

Review Comment:
   typo: excutor -> executor



##
server-common/src/test/java/org/apache/kafka/server/util/timer/SystemTimerReaperTest.java:
##
@@ -59,4 +60,16 @@ public void testReaper() throws Exception {
 timer.close();
 }
 }
+
+@Test
+public void testReaperClose() throws Exception {
+Timer timer = Mockito.mock(Timer.class);
+SystemTimerReaper timerReaper2 = new SystemTimerReaper("reaper", 
timer);
+timerReaper2.close();
+Mockito.verify(timer, Mockito.times(1)).close();
+
+SystemTimerReaper timerReaper = new SystemTimerReaper("reaper", new 
SystemTimer("timer"));
+timerReaper.close();
+TestUtils.waitForCondition(() -> timerReaper.isShutdown(), "reaper not 
shutdown");

Review Comment:
   we can verity `timerReaper2` instead of `timerReaper`



##
server-common/src/main/java/org/apache/kafka/server/util/timer/SystemTimer.java:
##
@@ -110,4 +110,9 @@ public int size() {
 public void close() {
 taskExecutor.shutdown();
 }
+
+// visible for testing
+boolean isExecutorTerminated() {

Review Comment:
   `isTerminated`. we don't need to expose name of inner variable



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[jira] [Commented] (KAFKA-16483) Apply `ClusterTestExtensions` to DeleteOffsetsConsumerGroupCommandIntegrationTest

2024-04-07 Thread Chia-Ping Tsai (Jira)


[ 
https://issues.apache.org/jira/browse/KAFKA-16483?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17834675#comment-17834675
 ] 

Chia-Ping Tsai commented on KAFKA-16483:


sure. I have assigned this to you [~yangpoan] 

> Apply `ClusterTestExtensions` to 
> DeleteOffsetsConsumerGroupCommandIntegrationTest
> -
>
> Key: KAFKA-16483
> URL: https://issues.apache.org/jira/browse/KAFKA-16483
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: PoAn Yang
>Priority: Minor
>
> By using ClusterTestExtensions, 
> DeleteOffsetsConsumerGroupCommandIntegrationTest get get away from 
> KafkaServerTestHarness dependency



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Assigned] (KAFKA-16483) Apply `ClusterTestExtensions` to DeleteOffsetsConsumerGroupCommandIntegrationTest

2024-04-07 Thread Chia-Ping Tsai (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-16483?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chia-Ping Tsai reassigned KAFKA-16483:
--

Assignee: PoAn Yang  (was: Chia-Ping Tsai)

> Apply `ClusterTestExtensions` to 
> DeleteOffsetsConsumerGroupCommandIntegrationTest
> -
>
> Key: KAFKA-16483
> URL: https://issues.apache.org/jira/browse/KAFKA-16483
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: PoAn Yang
>Priority: Minor
>
> By using ClusterTestExtensions, 
> DeleteOffsetsConsumerGroupCommandIntegrationTest get get away from 
> KafkaServerTestHarness dependency



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (KAFKA-16483) Apply `ClusterTestExtensions` to DeleteOffsetsConsumerGroupCommandIntegrationTest

2024-04-07 Thread PoAn Yang (Jira)


[ 
https://issues.apache.org/jira/browse/KAFKA-16483?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17834674#comment-17834674
 ] 

PoAn Yang commented on KAFKA-16483:
---

Hi [~chia7712], I'm interested in this. May I assign to myself? Thank you.

> Apply `ClusterTestExtensions` to 
> DeleteOffsetsConsumerGroupCommandIntegrationTest
> -
>
> Key: KAFKA-16483
> URL: https://issues.apache.org/jira/browse/KAFKA-16483
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Chia-Ping Tsai
>Priority: Minor
>
> By using ClusterTestExtensions, 
> DeleteOffsetsConsumerGroupCommandIntegrationTest get get away from 
> KafkaServerTestHarness dependency



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Created] (KAFKA-16483) Apply `ClusterTestExtensions` to DeleteOffsetsConsumerGroupCommandIntegrationTest

2024-04-07 Thread Chia-Ping Tsai (Jira)
Chia-Ping Tsai created KAFKA-16483:
--

 Summary: Apply `ClusterTestExtensions` to 
DeleteOffsetsConsumerGroupCommandIntegrationTest
 Key: KAFKA-16483
 URL: https://issues.apache.org/jira/browse/KAFKA-16483
 Project: Kafka
  Issue Type: Test
Reporter: Chia-Ping Tsai
Assignee: Chia-Ping Tsai


By using ClusterTestExtensions, 
DeleteOffsetsConsumerGroupCommandIntegrationTest get get away from 
KafkaServerTestHarness dependency



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [PR] KAFKA-16477: Detect thread leaked client-metrics-reaper in tests [kafka]

2024-04-07 Thread via GitHub


brandboat commented on code in PR #15668:
URL: https://github.com/apache/kafka/pull/15668#discussion_r1554955481


##
server-common/src/test/java/org/apache/kafka/server/util/timer/SystemTimerReaperTest.java:
##
@@ -59,4 +59,11 @@ public void testReaper() throws Exception {
 timer.close();
 }
 }
+
+@Test
+public void testReaperClose() throws Exception {

Review Comment:
   I've addressed the comments in the latest commit, many thanks !



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[jira] [Updated] (KAFKA-16482) Avoid changing ClusterConfig in `BeforeEach` phase

2024-04-07 Thread Chia-Ping Tsai (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-16482?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chia-Ping Tsai updated KAFKA-16482:
---
Description: 
IDE does not like the code style, and we can leverage `ClusterConfigProperty` 
to eliminate the false error from IDE

 

[https://github.com/apache/kafka/blob/trunk/core/src/test/scala/integration/kafka/coordinator/transaction/ProducerIdsIntegrationTest.scala#L42]

[https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java#L75]

 

https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java#L68

  was:
IDE does not like the code style, and we can leverage `ClusterConfigProperty` 
to eliminate the false error from IDE

 

[https://github.com/apache/kafka/blob/trunk/core/src/test/scala/integration/kafka/coordinator/transaction/ProducerIdsIntegrationTest.scala#L42]

https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java#L75


> Avoid changing ClusterConfig in `BeforeEach` phase
> --
>
> Key: KAFKA-16482
> URL: https://issues.apache.org/jira/browse/KAFKA-16482
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Cheng-Kai, Zhang
>Priority: Major
>
> IDE does not like the code style, and we can leverage `ClusterConfigProperty` 
> to eliminate the false error from IDE
>  
> [https://github.com/apache/kafka/blob/trunk/core/src/test/scala/integration/kafka/coordinator/transaction/ProducerIdsIntegrationTest.scala#L42]
> [https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java#L75]
>  
> https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java#L68



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (KAFKA-16482) Avoid changing ClusterConfig in `BeforeEach` phase

2024-04-07 Thread Chia-Ping Tsai (Jira)


[ 
https://issues.apache.org/jira/browse/KAFKA-16482?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17834663#comment-17834663
 ] 

Chia-Ping Tsai commented on KAFKA-16482:


sure. go ahead

> Avoid changing ClusterConfig in `BeforeEach` phase
> --
>
> Key: KAFKA-16482
> URL: https://issues.apache.org/jira/browse/KAFKA-16482
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Cheng-Kai, Zhang
>Priority: Major
>
> IDE does not like the code style, and we can leverage `ClusterConfigProperty` 
> to eliminate the false error from IDE
>  
> [https://github.com/apache/kafka/blob/trunk/core/src/test/scala/integration/kafka/coordinator/transaction/ProducerIdsIntegrationTest.scala#L42]
> https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java#L75



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Assigned] (KAFKA-16482) Avoid changing ClusterConfig in `BeforeEach` phase

2024-04-07 Thread Chia-Ping Tsai (Jira)


 [ 
https://issues.apache.org/jira/browse/KAFKA-16482?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chia-Ping Tsai reassigned KAFKA-16482:
--

Assignee: Cheng-Kai, Zhang  (was: Chia-Ping Tsai)

> Avoid changing ClusterConfig in `BeforeEach` phase
> --
>
> Key: KAFKA-16482
> URL: https://issues.apache.org/jira/browse/KAFKA-16482
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Cheng-Kai, Zhang
>Priority: Major
>
> IDE does not like the code style, and we can leverage `ClusterConfigProperty` 
> to eliminate the false error from IDE
>  
> [https://github.com/apache/kafka/blob/trunk/core/src/test/scala/integration/kafka/coordinator/transaction/ProducerIdsIntegrationTest.scala#L42]
> https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java#L75



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (KAFKA-16482) Avoid changing ClusterConfig in `BeforeEach` phase

2024-04-07 Thread Cheng-Kai, Zhang (Jira)


[ 
https://issues.apache.org/jira/browse/KAFKA-16482?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17834662#comment-17834662
 ] 

Cheng-Kai, Zhang commented on KAFKA-16482:
--

I am interested to this issue, could you assign it to me? [~chia7712] 

> Avoid changing ClusterConfig in `BeforeEach` phase
> --
>
> Key: KAFKA-16482
> URL: https://issues.apache.org/jira/browse/KAFKA-16482
> Project: Kafka
>  Issue Type: Test
>Reporter: Chia-Ping Tsai
>Assignee: Chia-Ping Tsai
>Priority: Major
>
> IDE does not like the code style, and we can leverage `ClusterConfigProperty` 
> to eliminate the false error from IDE
>  
> [https://github.com/apache/kafka/blob/trunk/core/src/test/scala/integration/kafka/coordinator/transaction/ProducerIdsIntegrationTest.scala#L42]
> https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java#L75



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Created] (KAFKA-16482) Avoid changing ClusterConfig in `BeforeEach` phase

2024-04-07 Thread Chia-Ping Tsai (Jira)
Chia-Ping Tsai created KAFKA-16482:
--

 Summary: Avoid changing ClusterConfig in `BeforeEach` phase
 Key: KAFKA-16482
 URL: https://issues.apache.org/jira/browse/KAFKA-16482
 Project: Kafka
  Issue Type: Test
Reporter: Chia-Ping Tsai
Assignee: Chia-Ping Tsai


IDE does not like the code style, and we can leverage `ClusterConfigProperty` 
to eliminate the false error from IDE

 

[https://github.com/apache/kafka/blob/trunk/core/src/test/scala/integration/kafka/coordinator/transaction/ProducerIdsIntegrationTest.scala#L42]

https://github.com/apache/kafka/blob/trunk/tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java#L75



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[PR] Fix usage of none in javadoc [kafka]

2024-04-07 Thread via GitHub


erikvanoosten opened a new pull request, #15674:
URL: https://github.com/apache/kafka/pull/15674

   - Use `Empty` instead of 'none' when referring to `Optional` values.
   - `Headers.lastHeader` returns `null` when no header is found.
   - Fix minor spelling mistakes.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] Fix usage of none in javadoc [kafka]

2024-04-07 Thread via GitHub


erikvanoosten commented on PR #15674:
URL: https://github.com/apache/kafka/pull/15674#issuecomment-2041343908

   I am a big fan of scala, but 'some' and 'none' are not the terminology used 
in the java world.
   This PR does not change any code, only javadoc.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org