mumrah commented on code in PR #13390:
URL: https://github.com/apache/kafka/pull/13390#discussion_r1135643036
##########
core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala:
##########
@@ -238,7 +238,7 @@ class BrokerToControllerChannelManagerImpl(
}
val threadName = threadNamePrefix match {
case None => s"BrokerToControllerChannelManager
broker=${config.brokerId} name=$channelName"
- case Some(name) => s"$name:BrokerToControllerChannelManager
broker=${config.brokerId} name=$channelName"
+ case Some(name) => s"${name}ToControllerChannelManager
broker=${config.brokerId} name=$channelName"
Review Comment:
What is `name` here and where is it supplied?
##########
core/src/main/scala/kafka/server/BrokerLifecycleManager.scala:
##########
@@ -182,7 +182,7 @@ class BrokerLifecycleManager(
*/
private[server] val eventQueue = new KafkaEventQueue(time,
logContext,
- threadNamePrefix.getOrElse(""),
+ threadNamePrefix.getOrElse("") + "BrokerLifecycleManager" + nodeId,
Review Comment:
I fired up this branch and yea `BrokerLifecycleManager1EventHandler` is not
pretty :)
I'm not sure we need node ID in the thread name since we will presumably
know which node a thread dump came from. It might actually be confusing since
the convention we have is `{thread name}-{thread number in pool}` like
"metrics-meter-tick-thread-1", "metrics-meter-tick-thread-2",
"data-plane-kafka-request-handler-0", "data-plane-kafka-request-handler-1",
etc. What about something like `broker-lifecycle-manager-event-handler` or
`BrokerLifecycleManager-EventHandler`
I also noticed somewhere we are not prefixing the event queue thread name,
there is just an `EventHandler` thread.
--
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
