jetty.project icon indicating copy to clipboard operation
jetty.project copied to clipboard

Server should NOT open connectors early in start sequence

Open kelunik opened this issue 1 year ago • 4 comments

Jetty version(s) 10.x, 11.x, 12.x

Jetty Environment core

Java version/vendor (use: java -version) 17.0.11 / any

OS type/version macOS / any

Description Jetty 10.x introduced an enhancement to open network sockets early in the start sequence: https://github.com/jetty/jetty.project/issues/2103. I guess this is preferable to fail early in case the network address is not available?

Unfortunately, this behavior is problematic for clients. For them it looks like the server hangs, because their network connections are accepted, but nothing happens. In my case the client is an Nginx instance that has two Jetty instances defined as upstreams. If one of them fails to connect, Nginx will try on the other server.

With Jetty 10-12 this won't work properly, because Nginx connects fine in case one of the upstreams is still starting, but then doesn't get a response and fails upon the read timeout (instead of failing at the connect timeout like it did with Jetty 9).

How to reproduce?

Run the below Java code and run curl during the start phase of Jetty (within the sleep period). With Jetty 9 this will result in Couldn't connect to server, while on Jetty 10-12 this will hang until the server is started and then return a response.

curl --insecure http://localhost:1337 -vvv
import java.util.Arrays;

import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.component.AbstractLifeCycle;


public class App {

   public static void main( String[] args ) throws Exception {
      var server = new Server(1337);

      server.setHandler(new Handler.Abstract() {

         @Override
         public boolean handle( Request request, Response response, Callback callback ) {
            callback.succeeded();
            return true;
         }
      });

      server.addManaged(new AbstractLifeCycle() {

         @Override
         protected void doStart() {
            try {
               Thread.sleep(10000);
            }
            catch ( InterruptedException e ) {
               throw new RuntimeException(e);
            }
         }
      });

      server.start();
      server.join();
   }
}

I managed to work around this issue by removing all connectors and re-adding them in the start sequence, but this seems like a very ugly hack. The code needs to be inserted right before server.start(); in the above example.

   Connector[] connectors = server.getConnectors();
      Arrays.stream(connectors).forEach(server::removeConnector);
      server.addManaged(new AbstractLifeCycle() {

         @Override
         protected void doStart() {
            Arrays.stream(connectors).forEach(server::addConnector);
         }
      });

kelunik avatar Jul 16 '24 13:07 kelunik

This is controversial, as the cloud providers and docker/k8s users prefer early open.

joakime avatar Jul 16 '24 13:07 joakime

Why do they prefer an early open?

Anyway, I guess a configuration option makes sense that allows to skip early open?

kelunik avatar Jul 16 '24 13:07 kelunik

Anyway, I guess a configuration option makes sense that allows to skip early open?

This is a sensible solution. Default is still early open. But it can be configured to be the opposite.

joakime avatar Jul 16 '24 13:07 joakime

Why do they prefer an early open?

Some webapps can take longer to deploy (scanning, init, etc) than the timeouts for those services. If the port doesn't open within the timeout then the service is viewed as "dead" and all kinds of error actions occur.

joakime avatar Jul 16 '24 13:07 joakime

PR #12049 merged.

joakime avatar Sep 24 '24 15:09 joakime