Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server should NOT open connectors early in start sequence #12047

Closed
kelunik opened this issue Jul 16, 2024 · 5 comments
Closed

Server should NOT open connectors early in start sequence #12047

kelunik opened this issue Jul 16, 2024 · 5 comments
Labels
Bug For general bugs on Jetty side

Comments

@kelunik
Copy link
Contributor

kelunik commented Jul 16, 2024

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: #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 kelunik added the Bug For general bugs on Jetty side label Jul 16, 2024
@joakime
Copy link
Contributor

joakime commented Jul 16, 2024

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

@kelunik
Copy link
Contributor Author

kelunik commented Jul 16, 2024

Why do they prefer an early open?

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

@joakime
Copy link
Contributor

joakime commented Jul 16, 2024

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
Copy link
Contributor

joakime commented Jul 16, 2024

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
Copy link
Contributor

joakime commented Sep 24, 2024

PR #12049 merged.

@joakime joakime closed this as completed Sep 24, 2024
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Jetty 12.0.14 Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Status: ✅ Done
Development

No branches or pull requests

2 participants