Fixes #2622 clear some state during reload #2627
Conversation
After a reload we need to clear out old ThreadLocal data, and Hystrix is also caching some things in statics
|
@dmlloyd any chance you could review this one? |
| * | ||
| * This is only for development mode, it must not be used for production applications. | ||
| * | ||
| * TODO: should this just provide a facacde that simply starts a new thread pool instead? |
There was a problem hiding this comment.
Maybe that would be a bit cleaner: just start a new pool, swap it in, and shut down the old one.
There was a problem hiding this comment.
you would need to also re-dispatch the current HTTP thread to the new pool, to prevent old context from hanging around.
There was a problem hiding this comment.
This approach works for now, this approach was listed as a todo because it would need a lot more testing (which is mostly manual testing at the moment as we do not have a good way to test dev mode).
This is a very low risk approach as it is basically the status quo with some fixes. Changing the behaviour of dev mode and hot reload has much more potential for regressions, which is why I left it as a TODO item.
| } | ||
|
|
||
| /** | ||
| * copied from JBoss threads |
There was a problem hiding this comment.
Maybe there should be an API for this.
There was a problem hiding this comment.
its all package private
| HystrixThreadPoolCompletionStream.reset(); | ||
| HystrixCommandStartStream.reset(); | ||
| HystrixThreadEventStream.getInstance().shutdown(); | ||
| Hystrix.reset(); |
There was a problem hiding this comment.
Hystrix.reset() should not be necessary as we call it when the CDI app context is destroyed - https://github.com/smallrye/smallrye-fault-tolerance/blob/master/implementation/src/main/java/io/smallrye/faulttolerance/HystrixInitializer.java#L64. I'm not sure about the streams though. But better safe than sorry.
There was a problem hiding this comment.
Let's go for the safe approach first and revisit that later.
@stuartwdouglas could you open an issue drafting the approach we should follow in the future. Better that than having a comment buried in the code.
After a reload we need to clear out old ThreadLocal data,
and Hystrix is also caching some things in statics