spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.63k stars 40.56k forks source link

Allow the embedded web server to be shut down gracefully #4657

Closed LutzStrobel closed 4 years ago

LutzStrobel commented 8 years ago

We are using spring-boot and spring-cloud to realize a micro service archtecture. During load tests we are facing lots of errors such as "entitymanager closed and others" if we shut the micro service down during load. We are wondering if there is an option to configure the embedded container to shut its service connector down and waits for an empty request queue before shutting down the complete application context.

If there is no such option, I did not find any, then it would be great to extend the shutdownhook of spring to respect such requirements.

RichardBradley commented 6 years ago

FWIW, I think this is a feature specific to Spring Boot, and doesn't need any Spring Framework support.

The entity which is being "gracefully shut down" here is not necessarily the entire Spring app context, but could just be the Spring Boot webapp. What is needed is a way for the Spring boot app to ask its EmbeddedServletContainer to stop accepting new requests, then to trigger a normal Spring shutdown when all in-flight requests have completed.

Since the TomcatEmbeddedServletContainer lives in the org.springframework.boot:spring-boot jar, this seems like a Spring Boot and/or a Tomcat feature request, rather than a Spring Framework feature.

I don't think there is currently any way to either a) ask Tomcat to stop accepting new requests without cancelling in-progress requests ("Pause" has problems, see above) or b) to ask Tomcat if there are any requests in progress, is there?

The thread above has gotten sidetracked into discussions of Spring-managed components dependency orders etc., which I think is not necessary for the desired feature.

wilkinsona commented 6 years ago

I don't think there is currently any way to either a) ask Tomcat to stop accepting new requests without cancelling in-progress requests or b) to ask Tomcat if there are any requests in progress, is there?

Both already possible, as shown by code that has already been shared above.

The thread above has gotten sidetracked into discussions of Spring-managed components dependency orders etc., which I think is not necessary for the desired feature.

We disagree. If we are to introduce the concept of graceful shutdown it's important that we introduce it in the right place. This applies irrespective of exactly what it is that's being gracefully shut down. We do not want to get into the situation where we introduce something that is specific to Spring Boot and its embedded container only to realise that a more general solution should have been introduced in Spring Framework and then built upon by Boot.

RichardBradley commented 6 years ago

Both already possible, as shown by code that has already been shared above.

As per nithril's comment that's not a graceful shutdown, as the server continues to accept requests during the shutdown period, but stalls them and fails them all at once at the end, instead of unbinding from the socket which is what I understand by "graceful shutdown", like apache httpd does

Perhaps that is a Tomcat bug, rather than a Spring Boot one, though.

This applies irrespective of exactly what it is that's being gracefully shut down.

In my opinion, this is premature generalisation. There is a real and immediate need for Spring Boot webapps to have graceful HTTP shutdown, which it is not currently possible to meet, as seen in this thread. I don't see an immediate need for a generalised Spring Component shutdown ordering framework (and as noted above, there is already such a thing to some extent via DisposableBean). It seems like it might be easier to fix the current specific case before starting a big rethink about the proposed general case.

Regardless of whether we do a generalised shutdown framework or a specific HTTP one, there is some integration work to be done in adding a graceful "unbind" command to each of the implementations of EmbeddedServletContainer. If that work were done now, the specific case would be within easy reach. I think the general case is a distraction from that.

Of course, I am just an interested bystander, so it is up to the project maintainers where to take this :-)

wilkinsona commented 6 years ago

that's not a graceful shutdown

That's a matter of opinion and is something that's already been raised and addressed above. We're just retreading old ground here without anything new being added to the discussion which is a waste of both my time and yours.

There is a real and immediate need for Spring Boot webapps to have graceful HTTP shutdown, which it is not currently possible to meet

The comments above show that's not the case. There are multiple users who have commented above with examples of something that works for their specific needs.

It seems like it might be easier to fix the current specific case before starting a big rethink about the proposed general case.

Being easier doesn't make it the right thing to do. If we forge ahead now with something that then doesn't fit well without something that's subsequently added to Spring Framework we'll be left with a mess that will be difficult to straighten out. We need to take the time now to think things through and give us the best possible chance to get things right first time.

wilkinsona commented 6 years ago

With Jetty, I think things can be a bit simpler than the example shared above. All you need to do is to add the StatisticsHandler:

@Bean
public WebServerFactoryCustomizer<JettyServletWebServerFactory> jettyCustomizer() {
    return (factory) -> {
        factory.addServerCustomizers((server) -> {
            StatisticsHandler handler = new StatisticsHandler();
            handler.setHandler(server.getHandler());
            server.setHandler(handler);
        });
    };
}

This is sufficient to switch on Jetty's graceful shutdown with its default timeout period of 30 seconds. To customise the timeout, a call to server.setStopTimeout(timeInMillis) can be made in the same server customizer. Closing the application context will take care of calling Server.stop().

Jetty's behaviour during graceful shutdown is as follows:

  1. New connections are refused
  2. Wait for up to the shutdown timeout period for the number of active requests to drop to zero
  3. Drop the connections for any requests that are still active
wilkinsona commented 6 years ago

Here's the equivalent configuration for Undertow:

@Bean
public GracefulUndertowShutdown GracefulUndertowShutdown() {
    return new GracefulUndertowShutdown();
}

@Bean
public WebServerFactoryCustomizer<UndertowServletWebServerFactory> undertowCustomizer(
        GracefulUndertowShutdown gracefulUndertowShutdown) {
    return (factory) -> {
        factory.addDeploymentInfoCustomizers((builder) -> {
            builder.addInitialHandlerChainWrapper(gracefulUndertowShutdown);
        });
    };
}

private static class GracefulUndertowShutdown
        implements ApplicationListener<ContextClosedEvent>, HandlerWrapper {

    private volatile GracefulShutdownHandler handler;

    @Override
    public HttpHandler wrap(HttpHandler handler) {
        this.handler = new GracefulShutdownHandler(handler);
        return this.handler;
    }

    @Override
    public void onApplicationEvent(ContextClosedEvent event) {
        try {
            this.handler.shutdown();
            this.handler.awaitShutdown(30000);
        }
        catch (InterruptedException ex) {
            Thread.currentThread().interrupt();
        }
    }

}

Undertow's behaviour during graceful shutdown is as follows:

  1. Respond to new requests with a 503
  2. Wait for up to the shutdown timeout period for the number of active requests to drop to zero
  3. Drop the connections for any requests that are still active
wilkinsona commented 6 years ago

With Jetty, I think things can be a bit simpler than the example shared above.

I take this back.

I said above that " closing the application context will take care of calling Server.stop()". That's true, but it'll get Jetty into a state where it rejects new connections after most of the context close processing has occurred. It needs to be happen earlier and when ContextRefreshedEvent is published is an ideal time to do so.

Having thought some more, I think it's actually a bit more complicated than the example shared above. We need to get Jetty into a state where it rejects new connections without actually calling Server.stop(). If we call Server.stop(), we may encounter problems that are similar to those that we encountered when we tried to invert the ordering of context close and server stop.

ghost commented 6 years ago

In my opinion there is only one logic which should work for all kind of loadbalancers:

  1. Stop accepting new requests (new connections or channels are refused)
  2. Waiting until all requests have been processed or shutdown timeout
  3. Shutdown Spring context

Responding with 503 only works for "real" http loadbalancers. No iptables or tcp (e.g. for ssl passthrough) loadbalancing.

pmhsfelix commented 6 years ago

@wilkinsona: does the first example works correctly with asynchronous processing? There can be pending requests and no busy executor threads. So, unless there is special logic on the executor, the shutdown could succeed even with pending requests.

wilkinsona commented 6 years ago

@pmhsfelix I don't know yet. @snicoll and I discussed it briefly last week and it's something that I need to investigate for all our supported containers.

pdeva commented 6 years ago

@wilkinsona could you update example for https://github.com/spring-projects/spring-boot/issues/4657#issuecomment-161354811 with code for Spring Boot 2?

it seems the relevant classes have been renamed and moved to different packages in Spring boot 2.

wilkinsona commented 6 years ago

Jetty does not consider asynchronous requests when shutting down gracefully. I've opened https://github.com/eclipse/jetty.project/issues/2717.

buch11 commented 6 years ago

Update : ---IGNORE THIS COMMENT---

I am not sure if this a widely faced problem or not but what I am facing is, we have a kafka stream processor running as a spring boot app(parent : spring-boot-starter-parent) inside a docker container.

As soon as we do a docker stop on container, we are observing 503s for some of the outbound calls to external REST api(using resttemplates).

Has there been any workarounds or plans to release a spring-boot version with a fix?

wilkinsona commented 6 years ago

@buch11 Please don’t drag this issue off-topic. Outbound HTTP calls in a Docker container are unrelated to graceful shutdown of the embedded container.

kppullin commented 6 years ago

@pdeva - Here's an updated snippet for Spring Boot 2:

    @Bean
    public WebServerFactoryCustomizer tomcatCustomizer() {
        return factory -> {
            if (factory instanceof TomcatServletWebServerFactory) {
                ((TomcatServletWebServerFactory) factory)
                        .addConnectorCustomizers(gracefulShutdown());
            }
        };
    }

    private static class GracefulShutdown implements TomcatConnectorCustomizer,
            ApplicationListener<ContextClosedEvent> {

        private static final Logger log = LoggerFactory.getLogger(GracefulShutdown.class);

        private volatile Connector connector;

        @Override
        public void customize(Connector connector) {
            this.connector = connector;
        }

        @Override
        public void onApplicationEvent(ContextClosedEvent event) {
            Executor executor = this.connector.getProtocolHandler().getExecutor();
            if (executor instanceof ThreadPoolExecutor) {
                try {
                    ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor;
                    threadPoolExecutor.shutdown();
                    if (!threadPoolExecutor.awaitTermination(30, TimeUnit.SECONDS)) {
                        log.warn("Tomcat thread pool did not shut down gracefully within "
                                + "30 seconds. Proceeding with forceful shutdown");
                    }
                }
                catch (InterruptedException ex) {
                    Thread.currentThread().interrupt();
                }
            }
        }
    }
arebya commented 5 years ago

There are unnecessary listener works for GracefulShutdown ,as written above , many application contexts can trigger the ContextClosedEvent

MayBing commented 5 years ago

How to do it with netty sever

MayBing commented 5 years ago

I do it with webflux server reactor-undertow ,it does not work.

               <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-webflux</artifactId>
            <exclusions>
                <exclusion>
                    <groupId>org.springframework.boot</groupId>
                    <artifactId>spring-boot-starter-reactor-netty</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-undertow</artifactId>
        </dependency>
      public class UndertowGracefulConfig {
    /**
     * https://github.com/spring-projects/spring-boot/issues/4657
     */
    @Bean
    public GracefulUndertowShutdown gracefulUndertowShutdown() {
        return new GracefulUndertowShutdown();
    }

    @Bean
    public WebServerFactoryCustomizer<UndertowReactiveWebServerFactory> undertowCustomizer(
            GracefulUndertowShutdown gracefulUndertowShutdown) {
        return (factory) -> {
            factory.addDeploymentInfoCustomizers((builder) -> {
                builder.addInitialHandlerChainWrapper(gracefulUndertowShutdown);
            });
        };
    }

    private static class GracefulUndertowShutdown implements ApplicationListener<ContextClosedEvent>, HandlerWrapper {

        private volatile GracefulShutdownHandler handler;

        @Override
        public HttpHandler wrap(HttpHandler handler) {
            this.handler = new GracefulShutdownHandler(handler);
            return this.handler;
        }

        @Override
        public void onApplicationEvent(ContextClosedEvent event) {
            try {
                this.handler.shutdown();
                this.handler.awaitShutdown(30000);
            } catch (InterruptedException ex) {
                Thread.currentThread().interrupt();
            }
        }

    }
}

when shutdown server with 'CTRL +C',exception throw Thread-4] onfigReactiveWebServerApplicationContext : Exception thrown from ApplicationListener handling ContextClosedEvent

java.lang.NullPointerException: null
        at com.example.springboot.demo.config.UndertowGracefulConfig$GracefulUndertowShutdown.onApplicationEvent(UndertowGracefulConfig.java:47) ~[classes!/:na]
        at com.example.springboot.demo.config.UndertowGracefulConfig$GracefulUndertowShutdown.onApplicationEvent(UndertowGracefulConfig.java:1) ~[classes!/:na]
        at org.springframework.context.event.SimpleApplicationEventMulticaster.doInvokeListener(SimpleApplicationEventMulticaster.java:172) ~[spring-context-5.1.3.RELEASE.jar!/:5.1.3.RELEASE]
        at org.springframework.context.event.SimpleApplicationEventMulticaster.invokeListener(SimpleApplicationEventMulticaster.java:165) ~[spring-context-5.1.3.RELEASE.jar!/:5.1.3.RELEASE]
        at org.springframework.context.event.SimpleApplicationEventMulticaster.multicastEvent(SimpleApplicationEventMulticaster.java:139) ~[spring-context-5.1.3.RELEASE.jar!/:5.1.3.RELEASE]
        at org.springframework.context.support.AbstractApplicationContext.publishEvent(AbstractApplicationContext.java:398) ~[spring-context-5.1.3.RELEASE.jar!/:5.1.3.RELEASE]
        at org.springframework.context.support.AbstractApplicationContext.publishEvent(AbstractApplicationContext.java:355) ~[spring-context-5.1.3.RELEASE.jar!/:5.1.3.RELEASE]
        at org.springframework.context.support.AbstractApplicationContext.doClose(AbstractApplicationContext.java:994) ~[spring-context-5.1.3.RELEASE.jar!/:5.1.3.RELEASE]
        at org.springframework.context.support.AbstractApplicationContext$1.run(AbstractApplicationContext.java:931) [spring-context-5.1.3.RELEASE.jar!/:5.1.3.RELEASE]

