Re: [PR] MINOR: Improve logging in AssignmentsManager [kafka]
showuon merged PR #15522: URL: https://github.com/apache/kafka/pull/15522 -- 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: Improve logging in AssignmentsManager [kafka]
showuon commented on PR #15522: URL: https://github.com/apache/kafka/pull/15522#issuecomment-2051055209 Failed tests are unrelated. -- 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: Improve logging in AssignmentsManager [kafka]
showuon commented on PR #15522: URL: https://github.com/apache/kafka/pull/15522#issuecomment-2049237739 @soarez , there is failed test... :( https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15522/7/testReport/junit/kafka.server/ReplicaAlterLogDirsThreadTest/Build___JDK_21_and_Scala_2_13___shouldRevertReassignmentsForIncompleteFutureReplicaPromotions__/ -- 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: Improve logging in AssignmentsManager [kafka]
showuon commented on PR #15522: URL: https://github.com/apache/kafka/pull/15522#issuecomment-2047096126 @soarez , oops, there's conflict due to I've just merged another PR. Please help resolve it. 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: Improve logging in AssignmentsManager [kafka]
showuon commented on PR #15522: URL: https://github.com/apache/kafka/pull/15522#issuecomment-2047072476 Sorry, forgot about this PR. The jdk8 job failed to complete due to infra's issue. Re-triggering now: https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15522/6/ -- 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: Improve logging in AssignmentsManager [kafka]
soarez commented on PR #15522: URL: https://github.com/apache/kafka/pull/15522#issuecomment-2046961029 Failing tests are all unrelated and tracked: * kafka.api.DelegationTokenEndToEndAuthorizationWithOwnerTest."testNoConsumeWithDescribeAclViaAssign(String).quorum=kraft" [KAFKA-8250](https://issues.apache.org/jira/browse/KAFKA-8250) * kafka.zk.ZkMigrationIntegrationTest.testMigrateTopicDeletions [7] Type=ZK, MetadataVersion=3.7-IV4, Security=PLAINTEXT [KAFKA-15793](https://issues.apache.org/jira/browse/KAFKA-15793) (reopened) * org.apache.kafka.connect.integration.OffsetsApiIntegrationTest.testAlterSinkConnectorOffsetsOverriddenConsumerGroupId [KAFKA-15914](https://issues.apache.org/jira/browse/KAFKA-15914) * org.apache.kafka.connect.integration.OffsetsApiIntegrationTest.testAlterSinkConnectorOffsetsZombieSinkTasks [KAFKA-15917](https://issues.apache.org/jira/browse/KAFKA-15917) * org.apache.kafka.connect.mirror.integration.MirrorConnectorsIntegrationBaseTest.testReplicateSourceDefault() [KAFKA-15787](https://issues.apache.org/jira/browse/KAFKA-15787) * org.apache.kafka.connect.mirror.integration.MirrorConnectorsIntegrationExactlyOnceTest.testOffsetTranslationBehindReplicationFlow() [KAFKA-15197](https://issues.apache.org/jira/browse/KAFKA-15197) * org.apache.kafka.connect.mirror.integration.MirrorConnectorsIntegrationSSLTest.testReplicateSourceDefault() [KAFKA-15926](https://issues.apache.org/jira/browse/KAFKA-15926) * org.apache.kafka.controller.QuorumControllerTest.testConfigurationOperations() [KAFKA-16504](https://issues.apache.org/jira/browse/KAFKA-16504) (new) * org.apache.kafka.tools.MetadataQuorumCommandTest.testDescribeQuorumStatusSuccessful [6] Type=Raft-Isolated, MetadataVersion=3.8-IV0, Security=PLAINTEXT [KAFKA-16174](https://issues.apache.org/jira/browse/KAFKA-16174) 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] MINOR: Improve logging in AssignmentsManager [kafka]
showuon commented on PR #15522: URL: https://github.com/apache/kafka/pull/15522#issuecomment-2033354648 ``` /home/jenkins/workspace/Kafka_kafka-pr_PR-15522/server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java:354: error: method onAssignment in class AssignmentsManager cannot be applied to given types; [2024-04-02T21:59:04.001Z] manager.onAssignment(new TopicIdPartition(TOPIC_1, 0), dirs[i % 3], onComplete); [2024-04-02T21:59:04.001Z]^ [2024-04-02T21:59:04.001Z] required: TopicIdPartition,Uuid,String,Runnable [2024-04-02T21:59:04.001Z] found: TopicIdPartition,Uuid,Runnable [2024-04-02T21:59:04.001Z] reason: actual and formal argument lists differ in length ``` Another compilation error: https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-15522/3/pipeline Please help fix it. 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: Improve logging in AssignmentsManager [kafka]
soarez commented on code in PR #15522:
URL: https://github.com/apache/kafka/pull/15522#discussion_r1548655446
##
core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala:
##
@@ -458,7 +458,7 @@ class ReplicaAlterLogDirsThreadTest {
ArgumentCaptor.forClass(classOf[org.apache.kafka.server.common.TopicIdPartition])
val logIdCaptureT1p0: ArgumentCaptor[Uuid] =
ArgumentCaptor.forClass(classOf[Uuid])
-
verify(directoryEventHandler).handleAssignment(topicIdPartitionCaptureT1p0.capture(),
logIdCaptureT1p0.capture(), any())
+
verify(directoryEventHandler).handleAssignment(topicIdPartitionCaptureT1p0.capture(),
logIdCaptureT1p0.capture(), "test", any())
Review Comment:
Thanks for pointing this out. Fixed the test and also resolved the conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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: Improve logging in AssignmentsManager [kafka]
showuon commented on code in PR #15522:
URL: https://github.com/apache/kafka/pull/15522#discussion_r1547005718
##
core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala:
##
@@ -458,7 +458,7 @@ class ReplicaAlterLogDirsThreadTest {
ArgumentCaptor.forClass(classOf[org.apache.kafka.server.common.TopicIdPartition])
val logIdCaptureT1p0: ArgumentCaptor[Uuid] =
ArgumentCaptor.forClass(classOf[Uuid])
-
verify(directoryEventHandler).handleAssignment(topicIdPartitionCaptureT1p0.capture(),
logIdCaptureT1p0.capture(), any())
+
verify(directoryEventHandler).handleAssignment(topicIdPartitionCaptureT1p0.capture(),
logIdCaptureT1p0.capture(), "test", any())
Review Comment:
Looks like this change failed tests:
https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15522/2/testReport/junit/kafka.server/ReplicaAlterLogDirsThreadTest/Build___JDK_11_and_Scala_2_13___shouldRevertAnyScheduledAssignmentRequestIfAssignmentIsCancelled__/
Maybe `anyString()`?
--
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: Improve logging in AssignmentsManager [kafka]
soarez commented on code in PR #15522: URL: https://github.com/apache/kafka/pull/15522#discussion_r1546358504 ## core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala: ## @@ -105,7 +105,7 @@ class ReplicaAlterLogDirsThread(name: String, topicId <- partition.topicId directoryId <- partition.logDirectoryId() topicIdPartition = new TopicIdPartition(topicId, topicPartition.partition()) -} directoryEventHandler.handleAssignment(topicIdPartition, directoryId, () => ()) +} directoryEventHandler.handleAssignment(topicIdPartition, directoryId, "Reverting queued dir reassignment mid future replica promotion", () => ()) Review Comment: Changed the wording. Let me know what you think. -- 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: Improve logging in AssignmentsManager [kafka]
soarez commented on code in PR #15522:
URL: https://github.com/apache/kafka/pull/15522#discussion_r1546352789
##
core/src/main/scala/kafka/server/ReplicaManager.scala:
##
@@ -624,6 +624,13 @@ class ReplicaManager(val config: KafkaConfig,
errorMap
}
+ def topicNameFromId(topicId: Uuid): Option[String] =
+allPartitions.values.flatMap {
+ case HostedPartition.Offline(Some(partition)) if
partition.topicId.contains(topicId) => Some(partition.topic)
+ case Online(partition) if partition.topicId.contains(topicId) =>
Some(partition.topic)
+ case _ => None
+}.headOption
Review Comment:
Looks like I forgot to remove this.
--
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: Improve logging in AssignmentsManager [kafka]
soarez commented on code in PR #15522: URL: https://github.com/apache/kafka/pull/15522#discussion_r1546351970 ## core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala: ## @@ -105,7 +105,7 @@ class ReplicaAlterLogDirsThread(name: String, topicId <- partition.topicId directoryId <- partition.logDirectoryId() topicIdPartition = new TopicIdPartition(topicId, topicPartition.partition()) -} directoryEventHandler.handleAssignment(topicIdPartition, directoryId, () => ()) +} directoryEventHandler.handleAssignment(topicIdPartition, directoryId, "Reverting queued dir reassignment mid future replica promotion", () => ()) Review Comment: These aren't the words we'd want to couple together. I meant "mid [future replica promotion]". i.e. in the middle of the process of promoting a future replica. Perhaps there's a better way to express this. -- 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: Improve logging in AssignmentsManager [kafka]
showuon commented on code in PR #15522:
URL: https://github.com/apache/kafka/pull/15522#discussion_r1546066080
##
core/src/main/scala/kafka/server/ReplicaManager.scala:
##
@@ -624,6 +624,13 @@ class ReplicaManager(val config: KafkaConfig,
errorMap
}
+ def topicNameFromId(topicId: Uuid): Option[String] =
+allPartitions.values.flatMap {
+ case HostedPartition.Offline(Some(partition)) if
partition.topicId.contains(topicId) => Some(partition.topic)
+ case Online(partition) if partition.topicId.contains(topicId) =>
Some(partition.topic)
+ case _ => None
+}.headOption
Review Comment:
What's this for?
##
core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala:
##
@@ -105,7 +105,7 @@ class ReplicaAlterLogDirsThread(name: String,
topicId <- partition.topicId
directoryId <- partition.logDirectoryId()
topicIdPartition = new TopicIdPartition(topicId,
topicPartition.partition())
-} directoryEventHandler.handleAssignment(topicIdPartition, directoryId, ()
=> ())
+} directoryEventHandler.handleAssignment(topicIdPartition, directoryId,
"Reverting queued dir reassignment mid future replica promotion", () => ())
Review Comment:
What does "mid future" mean here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] MINOR: Improve logging in AssignmentsManager [kafka]
showuon commented on PR #15522: URL: https://github.com/apache/kafka/pull/15522#issuecomment-2022115723 Will check it this week or next. -- 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: Improve logging in AssignmentsManager [kafka]
soarez commented on PR #15522: URL: https://github.com/apache/kafka/pull/15522#issuecomment-1991528082 @showuon PTAL -- 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
