Fixes #5306 - Default jetty.*.acceptors should be 1. (#6236)

* Fixes #5306 - Default jetty.*.acceptors should be 1.

Changed the acceptor default to 1, with -1 calculating a value based on the number of cores.
Updated documentation.
Fixed a glitch in ManagedSelector.getMaxSelectedKeys() to return long, not double.

Signed-off-by: Simone Bordet <[email protected]>

Author: sbordet

documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-http.adoc

@@ -29,22 +29,26 @@ The network port that Jetty listens to for clear-text HTTP/1.1 connections -- de
 `jetty.http.idleTimeout`::
 The amount of time a connection can be idle (i.e. no bytes received and no bytes sent) until the server decides to close it to save resources -- default `30` seconds.
 `jetty.http.acceptors`::
-The number of threads that compete to accept connections -- default -1 (i.e. an accept heuristic decides the value based on the number of cores).
+The number of threads that compete to accept connections -- default 1. Use -1 to let the accept heuristic decides the value; the current heuristic calculates a value based on the number of cores).
+Refer to xref:og-module-http-acceptors[this section] for more information about acceptor threads.
 `jetty.http.selectors`::
-The number of NIO selectors (with an associated thread) that manage connections -- default -1 (i.e. a select heuristic decides the value based on the number of cores).
+The number of NIO selectors (with an associated thread) that manage connections -- default -1 (i.e. a select heuristic decides the value; the current heuristic calculates a value based on the number of cores).
 
 [[og-module-http-acceptors]]
 ====== Configuration of Acceptors
 
-Accepting connections is a blocking operation, so a thread is blocked in the `accept()` call until a connection is accepted, and other threads are blocked on the lock acquired just before the `accept()` call.
+Accepting connections from remote clients may be configured as a blocking operation, or a non-blocking operation.
+
+When accepting connections is configured as a blocking operation (the number of acceptors is greater than zero), a thread is blocked in the `accept()` call until a connection is accepted, and other acceptor threads (if any) are blocked on the lock acquired by the accepting thread just before the `accept()` call.
 
 When the accepting thread accepts a connection, it performs a little processing of the just accepted connection, before forwarding it to other components.
 
 During this little processing other connections may be established; if there is only one accepting thread, the newly established connections are waiting for the accepting thread to finish the processing of the previously accepted connection and call again `accept()`.
 
-Servers that manage a very high number of connections that may (naturally) come and go, or that handle inefficient protocols that open and close connections very frequently (such as HTTP/1.0) may benefit of an increased number of acceptor threads.
+Servers that manage a very high number of connections that may (naturally) come and go, or that handle inefficient protocols that open and close connections very frequently (such as HTTP/1.0) may benefit of an increased number of acceptor threads, so that when one acceptor thread processes a just accepted connection, another acceptor thread can immediately take over accepting connections.
 
-// TODO: expand on acceptors=0 and non-blocking accepts
+When accepting connections is configured as a non-blocking operation (the number of acceptors is zero), then the server socket is set in non-blocking mode and added to a NIO selector.
+In this way, no dedicated acceptor threads exist: the work of accepting connections is performed by the selector thread.
 
 [[og-module-http-selectors]]
 ====== Configuration of Selectors

documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl.adoc

@@ -29,13 +29,13 @@ Among the configurable properties, the most relevant are:
 
 `jetty.ssl.port`::
 The network port that Jetty listens to for secure connections -- default `8443`.
-`jetty.http.idleTimeout`::
+`jetty.ssl.idleTimeout`::
 The amount of time a connection can be idle (i.e. no bytes received and no bytes sent) until the server decides to close it to save resources -- default `30000` milliseconds.
-`jetty.http.acceptors`::
-The number of threads that compete to accept connections -- default -1 (i.e. an accept heuristic decides the value based on the number of cores).
+`jetty.ssl.acceptors`::
+The number of threads that compete to accept connections -- default 1. Use -1 to let the accept heuristic decides the value; the current heuristic calculates a value based on the number of cores).
 Refer to xref:og-module-http-acceptors[this section] for more information about acceptor threads.
-`jetty.http.selectors`::
-The number of NIO selectors (with an associated thread) that manage connections -- default -1 (i.e. a select heuristic decides the value based on the number of cores).
+`jetty.ssl.selectors`::
+The number of NIO selectors (with an associated thread) that manage connections -- default -1 (i.e. a select heuristic decides the value; the current heuristic calculates a value based on the number of cores).
 Refer to xref:og-module-http-selectors[this section] for more information about selector threads.
 
 The module properties to configure the KeyStore and TLS parameters are:

jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java

@@ -158,7 +158,7 @@ public double getAverageSelectedKeys()
     }
 
     @ManagedAttribute(value = "Maximum number of selected keys", readonly = true)
-    public double getMaxSelectedKeys()
+    public long getMaxSelectedKeys()
     {
         return _keyStats.getMax();
     }

jetty-server/src/main/config/etc/jetty-http.xml

@@ -22,7 +22,7 @@
     <Arg>
       <New id="httpConnector" class="org.eclipse.jetty.server.ServerConnector">
         <Arg name="server"><Ref refid="Server" /></Arg>
-        <Arg name="acceptors" type="int"><Property name="jetty.http.acceptors" default="-1"/></Arg>
+        <Arg name="acceptors" type="int"><Property name="jetty.http.acceptors" default="1"/></Arg>
         <Arg name="selectors" type="int"><Property name="jetty.http.selectors" default="-1"/></Arg>
         <Arg name="factories">
           <Array type="org.eclipse.jetty.server.ConnectionFactory">

jetty-server/src/main/config/etc/jetty-ssl.xml