2018-12-23 11:39:23.724  INFO 11152 --- [       Thread-4] org.quartz.core.QuartzScheduler          : Scheduler clusteredScheduler_$_bing-work1545536356420 paused.
2018-12-23 11:39:23.729  INFO 11152 --- [       Thread-4] o.s.s.quartz.SchedulerFactoryBean        : Shutting down Quartz Scheduler
2018-12-23 11:39:23.731  INFO 11152 --- [       Thread-4] org.quartz.core.QuartzScheduler          : Scheduler clusteredScheduler_$_bing-work1545536356420 shutting down.
2018-12-23 11:39:23.732  INFO 11152 --- [       Thread-4] org.quartz.core.QuartzScheduler          : Scheduler clusteredScheduler_$_bing-work1545536356420 paused.
2018-12-23 11:39:23.733  INFO 11152 --- [       Thread-4] org.quartz.core.QuartzScheduler          : Scheduler clusteredScheduler_$_bing-work1545536356420 shutdown complete.
2018-12-23 11:39:23.735  INFO 11152 --- [       Thread-4] com.zaxxer.hikari.HikariDataSource       : mysql - Shutdown initiated...
2018-12-23 11:39:23.738  INFO 11152 --- [       Thread-4] com.zaxxer.hikari.HikariDataSource       : mysql - Shutdown completed.
2018-12-23 11:39:41.990 ERROR 11152 --- [   XNIO-1 I/O-3] io.undertow.request                      : UT005071: Undertow request failed HttpServerExchange{ GET /long-process request {Postman-Token=[a6556ff1-b008-4eb9-b61a-0d6e78b730ce], Accept=[*/*], Connection=[keep-alive], cache-control=[no-cache], accept-encoding=[gzip, deflate], User-Agent=[PostmanRuntime/7.4.0], Host=[localhost:8080]} response {Connection=[keep-alive], Content-Type=[text/plain;charset=UTF-8], Content-Length=[16], Date=[Sun, 23 Dec 2018 03:39:41 GMT]}}

java.util.concurrent.RejectedExecutionException: XNIO007007: Thread is terminating
        at org.xnio.nio.WorkerThread.execute(WorkerThread.java:590) [xnio-nio-3.3.8.Final.jar!/:3.3.8.Final]
        at io.undertow.server.HttpServerExchange$ReadDispatchChannel.invokeListener(HttpServerExchange.java:2166) ~[undertow-core-2.0.16.Final.jar!/:2.0.16.Final]
        at io.undertow.server.HttpServerExchange$ReadDispatchChannel.runResume(HttpServerExchange.java:2387) ~[undertow-core-2.0.16.Final.jar!/:2.0.16.Final]
        at io.undertow.server.HttpServerExchange.runResumeReadWrite(HttpServerExchange.java:1860) ~[undertow-core-2.0.16.Final.jar!/:2.0.16.Final]
        at io.undertow.server.Connectors.executeRootHandler(Connectors.java:387) ~[undertow-core-2.0.16.Final.jar!/:2.0.16.Final]
        at io.undertow.server.protocol.http.HttpReadListener.handleEventWithNoRunningRequest(HttpReadListener.java:255) [undertow-core-2.0.16.Final.jar!/:2.0.16.Final]
        at io.undertow.server.protocol.http.HttpReadListener.handleEvent(HttpReadListener.java:136) [undertow-core-2.0.16.Final.jar!/:2.0.16.Final]
        at io.undertow.server.protocol.http.HttpOpenListener.handleEvent(HttpOpenListener.java:162) [undertow-core-2.0.16.Final.jar!/:2.0.16.Final]
        at io.undertow.server.protocol.http.HttpOpenListener.handleEvent(HttpOpenListener.java:100) [undertow-core-2.0.16.Final.jar!/:2.0.16.Final]
        at io.undertow.server.protocol.http.HttpOpenListener.handleEvent(HttpOpenListener.java:57) [undertow-core-2.0.16.Final.jar!/:2.0.16.Final]
        at org.xnio.ChannelListeners.invokeChannelListener(ChannelListeners.java:92) [xnio-api-3.3.8.Final.jar!/:3.3.8.Final]
        at org.xnio.ChannelListeners$10.handleEvent(ChannelListeners.java:291) [xnio-api-3.3.8.Final.jar!/:3.3.8.Final]
        at org.xnio.ChannelListeners$10.handleEvent(ChannelListeners.java:286) [xnio-api-3.3.8.Final.jar!/:3.3.8.Final]
        at org.xnio.ChannelListeners.invokeChannelListener(ChannelListeners.java:92) [xnio-api-3.3.8.Final.jar!/:3.3.8.Final]
        at org.xnio.nio.QueuedNioTcpServer$1.run(QueuedNioTcpServer.java:129) [xnio-nio-3.3.8.Final.jar!/:3.3.8.Final]
        at org.xnio.nio.WorkerThread.safeRun(WorkerThread.java:582) [xnio-nio-3.3.8.Final.jar!/:3.3.8.Final]
        at org.xnio.nio.WorkerThread.run(WorkerThread.java:466) [xnio-nio-3.3.8.Final.jar!/:3.3.8.Final]
ShannonHickey commented 5 years ago

@wilkinsona (or others), I'm wondering if you can answer something about the spring-boot/Tomcat case. I completely understand the need to stop accepting new connections [i.e. connector.pause()] before allowing the application context to be stopped. But what is the significance of waiting for the tomcat thread pool to shut down? How necessary is it. Assume a spring-boot/Tomcat service that does nothing to implement graceful shutdown. Ignoring new connection attempts, does a SIGTERM result in the application context being stopped even before in-flight requests are finished being handled? It surprised me a little to learn that that we need to takes steps to prevent new connections during shutdown. But it would surprise me a great deal more to think that spring-boot shutdown doesn't even deal properly with in-flight requests. Thanks for your help!

wilkinsona commented 5 years ago

As explained above, the application context is stopped and then the embedded container is closed. This means that the context may be closed while requires are in-flight. When Tomcat is the embedded container, it will, by default, wait for 5 seconds for in-flight requests to complete before it closes the connection. If you don't want the application context to be closed until in-flight requests have completed, the example above shows roughly what needs to be done.

ShannonHickey commented 5 years ago

@wilkinsona thanks for the quick response! How unfortunate. I really look forward to a solution arriving within spring-boot. In the meantime, I hope you don't mind one more question: we have found that with async requests, the protocol handler's executor might be empty even in the face of in-flight requests. To deal with this we first wait for the protocol handler's connection count to drop to 0 and then follow with the same logic you've used. Any thoughts on this addition?

wilkinsona commented 5 years ago

That's where things start getting tricky, and is one reason why we've yet to implement something.

Tomcat will timeout all async requests by calling timeoutAsync(-1) on the endpoint's underlying processor. You may want to consider doing something similar or you may want to wait a while for them to complete.

There's also keep-alive connections to consider. They could prevent the connection count from ever dropping to 0 and you'd end up having to close the socket to drop the connections. I'd like to investigate the possibility of causing the container to send Connection: close in the response and then close the connection once graceful shutdown has begun. Unfortunately, I've yet to have time to do so across all the containers that we support.

ShannonHickey commented 5 years ago

@wilkinsona what a tangled mess :-)

For what it's worth, I did some testing on keep-alive and it appears that after the pause(), any subsequent request on a keep-alive connection results in a close. Of course, that doesn't help if there are no subsequent requests on the keep-alive connection.

Duncan-tree-zhou commented 5 years ago

@wilkinsona could you update example for #4657 (comment) with code for Spring Boot 2?

it seems the relevant classes have been renamed and moved to different packages in Spring boot 2.

the following code work for me in spring-boot-starter-web -> 2.1.3.RELEASE with the default embedded container tomcat


    @Bean
    public GracefulShutdownTomcat gracefulShutdownTomcat() {
        return new GracefulShutdownTomcat();
    }

    @Bean
    public ServletWebServerFactory servletContainer(GracefulShutdownTomcat gracefulShutdownTomcat) {
        TomcatServletWebServerFactory tomcat = new TomcatServletWebServerFactory();
        tomcat.addConnectorCustomizers(gracefulShutdownTomcat);
        return tomcat;
    }

    private static class GracefulShutdownTomcat implements TomcatConnectorCustomizer,
            ApplicationListener<ContextClosedEvent> {

        private static final Logger log = LoggerFactory.getLogger(GracefulShutdownTomcat.class);

        private volatile Connector connector;

        @Override
        public void customize(Connector connector) {
            this.connector = connector;
        }

        long timeWait = 30;

        @Override
        public void onApplicationEvent(ContextClosedEvent event) {
            this.connector.pause();
            Executor executor = this.connector.getProtocolHandler().getExecutor();
            if (executor instanceof ThreadPoolExecutor) {
                try {
                    ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor;
                    threadPoolExecutor.shutdown();
                    if (!threadPoolExecutor.awaitTermination(timeWait, TimeUnit.SECONDS)) {
                        log.warn("Tomcat thread pool did not shut down gracefully within " + timeWait + " seconds. Proceeding with forceful shutdown");
                    }
                }
                catch (InterruptedException ex) {
                    Thread.currentThread().interrupt();
                }
            }
        }

    }
codingman1990 commented 5 years ago

@wilkinsona I did what you said before.when connector.pause, any new request will be stocked but not fail fast.Is there any method that can make new request fail fast?

wilkinsona commented 5 years ago

You can either leave the connector running and reject the requests in a filter by returning a 5xx response. Alternatively you could stop the connector rather than pausing it, but that will prevent in-flight requests from completing.

codingman1990 commented 5 years ago

You can either leave the connector running and reject the requests in a filter by returning a 5xx response. Alternatively you could stop the connector rather than pausing it, but that will prevent in-flight requests from completing.

Thanks for your answer!I think returning a 5xx response isn't a good choice,it's just a feak pause.But i asked my PHP worker, swoole can really graceful shutdown progress with new request fail fast.But we still don't know how does swoole works.

czjxy881 commented 5 years ago

How to do it with netty sever

czjxy881 commented 5 years ago

In spring boot 2.1.5 with reactor-netty 0.8.8, I found the netty graceful shutdown failed because of the LoopResource just subscibe the disposeLater instead of block it.

//reactor.netty.resources.LoopResources#dispose
@Override
default void dispose() {
    //noop default
    disposeLater().subscribe();
}
gaodengpan commented 5 years ago

Is there any method that can make new request fail fast?

@codingman1990 You can try closing the ServerSocket after pause the connector. connector.getProtocolHandler().closeServerSocketGraceful();

dd4yd commented 5 years ago

@wilkinsona I did what you said before.when connector.pause, any new request will be stocked but not fail fast.Is there any method that can make new request fail fast?

You can either leave the connector running and reject the requests in a filter by returning a 5xx response. Alternatively you could stop the connector rather than pausing it, but that will prevent in-flight requests from completing.

Hello, this post has been extremely helpful, I just have one question. I'm following your walk through, but how/ where do I return a 5xx response? I tried adding a ResponseStatus annotation with a 5xx HttpStatus to the onApplicationEvent method, although I can still make requests, hence its not properly returning a 5xx status

ahw9995 commented 5 years ago

Hello everyone. I'm not good at English so I hope you understand it.

When using tomcat on Spring Boot, connector.pause() didn't seem to work as I wanted. What I wanted was a situation where kill would start throwing an error immediately when a request came to the server.

So I directly touched protocolHandler like below. When I did this I confirmed that it worked as I wanted. But I'd like to ask for your opinion that this code will cause side effects.

Please comment.

public class GracefulShutdown implements TomcatConnectorCustomizer, ApplicationListener<ContextClosedEvent> {

  private volatile  Connector connector;
  private static final String protocolHandlerClassName = "org.apache.coyote.http11.Http11NioProtocol";

  @Override
  public void customize(Connector connector) {
    this.connector = connector;
  }

  @Override
  public void onApplicationEvent(ContextClosedEvent contextClosedEvent) {

//    this.connector.pause();
    this.protocolHandlerClose();

    Executor executor = this.connector.getProtocolHandler().getExecutor();

    if (executor instanceof ThreadPoolExecutor) {
      ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor;

      threadPoolExecutor.shutdown();

      try {
        if (!threadPoolExecutor.awaitTermination(60, TimeUnit.SECONDS)) {
          threadPoolExecutor.shutdownNow();
        }
      } catch (InterruptedException e) {
        e.printStackTrace();
      }
    }
  }

  private void protocolHandlerClose() {
    try {
      Class<?> clazz = Class.forName(protocolHandlerClassName);
      ProtocolHandler p = (ProtocolHandler) clazz.getConstructor().newInstance();
      p.pause();

    } catch (Exception e) {
      e.printStackTrace();
    }
  }
}
pradeepanchan commented 4 years ago

Hi,

Thanks for the code references. I was able to make my application stop accepting new requests and wait for threads to complete processing.

However, we deploy our services on Kubernetes, and I also want that the HTTP-requests on management port are also rejected, so the the readiness probes from Kubernetes to my service fails and it stops routing traffic to the pod on which my service is running.

I tried going through the Class ManagementWebServerFactoryCustomizer and make a object out of it. But my service shuts down with the error that the management port is already in use.

Any hints on how to go about getting this requirement fulfilled ?

bclozel commented 4 years ago

@pradeepanchan if the goal is to prevent the application from getting new requests, then k8s stopping routing traffic to the application is a good thing? The alternative would be that k8s routes traffic to the instance but it is refusing connections. Maybe I misunderstood the actual requirement?

pradeepanchan commented 4 years ago

@bclozel , the problem is that the pausing server.port or shutting down the ThreadPoolExecutor only ensures that requests going to the main port is paused or rejected. Requests sent to the management port (management.server.port) at actuator/health keep getting an "UP" status, until the service shuts down completely. Because of this, k8s will keep routing requests to this instance which is shutting down and the clients will keep noticing a failure (which i feel is unnecessary). I think the better option would be that the k8s will route the requests to the alternative pod which is still running.

The only way to get this done is that the instance shutting down starts failing the requests arriving at the management.server.port.

One workaround is to combine the management.server.port and server.port to be the same, but I don't think the architects in my organization would agree to that.

wilkinsona commented 4 years ago

That's another example of the complexity of graceful shutdown. The behaviour that you want typically depends on the intelligence of the routing mechanism that's sitting in front of the app.

When using Kubernetes and HTTP endpoints for liveness and readiness checks, the readiness check needs to start failing when you initiate the graceful shutdown but the liveness check should continue to succeed, either until the app has stopped itself or until you're reading for Kubernetes to kill the pod.

If you are running the application and the management server on the same port, you don't want to stop accepting requests when graceful shutdown is initiated as the liveness check would fail. Instead, you want the readiness probe to start failing so that Kubernetes no longer routes traffic to the pod. Once the graceful shutdown has completed or a timeout period has elapsed, the liveness check can then start to fail so that the pod is killed. This is something that we hope to support out of the box in the future. At the moment I would recommend using a custom Actuator endpoint to achieve this behaviour.

If your application is using separate ports for the management server and the application then you could stop the application from handling requests and allow the management server to continue to do so. You would still need a custom health indicator to cause the application to appear alive but not ready until the graceful shutdown has completed or taken too long.

kamkie commented 4 years ago

there are 2 project that can help with that:

there are not ideal but with appropriate configuration of health check groups (spring boot 2.2) it should cover most cases

pradeepanchan commented 4 years ago

That's another example of the complexity of graceful shutdown. The behaviour that you want typically depends on the intelligence of the routing mechanism that's sitting in front of the app.

When using Kubernetes and HTTP endpoints for liveness and readiness checks, the readiness check needs to start failing when you initiate the graceful shutdown but the liveness check should continue to succeed, either until the app has stopped itself or until you're reading for Kubernetes to kill the pod.

You are right about this. But we can take care of it, by ensuring that the frequency and failure threshold of the liveness probe is larger than that of the shutdown wait time. And those of the readiness probe is shorter than that of the shutdown wait time.

pradeepanchan commented 4 years ago

there are 2 project that can help with that:

* https://github.com/timpeeters/spring-boot-graceful-shutdown

* https://github.com/SchweizerischeBundesbahnen/springboot-graceful-shutdown

there are not ideal but with appropriate configuration of health check groups (spring boot 2.2) it should cover most cases

Thanks @kamkie , i got the hint :-)

wilkinsona commented 4 years ago

When using Kubernetes and HTTP endpoints for liveness and readiness checks, the readiness check needs to start failing when you initiate the graceful shutdown but the liveness check should continue to succeed, either until the app has stopped itself or until you're reading for Kubernetes to kill the pod.

I now believe this is incorrect.

When Kubernetes is taking a pod out of service it will send the SIGTERM signal to it. This will cause a Spring Boot application to start shutting down. At this point, the liveness and readiness probes are no longer called.

In parallel, Kubernetes will also initiate the sequence of events that results in requests no longer being routed to the application. The parallel and eventual nature of this means that there is a period of time when the application has received SIGTERM but requests are still routed to it. Ideally, these requests should be handled so that clients do not become aware of the application being shut down.

The recommended way to minimise or perhaps even eliminate the window where requests are still received after SIGTERM is to sleep in a pre-stop hook. In the event of the window only being minimised, continuing to accept new requests during graceful shutdown should reduce the number of errors that are seen by clients. Such errors cannot be eliminated entirely as, if there are no active requests, the graceful shutdown may complete and stop accepting new connections while Kubernetes contains to route requests to the application.

We may need to provide a configurable strategy for graceful shutdown. One that rejects new connections as soon as graceful shutdown begins and one that accepts them. The former could be used in environments where the routing change is guaranteed to happen before graceful shutdown begins and the latter in environments like Kubernetes where the routing change and graceful shutdown run in parallel.

wilkinsona commented 4 years ago

We discussed the need for different strategies for handling requests during graceful shutdown, and decided not to do anything at this time. As explained above, it is impossible to completely close the window on the application's side so it is hard to justify adding complexity that attempts to do so, particularly for a problem that is particular to Kubernetes. In that deployment environment, our recommendation will be to sleep for a period in a pre-stop hook. The length of that period will vary from deployment to deployment, depending on the needs of the application and the time it takes for Kubernetes to propagate the routing change.

wilkinsona commented 4 years ago

https://github.com/spring-projects/spring-boot/commit/308e1d36759db12f3e49fa158514e6d0c8fd66ed adds built-in support for gracefully shutting down Jetty, Tomcat, Reactor Netty, and Undertow. Support is provided for both reactive and Servlet-based web applications and can be enabled by configuring the server.shutdown.grace-period property. The exact behaviour during graceful shutdown varies a little from container to container. If you are interested in this feature, we'd love to hear your feedback before 2.3 is released. The functionality will be available shortly in 2.3 snapshots and will also be in this week's 2.3.0.M3 release. Please take it for a spin and let us know if it meets your needs.

wilkinsona commented 4 years ago

A snapshot containing the new functionality is now available from https://repo.spring.io/snapshot.

joedj commented 4 years ago

Thanks for working on this @wilkinsona

I find the documentation a bit confusing, specifically "Jetty, Reactor Netty, and Tomcat will stop accepting requests at the network layer."

Most of our applications run on AWS Elastic Container Service (ECS). They are primarily behind two different types of load-balancers:

AWS Application Load Balancer (ALB): With ALB, we can set a Deregistration Delay on the load-balancer. As soon as the deregistration process starts for a container, the load-balancer will not send any new requests to the app - even on existing connections - because ALB is protocol-aware. ECS will wait Deregistration Delay seconds before sending SIGTERM to the application. This new graceful shutdown process in Spring does not really buy us anything new here, because we have already stopped receiving new requests/connections, and have been given time to handle any outstanding in-flight requests even before the application shutdown process begins.

AWS Network Load Balancer (NLB) With NLB, we can set a Deregistration Delay on the load-balancer. As soon as the deregistration process starts for a container, the load-balancer will not send any new connections to the app, but it will continue to send requests on existing connections. ECS will wait Deregistration Delay seconds before sending SIGTERM to the application. At this point, application shutdown begins. ECS gives the application a configurable amount of time to shut down, which defaults to 30sec, at which point it will send SIGKILL. The app will continue to receive new requests on existing connections until it terminates or closes them. The new graceful shutdown process in Spring could help us here, but only if it gives us the ability to gracefully close these existing connections after processing outstanding in-flight requests.

Right now, we use custom application logic (in a Filter) to discover when the LB deregistration process begins, and we start sending Connection:close after any in-flight requests complete. This gives us the ability to gracefully close connections before the application shutdown process even begins, but is cumbersome to implement, and adds significant delays to the shutdown process for our applications (which, in turn, adds delays to the overall release process).

Framework support for gracefully closing existing connections after in-flight requests complete (once the app receives SIGTERM) would be very helpful in this scenario.

wilkinsona commented 4 years ago

Thanks for taking a look, @joedj.

Do you know what happens for new requests that arrive on existing idle connections, during graceful shutdown?

We're making as much use as possible of each container's built-in capabilities, which often aren't documented or consider the exact behaviour to be an implementation detail and this is one area where the containers vary. For example, Tomcat will close idle connections such that a new request cannot be made on it.

What about new requests that arrive on existing connections after the current in-flight request completes?

I don't believe that any of the containers distinguish between this scenario and the scenario above. As above, the exact behaviour varies from container to container.

Would it be possible to also document this?

There's an awkward balance for us to strike here. The currently described difference (rejected requests at the network layer versus them being accepted and then responded to with a 503) is within our control but, beyond that, the subtleties are container-specific and are often undocumented implementation details. I can only assume that's because the users of each container haven't needed to know the precise details to make use of the feature. If we documented things more precisely we'd be running the risk of our documentation becoming out of date as a container made a change to its behaviour. We could, perhaps, document what we do in terms of API calls as that is within our control.

Do any/all of the implementations send Connection:close after processing an in-flight request, such that the connection is gracefully terminated?

At the time of writing, Tomcat and Undertow do not. When an in-flight request completes successfully after graceful shutdown has begun, both respond with a Connection: keep-alive header. At this point, Tomcat won't deal with any more requests at the HTTP level. Undertow will deal with requests at the HTTP level and will respond with a 503. This response also has a Connection: keep-alive header. I haven't tested Jetty or Reactor Netty.

is this new functionality exposed in such a way that applications could implement this easily themselves

The container will be shutting down gracefully if you've configured server.shutdown.grace-period and the context closed event has been published. You could use this as a signal to set Connection: Close in any responses. That said, you may want to explore pushing this down into the container that you're using by requesting a change in behaviour or to understand why the container does not do this by default before doing so.

This new graceful shutdown process in Spring does not really buy us anything new here, because we have already stopped receiving new requests/connections, and have been given time to handle any outstanding in-flight requests even before the application shutdown process begins.

It sounds like the time that you've been given to handle outstanding in-flight requests is of fixed duration. For example, if you're being given 30 seconds to finish handling in-flight requests, shutdown will take at least 30 seconds even if those requests complete after 1 second. With graceful shutdown, you should be able to configure a deregistration delay of 0 seconds so that SIGTERM is sent as soon as the load-balancer has stopped sending requests to the app. SIGTERM results in the application context being closed. With server.shutdown.grace-period configured, this close processing will wait for up to the grace period for active requests to complete, before proceeding to shut down the app. In the scenario above where the active requests complete after 1 second, this would allow the shutdown to complete in a little over 1 second rather than a little over 30 seconds.

The new graceful shutdown process in Spring could help us here, but only if it gives us the ability to gracefully close these existing connections after processing outstanding in-flight requests.

While Tomcat will not send Connection: close, I believe that it will behave in a way that will work for you out of the box. Undertow, however, will not. If you have an opportunity to try things out in your environment with the container that you chose to use, it'd be great to know whether or not this is indeed the case.

Right now, we use custom application logic (in a Filter) to discover when the LB deregistration process begins, and we start sending Connection:close after any in-flight requests complete. This gives us the ability to gracefully close connections before the application shutdown process even begins, but is cumbersome to implement, and adds significant delays to the shutdown process for our applications (which, in turn, adds delays to the overall release process).

I'd like to understand why sending Connection:close adds significant delays to the shutdown process. Can you expand a bit on that please?

joedj commented 4 years ago

Hey @wilkinsona, thanks for the response.

I'll just answer your last question, for now, until I hopefully get a chance to play around:

I'd like to understand why sending Connection:close adds significant delays to the shutdown process. Can you expand a bit on that please?

(This is specifically in relation to shutting down gracefully while using NLB, which will continue to send requests on existing connections up until they are closed)

Because we don't currently have a nice way to gracefully shut down the container upon receiving SIGTERM (and some other internal legacy reasons that mean we currently only have a max of 30sec after SIGTERM), we instead perform the graceful connection draining before the actual app shutdown process begins.

To do this, we use the LB Deregistration Delay functionality. For NLB, this controls the duration between when the app will stop receiving new TCP connections, and when the app will receive SIGTERM.

For our app to discover that it has changed state in the LB from "healthy" to "draining" (signalling that the Deregistration Delay period has started), it polls the AWS APIs. This happens once per minute - any more frequent than that, and we often hit API rate limits. So, the Deregistration Delay needs to be at least 1min or so, for us to be sure we have even seen the state change.

Once we notice the state change, there are additional delays:

...but those additional delays also exist if we start draining in response to SIGTERM, rather than in response to an LB state change.

So really, using the Spring support for graceful shutdown in response to SIGTERM would only allow us to save that initial 60sec delay while polling the LB state (edit: and also, as you accurately pointed out in your earlier response, it would also mean that shutdown completes when all the in-flight requests are finished, rather than waiting for the fixed Deregistration Delay period - in practice, this could reduce the container shutdown time from 3min+ to mere seconds (or 60sec, i.e. the idle timeout we need to wait for any new requests on existing connections, assuming the web container we're using supports this and doesn't close those idle connections immediately when graceful shutdown begins).

...and it would also allow us to delete all that supporting code and get rid of the unreliable scheduled polling.

ractive commented 4 years ago

On kubernetes we have a different requirement when shutting down gracefully when doing rolling updates or scaling a deployment down.

According to this blog post and to our own experience you need to wait for a couple of seconds after receiving the SIGTERM from kubernetes before stopping to accept requests to actually still be able to handle stray connections that are coming in. This is because the kubernetes endpoint object of this app has not yet been removed from the kuberenetes service via the endpoint-controller and kube-proxy although the SIGTERM already has been sent by the Kubelet, so requests are still routed to this endpoint. When we stop accepting connections immediately after receiving the SIGTERM (which we already do) we see "connection refused" errors from the clients. So another configuration option we'd like to see is something like server.shutdown.wait-for-seconds that would wait for this amount of seconds before actually pausing any connector and continuing with the shutdown.

wilkinsona commented 4 years ago

Thanks, @ractive. What you've described matches our experience too. Rather than implementing the wait in Spring Boot, our recommendation is to use a pre-stop hook configured to run something like sh -c sleep 10. We have some documentation updates planned that will cover this and more from a Kubernetes perspective.

pradeepanchan commented 4 years ago

When Kubernetes is taking a pod out of service it will send the SIGTERM signal to it. This will cause a Spring Boot application to start shutting down. At this point, the liveness and readiness probes are no longer called.

In parallel, Kubernetes will also initiate the sequence of events that results in requests no longer being routed to the application. The parallel and eventual nature of this means that there is a period of time when the application has received SIGTERM but requests are still routed to it. Ideally, these requests should be handled so that clients do not become aware of the application being shut down.

The recommended way to minimise or perhaps even eliminate the window where requests are still received after SIGTERM is to sleep in a pre-stop hook. In the event of the window only being minimised, continuing to accept new requests during graceful shutdown should reduce the number of errors that are seen by clients. Such errors cannot be eliminated entirely as, if there are no active requests, the graceful shutdown may complete and stop accepting new connections while Kubernetes contains to route requests to the application.

This is correct. Its also documented in the book : Kubernetes in Action (ISBN: 9781617293726), under Chapter 17. Best practices for developing apps.

I think the sleep based configuration would be good enough in our case (infact we did the same thing in our application). I expect that the configuration server.shutdown.grace-period will be well-documented so that users will know what exactly will happen when its configured.

Thanks for taking this up.

gnagy commented 4 years ago

Hi, I have a spring-boot app with HTTP endpoints and Kafka consumers. I have tested this graceful shutdown implementation and it looks like the whole shutdown is delayed waiting for HTTP with Thread.sleep(), so no other components are notified. This means I need 2*n graceful period if I wanted to give HTTP and Kafka n seconds to finish their thing.

@wilkinsona I saw your comment about not needing SmartLifecycle just for tomcat, but have you considered other components needing graceful shutdown as well? In my case, I would like to leave some time for both @Controllers and Kafka Listeners to be able to finish already started processing -- which in some cases can make external calls with exponential backoff, so I would also like to be able to check in retry loops if shutdown was initiated.

I'm thinking about such logic:

Would you consider changing the above logic, or recommend a custom implementation instead?