dajac commented on a change in pull request #11422:
URL: https://github.com/apache/kafka/pull/11422#discussion_r753096281



##########
File path: core/src/test/scala/unit/kafka/network/SocketServerTest.scala
##########
@@ -1854,6 +1854,23 @@ class SocketServerTest {
     })
   }
 
+  @Test
+  def testListenBacklogSize(): Unit = {
+    val backlogSize = 128
+    props.put("socket.listen.backlog.size", backlogSize.toString)
+
+    // TCP listen backlog size is the max count of pending connections (i.e. 
connections such that
+    // 3-way handshake is done at kernel level and waiting to be accepted by 
the server application.
+    // From client perspective, such connections should be visible as already 
"connected")
+    // Hence, we can check if listen backlog size is properly configured by 
trying to connect the server
+    // without starting acceptor thread.
+    withTestableServer(KafkaConfig.fromProps(props), { testableServer =>
+      1 to backlogSize foreach { _ =>
+        assertTrue(connect(testableServer).isConnected)

Review comment:
       Naive question: Is `isConnected` immediately true or is there a risk 
that it is not true yet when the assertion is ran?

##########
File path: core/src/test/scala/unit/kafka/network/SocketServerTest.scala
##########
@@ -1854,6 +1854,23 @@ class SocketServerTest {
     })
   }
 
+  @Test
+  def testListenBacklogSize(): Unit = {
+    val backlogSize = 128
+    props.put("socket.listen.backlog.size", backlogSize.toString)
+
+    // TCP listen backlog size is the max count of pending connections (i.e. 
connections such that
+    // 3-way handshake is done at kernel level and waiting to be accepted by 
the server application.
+    // From client perspective, such connections should be visible as already 
"connected")
+    // Hence, we can check if listen backlog size is properly configured by 
trying to connect the server
+    // without starting acceptor thread.
+    withTestableServer(KafkaConfig.fromProps(props), { testableServer =>
+      1 to backlogSize foreach { _ =>
+        assertTrue(connect(testableServer).isConnected)
+      }
+    }, false)
+  }

Review comment:
       Would it make sense to add an extra connection and to verify that 
`isConnected` is false?




-- 
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


Reply via email to