@@ -15,7 +15,7 @@
     <Arg>
       <New id="sslConnector" class="org.eclipse.jetty.server.ServerConnector">
         <Arg name="server"><Ref refid="Server" /></Arg>
-        <Arg name="acceptors" type="int"><Property name="jetty.ssl.acceptors" default="-1"/></Arg>
+        <Arg name="acceptors" type="int"><Property name="jetty.ssl.acceptors" default="1"/></Arg>
         <Arg name="selectors" type="int"><Property name="jetty.ssl.selectors" default="-1"/></Arg>
         <Arg name="factories">
           <Array type="org.eclipse.jetty.server.ConnectionFactory">

jetty-server/src/main/config/modules/http.mod

@@ -26,7 +26,7 @@ etc/jetty-http.xml
 # jetty.http.idleTimeout=30000
 
 ## The number of acceptors (-1 picks a default value based on number of cores).
-# jetty.http.acceptors=-1
+# jetty.http.acceptors=1
 
 ## The number of selectors (-1 picks a default value based on number of cores).
 # jetty.http.selectors=-1

jetty-server/src/main/config/modules/ssl.mod

@@ -28,7 +28,7 @@ etc/jetty-ssl-context.xml
 # jetty.ssl.idleTimeout=30000
 
 ## The number of acceptors (-1 picks a default value based on number of cores).
-# jetty.ssl.acceptors=-1
+# jetty.ssl.acceptors=1
 
 ## The number of selectors (-1 picks a default value based on number of cores).
 # jetty.ssl.selectors=-1

jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNetworkConnector.java

@@ -30,7 +30,6 @@
 @ManagedObject("AbstractNetworkConnector")
 public abstract class AbstractNetworkConnector extends AbstractConnector implements NetworkConnector
 {
-
     private volatile String _host;
     private volatile int _port = 0;
 

jetty-server/src/test/java/org/eclipse/jetty/server/ServerConnectorAcceptTest.java

@@ -0,0 +1,150 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.server;
+
+import java.io.IOException;
+import java.net.Socket;
+import java.nio.charset.StandardCharsets;
+import java.util.Collection;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.http.HttpTester;
+import org.eclipse.jetty.io.ManagedSelector;
+import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class ServerConnectorAcceptTest
+{
+    @ParameterizedTest
+    @ValueSource(ints = {0, 1, 2})
+    public void testAccept(int acceptors) throws Exception
+    {
+        Server server = new Server();
+        ServerConnector connector = new ServerConnector(server, acceptors, 1);
+        server.addConnector(connector);
+        server.setHandler(new AbstractHandler()
+        {
+            @Override
+            public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
+            {
+                jettyRequest.setHandled(true);
+            }
+        });
+        server.start();
+
+        int runs = 4;
+        try
+        {
+            for (int r = 0; r < runs; ++r)
+            {
+                test(acceptors, connector);
+            }
+        }
+        finally
+        {
+            server.stop();
+        }
+    }
+
+    private void test(int acceptors, ServerConnector connector) throws InterruptedException
+    {
+        int threads = 8;
+        int iterations = 4096;
+
+        CyclicBarrier barrier = new CyclicBarrier(threads + 1);
+        CountDownLatch latch = new CountDownLatch(threads * iterations);
+        IntStream.range(0, threads)
+            .mapToObj(t -> new Thread(() ->
+            {
+                try
+                {
+                    assertTrue(awaitBarrier(barrier));
+
+                    long start = System.nanoTime();
+                    for (int i = 0; i < iterations; ++i)
+                    {
+                        try (Socket socket = new Socket("localhost", connector.getLocalPort()))
+                        {
+                            String request = "GET / HTTP/1.1\r\n" +
+                                "Host: localhost\r\n" +
+                                "Connection: close\r\n" +
+                                "\r\n";
+                            socket.getOutputStream().write(request.getBytes(StandardCharsets.UTF_8));
+                            HttpTester.Response response = HttpTester.parseResponse(socket.getInputStream());
+                            assertNotNull(response);
+                            assertEquals(HttpStatus.OK_200, response.getStatus());
+                        }
+                        catch (IOException x)
+                        {
+                            x.printStackTrace();
+                        }
+                        finally
+                        {
+                            latch.countDown();
+                        }
+                    }
+                    long elapsed = System.nanoTime() - start;
+                    System.err.printf("%d acceptors, %d threads, %d requests each, time = %d ms%n",
+                        acceptors,
+                        threads,
+                        iterations,
+                        TimeUnit.NANOSECONDS.toMillis(elapsed));
+                }
+                finally
+                {
+                    assertTrue(awaitBarrier(barrier));
+                }
+            }))
+            .forEach(Thread::start);
+
+        // Wait for all the threads to be ready.
+        assertTrue(awaitBarrier(barrier));
+
+        // Wait for all the threads to be finished.
+        assertTrue(awaitBarrier(barrier));
+
+        // Verify that all requests succeeded.
+        assertTrue(latch.await(15, TimeUnit.SECONDS));
+
+        Collection<ManagedSelector> selectors = connector.getSelectorManager().getBeans(ManagedSelector.class);
+        selectors.stream()
+            .map(s -> String.format("avg selected keys = %.3f, selects = %d", s.getAverageSelectedKeys(), s.getSelectCount()))
+            .forEach(System.err::println);
+        selectors.forEach(ManagedSelector::resetStats);
+    }
+
+    private boolean awaitBarrier(CyclicBarrier barrier)
+    {
+        try
+        {
+            barrier.await();
+            return true;
+        }
+        catch (Throwable x)
+        {
+            return false;
+        }
+    }
+}