Eliminate thread-isolated HystrixObservableCommands

[mattrjacobs]

After writing/debugging a few HystrixObservableCommands in real systems, I've come to the conclusion that we should not support thread-isolation for HystrixObservableCommands. Here's why:

In a HystrixCommand, Hystrix is responsible for turning a function returning T into Observable<T>. Hystrix will call subscribeOn on this Observable to move it to a Hystrix thread if the command is thread-isolated.

In a HystrixObservableCommand, Hystrix is given a function that already returns an Observable<T>. Hystrix doesn't know anything about where the source observable runs. It might already have a subscribeOn in its observable chain, in which case any further subscribeOns will be no-ops. This implies that Hystrix can not always perform thread-isolation.

Instead, we should trust that the user has already decided where the source observable should run, and apply all of the Hystrix logic to that thread. This is semaphore-isolation, and it's, IMO, the only reasonable choice for HystrixObservableCommands.

This will change some semantics, so I want to make this 1.6.0.

[yanglifan]

So in Hystrix 1.6, the execution in a HystrixObservableCommand will only be semaphore isolated?

[mattrjacobs]

I haven't made this change yet, so I don't know in which version this will land. I'm planning on spending some time rounding up all the current/planned deprecations and mapping the deprecation/deletions to releases. Will ping this thread when I have that.

[swapnilgawade16]

I am evaluating hystrix for my new project. Correct me if I am wrong but it seems I have a requirement where in I would require thread isolation for hystericObservableCommand. We would be using vertx framework along with hystrix. Vertx is non blocking async framework but also support blocking execution. So we would be using mix of both blocking and non blocking code. As part of wrapping a service call in hystrix we would have some blocking code as well as some non blocking in same service. Example would be some blocking code processing using legacy java library then send a message using vertx event bus and pass handler for getting reply that is async/ non blocking and when reply is received service execution is complete. In such case won't it be good to use hystrixObservableCommand with thread isolation as my service would have both blocking and non blocking code. Do let me know if what ever I have thought of make sence ?

[mattrjacobs]

If you have blocking I/O, use a thread-isolated HystrixCommand. If you have nonblocking I/O, use a semaphore-isolated HystrixObservableCommand.

Any concerns with that approach?

[agentgt]

I very much agree with the proposed approach particularly with #1103 in mind.

