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