junrao commented on code in PR #17473:
URL: https://github.com/apache/kafka/pull/17473#discussion_r1803495546
##########
core/src/main/scala/kafka/network/SocketServer.scala:
##########
@@ -929,17 +936,17 @@ private[kafka] class Processor(
private object ConnectionId {
def fromString(s: String): Option[ConnectionId] = s.split("-") match {
- case Array(local, remote, index) =>
BrokerEndPoint.parseHostPort(local).flatMap { case (localHost, localPort) =>
+ case Array(local, remote, processorId, index) =>
BrokerEndPoint.parseHostPort(local).flatMap { case (localHost, localPort) =>
BrokerEndPoint.parseHostPort(remote).map { case (remoteHost,
remotePort) =>
- ConnectionId(localHost, localPort, remoteHost, remotePort,
Integer.parseInt(index))
+ ConnectionId(localHost, localPort, remoteHost, remotePort,
Integer.parseInt(processorId), Integer.parseInt(index))
}
}
case _ => None
}
}
- private[network] case class ConnectionId(localHost: String, localPort: Int,
remoteHost: String, remotePort: Int, index: Int) {
- override def toString: String =
s"$localHost:$localPort-$remoteHost:$remotePort-$index"
+ private[network] case class ConnectionId(localHost: String, localPort: Int,
remoteHost: String, remotePort: Int, processorId: Int, index: Int) {
Review Comment:
The connectionId logic is split between `SocketServer` and
`Selector.generateConnectionId`. Perhaps we could consolidate them into a top
level class ConnectionIdForServer, which can be constructed from either a
string or a socket. It also has a `toString` method to get the string
representation of the connectionId.
##########
clients/src/main/java/org/apache/kafka/common/network/Selector.java:
##########
@@ -237,15 +237,16 @@ public Selector(long connectionMaxIdleMS, int
failedAuthenticationDelayMs, Metri
* Generates a unique connection ID for the given socket.
*
* @param socket The socket for which the connection ID is to be generated.
+ * @param processorId The ID of the processor that will handle this
connection.
Review Comment:
The ID of the processor => The ID of the server processor
##########
core/src/test/scala/unit/kafka/network/SocketServerTest.scala:
##########
@@ -2050,6 +2053,25 @@ class SocketServerTest {
}
}
+ @Test
+ def testConnectionDisconnectListenerInvokedOnClose(): Unit = {
+ var listenerConnectionId: String = ""
Review Comment:
Should this be volatile since it's read and written by different threads?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
