Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hystrix timeout + rx retry() = deadlock #1100

Closed
costimuraru opened this issue Feb 19, 2016 · 2 comments
Closed

Hystrix timeout + rx retry() = deadlock #1100

costimuraru opened this issue Feb 19, 2016 · 2 comments

Comments

@costimuraru
Copy link

We seem to have gotten into a situation where we're stuck in a deadlock. The use case:
Setup:
[] Convert the HystrixCommand into an Observable
[] Wrap this observable with a retry(x times)
When:
[] HystrixCommand times out
Then:
[] Instead of retrying the same command x times, a deadlock occurs.

Below you can see the code that exhibits this deadlock.

io.reactivex:rxjava:1.1.1
com.netflix.hystrix:hystrix-core:1.4.23

public class Command extends HystrixCommand<String> {

    public Command() {
        super(Setter.withGroupKey(asKey("test"))
                .andThreadPoolPropertiesDefaults(
                        HystrixThreadPoolProperties.Setter()
                                .withCoreSize(10)
                )
                .andCommandPropertiesDefaults(
                        HystrixCommandProperties.Setter()
                                .withExecutionTimeoutInMilliseconds(10)
                ));
    }

    @Override
    protected String run() throws Exception {
        Thread.sleep(200);
        return "response";
    }
}
public class CommandWithRetryTest {

    public static void main(String[] Args) {
        String result = Observable.just(1)
                .flatMap(i -> new Command()
                                .toObservable()
                                .retry(2) // This triggers the deadlock.
                )
                .toBlocking()
                .first(); // ===> deadlock
    }
}
@mattrjacobs
Copy link
Contributor

@costimuraru You shouldn't use retry() on an Observable that Hystrix produces. The Hystrix model is that each command object is single-use. Therefore, if a command fails once, it will never produce anything different.

That said, a deadlock is pretty harsh, so I'll see why this is happening and if there's anything that Hystrix can do to produce an Observable that makes this error more tolerable.

@mattrjacobs
Copy link
Contributor

I believe that this got fixed by #1396. The subscribe-after-unsubscribe case was not handled well before. In #1370, it manifested as a memory leak. In this case, as a deadlock. I added a unit test in #1406 to show that this is fixed in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants