search

Netflix / Hystrix

Hystrix is a latency and fault tolerance library designed to isolate points of access to remote systems, services and 3rd party libraries, stop cascading failure and enable resilience in complex distributed systems where failure is inevitable.
24.07k stars 4.7k forks source link

threadPoolProperties ignored in GenericSetterBuilder#buildObservableCommandSetter #1611

Open ghostganz opened 7 years ago

ghostganz commented 7 years ago

Using HystrixCommand annotation on a method returning Observable, the "threadPoolProperties" are ignored.

https://github.com/Netflix/Hystrix/blob/master/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/GenericSetterBuilder.java

It's because the build() method (for non-Observables) copies those settings, but they are ignored in the buildObservableCommandSetter(). There's a TODO there hinting at the problem.

Most users won't notice this, since the isolation strategy doesn't default to THREAD for observables.

ghostganz commented 7 years ago

More of the same problem: HystrixObservableCommand.Setter has protected fields threadPoolKey and threadPoolPropertiesDefaults, but there's no way to change them.

ramnight commented 7 years ago

Is that a bug? Or is there any other reason? I want to use the Observable in THREAD mode, but I can't change the thread pool config。

mattrjacobs commented 7 years ago

I can't think of any reason you would use a thread pool with HystrixObservableCommand. There's an outstanding issue to deprecate and remove this API (#1200), but that hasn't been done yet.

ramnight commented 7 years ago

I found solution in https://github.com/Netflix/Hystrix/issues/805#issuecomment-109371037, thanks a lot.

ghostganz commented 7 years ago

We have, for historical reasons, things that use Observable although they aren't completely asynchronous, so we need threads to have those things running well.

In an ideal world that wouldn't be needed, but ...