It is extremely confusing to users (for example #1106 ) as they don't know if they are controlling the onSubscribe thread or the thread publishing items and what Hystrix is circuit breaking on as well as timing out on.

Of course for major breaking versions I would recommend removing HystrixObservableCommand all together and have the conversion/wrapping to Future/Promise/Observable be pluggable and not part of the core (e.g. HystrixListenableFutureCommand for those preferring Guava).

Perhaps even making the HystrixCommand have a method that takes some sort parameter to convert it ie HystrixCommand.deferTo(Observable.class) -> Observable. That is any HystrixCommand can be translated to another Observable-like-type through a plugin system. I know this works because I do this for my own companies message bus library.

[aknrdureegaesr]

Just a remark:

The documentation at https://github.com/Netflix/Hystrix/wiki/How-To-Use has this sample code:

    @Override
    protected Observable<String> construct() {
        return Observable.create(new Observable.OnSubscribe<String>() {
            @Override
            public void call(Subscriber<? super String> observer) {
                try {
                    if (!observer.isUnsubscribed()) {
                        // a real example would do work like a network call here
                        observer.onNext("Hello");
                        observer.onNext(name + "!");
                        observer.onCompleted();
                    }
                } catch (Exception e) {
                    observer.onError(e);
                }
            }
         } );
    }

That example code invites you to do blocking on onSubscribe, which one should not do without a thread pool backing the subscription process. It goes against the grain of what is proposed here.

I'm not against what is proposed here. But it seems to me that documentation should be changed (preferably soon). What do you think?

[mattrjacobs]

@aknrdureegaesr Yes, you're right - this would not be a good idea. I'll update the docs

[mattrjacobs]

I just updated the example to make the Observable returned by construct() get subscribed on an Rx IO thread. Do you think that's more clear?

[aknrdureegaesr]

Do you think that's more clear?

Slightly so. But it's not as clear as it could be and easy to overlook.

In my humble opinion, the entire user guide needs a rewrite.

In particular, in https://github.com/Netflix/Hystrix/wiki/How-To-Use :

When you wrap behavior that does not perform network access, but where latency is a concern or the threading overhead is unacceptable, you can set the executionIsolationStrategy property to ExecutionIsolationStrategy.SEMAPHORE and Hystrix will use semaphore isolation instead.

Also, in https://github.com/Netflix/Hystrix/wiki/Configuration#execution.isolation.strategy :

Thread or Semaphore

The default, and the recommended setting, is to run commands using thread isolation (THREAD).

Commands executed in threads have an extra layer of protection against latencies beyond what network timeouts can offer.

Generally the only time you should use semaphore isolation (SEMAPHORE) is when the call is so high volume (hundreds per second, per instance) that the overhead of separate threads is too high; this typically only applies to non-network calls.

FWIW: In our project, we use org.apache.http.impl.nio.client.CloseableHttpAsyncClient. In our opinion (and experience thus far), that combines well with RxJava, HystrixObservableCommand, and semaphore isolation for the latter.

More conceptually speaking, the present documentation:

• treats semaphore isolation as a second class citizen and advises against it for common use-cases, but
• treats HystrixObservableCommand and HystrixCommand as being on the same footing.

Taking this issue into account, one of those two should be revised.

My personal opinion is, the advice against semaphore isolation should be rewritten.

[agentgt]

@aknrdureegaesr

FWIW: In our project, we use org.apache.http.impl.nio.client.CloseableHttpAsyncClient. In our opinion (and experience thus far), that combines well with RxJava, HystrixObservableCommand, and semaphore isolation for the latter.

I have some experience as well with RxJava and Observable and I have to say Observable is so overloaded and extremely confusing. It is like the crappy Katy Perry Hot n Cold song. Seriously read the lyrics to that song basically describes every time I'm dealing with someone elses Observable.

For example in your case are you actually streaming or are you just using the HystrixObservableCommand as an async cold/hot Future (that is do you have multiple onNext calls)? When you call the Hystrix command do you call toObservable() or observe()... there is a massive difference! If you are doing it cold when should the semaphore logic be done (onConnect/onSubscribe or just getting the Observable? Not to mention semaphores technically are blocking to some extent (even the try attempt). Consequently the semaphore anti advice IMO is still sort of sound.

Really the problem is exposing Observable all together. If Hystrix was a streaming library it would have been a great choice but it is not and confusion abounds.

It would greatly simplify everything, scale, and fit most use cases if HystrixCommand was always a memoized cold Single or the analogous memoized cold Reactor Mono but with out all the transformation bundled with those guys. That is expose its own simplified interfaces or just use the Reactive Streams interfaces whenever they pump out a single. Even Guava's ListenableFuture is a better choice if you went with default of always being hot (cold is preferred because you can't go from hot->cold).

[aknrdureegaesr]

We use observe().

We don't presently use HystrixObservableCommand with Observables that emit more than once (not needed for our HTTP calls). But we may do so in the future, should we decide to open up the HystrixObservableCommand umbrella over our database queries, assuming we find and decide to use an nio-based database client library.

To the frustrated tone of your post, what can I say? If Observables confuse you, consider using something else. We are quite happy with them.

[agentgt]

I am ambivalent about them (Observables) for sure and I'm sort of regretting the tone (thanks for noticing though and apologies for my passion 😄 ). It is also fairly off topic so sorry about that as well.

However just to be clear it isn't that Observables necessarily confuse me it is the complexity of variable behavior of how other developers use them and how they will potentially use it incorrectly.

Take your example you have to tell all your developers now not to use toObservable(). But toObservable is actually the correct choice as your suppose to let the endpoint actually do the subscribing (e.g. hand off the Observable to the web framework). Your entire controller or consumer or whatever endpoint code is supposed to be monadically transforming the observable and handing it off.

This is especially concerning because you said you want to do streaming later on of which observe() is clearly the wrong choice.

That being said I generally like the idea of using Observables for streaming and I do generally like RxJava/Reactor it just requires a diligent programmer. The complexity some ways makes a case for using green threads (Quasar) otherwise I feel many people might just give up and switch to Go lang.

[carlosbarragan]

I am coming pretty late to the party but I am also a bit confused by the examples in the documentation. @mattrjacobs you mentioned, you changed the example to make the ObservableCommand subscribe on an IO Thread. As I understand, that is ok if the underlying call is blocking.

That being said, if I have an http call that is non-blocking async (like Spring AsyncRestTemplate) I actually shouldn't need to subscribe to an IO Thread as the call is inherently non-blocking right?