Implement destroy() method for graceful shutdown of MetricsStreamServlet #128
Conversation
|
Hystrix-pull-requests #4 SUCCESS |
| @@ -126,8 +138,15 @@ private void handleRequest(HttpServletRequest request, HttpServletResponse respo | |||
| response.getWriter().println("data: " + json + "\n"); | |||
| } | |||
| } | |||
|
|
|||
| /* shortcut breaking out of loop if we have been destroyed */ | |||
| if(isDestroyed) { | |||
There was a problem hiding this comment.
No it's not. I put it there just so destroying wouldn't potentially wait for the Thread.sleep but that's generally 500ms to maybe 2000ms - but someone could theoretically inject a very long delay. That's the only edge case this doesn't gracefully handle (someone putting in a 10000ms delay for example) since we don't interrupt the threads (and we don't track them all) but I don't know that this edge case is a big concern.
Short answer - probably not worth the optimization.
|
This implementation looks almost identical to mine :-). I will try this out probably tomorrow, and report then |
|
Thanks, I'll look for your report back. |
|
Looks good... Problem have disapeared after introduction of this fix. |
|
Great, I'll merge it. Thanks for testing and reporting back. |
…s-servlet Implement destroy() method for graceful shutdown of MetricsStreamServlet
#127
This has NOT been tested but is a possible fix to this issue.
@ogafosir I don't have time to validate this but this is probably what is needed. If you can confirm this works then I can merge it in, or if it doesn't go ahead and implement a valid fix and submit a new pull request.