HystrixMetricsStreamServlet.destroy() not invoked until container timeout

[codependent]

Scenario:

1. JEE App with hystrix + hystrix-metrix-event-stream (v1.3.18).
2. Hystrix dashboard requesting data to the App.
3. When we stop App in our server we see that the following loop doesn't stop:

HystrixMetricsStreamServlet.handleRequest(){
...  
   while (poller.isRunning() && !isDestroyed) {

The servlet's destroy() method is not called, thus isDestroyed is not modified until timeout. This makes sense since, according to the servlet spec: void destroy() - Called by [...] This method is only called once all threads within the servlet's service method have exited or after a timeout period has passed.

The problem is that since there is a running request, whenever we want to republish/stop our application, we have to wait 60 seconds until our container (Websphere Application Server 8) forces the destruction of the servlet.

[benjchristensen]

What is your suggested solution?

I personally have not used a deployment model in a very long time that depends on graceful shutdown. We always kill the entire container/app and restart cleanly. This is a good example of why.

[codependent]

Lucky you ;-) Unfortunately we have to work with WAS, which runs several other applications so shutting down the server for each deployment is not a possibility.

I have been thinking about it but cannot come up with a solution, Just was wondering if anyone could suggest a workaround...

[jboyd01]

I hit the same issue with WebSphere. I meant to submit as a pull request but welcome review here. I added a static shutdown() method to HystrixMetricsStreamServlet and call it from one of my other servlets that only implements init() and destroy().

Also note the extra check required after the flush to detect when a polling client has disconnected. WebSphere buffers the response and from what I saw, the servlet never stops writing to the output stream even 20 minutes after the client disconnected.

diff HystrixMetricsStreamServlet.java HystrixMetricsStreamServlet.java.orig
66,79c66
<     private static volatile boolean isDestroyed = false;
<
<     /**
<      * hack added to support WebSphere.  WAS won't shutdown a servlet until after a 60 second timeout if there
<      * is an instance of the servlet executing a request.  Add this method to enable a hook to notify us to shutdown.
<      */
<     public static void shutdown() {
<       isDestroyed = true;
<     }
<
<     @Override
<     public void init() throws ServletException {
<       isDestroyed = false;
<     }
---
>     private volatile boolean isDestroyed = false;
162,166d148
<                         // explicitly check for client disconnect - PrintWriter does not throw exceptions
<                         if (response.getWriter().checkError()) {
<                               throw new IOException("io error");
<                         }
<

[codependent]

@jboyd01 I just tested it and works like a charm, thanks for sharing. I'll wait for your pull request to get it integrated into hystrix.

[mattrjacobs]

@jboyd01 I don't have a great way of testing this, as I'm not familiar with Websphere or WAS. Are you (or anyone else) still interested in submitting a pull request?

[mattrjacobs]

Moving this out of RC6. I'm happy to merge in a PR from someone who experiences this issue, but don't have enough context to work on it myself.

[codependent]

@mattrjacobs I could (and will) submit jboyd01's solution as a PR. We are planning to implement Hystrix-enabled services in the next months and will definitely need that workaround.

[mattrjacobs]

Great, thanks @codependent!

[lasselasse]

We also just hit this issue running the metrics stream servlet from within embedded Jetty.
Very excited to see this being addressed soon!

[mattrjacobs]

@codependent / @lasselasse I just merged this into master. Are you able to test @jboyd01 's solution currently or do you need an official RC release?

[codependent]

@mattrjacobs I don't really need a RC thanks. I'll test it tomorrow (out of the office right now) and let you know if it's OK.

[mattrjacobs]

Great, thanks @codependent

[codependent]

@mattrjacobs I just tested it, and it works OK. Thanks to @jboyd01 for this workaround.

[lasselasse]

@mattrjacobs tested the patch and it works great, thanks!

May I suggest the doGet method to be modified as follows so the servlet will directly reject requests once it has been shutdown? (At the moment it would still start a new HystrixMetricsPoller only to shut it down again.)

    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
        if (isDestroyed) {
            response.sendError(503, "Service is shut down.");
        } else {
            handleRequest(request, response);
        }
    }

[codependent]

I'd like to point out one important thing: jboyd01 suggested using another servlet's destroy() to invoke HystrixMetricsStreamServlet.shutdown() but this won't work under Websphere.

The reason is that WS doesn't invoke any servlet.destroy() until all requests have finished. And yet, it does destroy JSPs (which, like everybody knows, are a kind of servlet, WTH?).

The only way I found to invoke HystrixMetricsStreamServlet.shutdown() is through a startup bean EJB. As simple as this:

package com.whatever;
import javax.annotation.PostConstruct;
import javax.annotation.PreDestroy;
import javax.ejb.Singleton;
import javax.ejb.Startup;

import com.netflix.hystrix.Hystrix;
import com.netflix.hystrix.contrib.metrics.eventstream.HystrixMetricsStreamServlet;

@Singleton
@Startup
public class HystrixStopper {

    @PreDestroy
    public void preDestroy(){
        //Stops the metrics servlet
        HystrixMetricsStreamServlet.shutdown();
        //Shuts down all threads started by Hystrix
        Hystrix.reset();
    }
}

[lasselasse]

Since we run a Spring application I'm triggering the shutdown from a ContextClosedEvent-listener which is invoked before the embedded Jetty is shutdown.

import com.netflix.hystrix.contrib.metrics.eventstream.HystrixMetricsStreamServlet;
import org.springframework.context.ApplicationListener;
import org.springframework.context.event.ContextClosedEvent;

public class ApplicationContextShutdownHandler implements ApplicationListener<ContextClosedEvent> {

    @Override
    public void onApplicationEvent(final ContextClosedEvent event) {
        HystrixMetricsStreamServlet.shutdown();
    }

}

[jboyd01]

@codependent I'm late replying, but wanted to note that I'm successfully exercising the shutdown of the stream servlet from a 2nd servlet under WebSphere 8.5.5.2 ND. I have a HystrixControl servlet with just two methods:

    public void destroy() {
        HystrixMetricsStreamServlet.shutdown();
        Hystrix.reset();
    }

    public void init(){
        // invoke code to set configuration source & register concurrency strategy
    }

and its registered via web.xml:

    <servlet>
        <servlet-name>HystrixControl</servlet-name>
        <servlet-class>
            com.acme.projectx.init.HystrixControl
        </servlet-class>
        <load-on-startup>2</load-on-startup>
    </servlet>

I exercise my application, then hit the hystrix.stream with curl and let it stream away. When I stop the application via admin console I get immediate shutdown - - the streaming stops and WebSphere reports the app as stopped in the logs within seconds of the stop attempt.

I do like the options of doing this via an EJB and ContextClosedEvent listener. I think it may be wise to start a doc page that details App Server specific implementation details.

[codependent]

@jboyd01 thanks for the feedback :-) Right now it doesn't work for us but it must be something related to the container so I'll take it into account when we move from 8.0.x.x to 8.5.

[mattrjacobs]

@codependent I'm inclined to close this issue and let you open a new issue with details specific to your case. Does that sound OK to you?

[codependent]

@mattrjacobs It's alright to me.

[dsukner]

#530 does not seem to have fixed the client disconnect detection on WebSphere