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

Fixed HystrixContextScheduler to conform with RxJava Worker contract #596

Merged
merged 1 commit into from
Feb 2, 2015

Conversation

akarnokd
Copy link
Contributor

@akarnokd akarnokd commented Feb 2, 2015

Fixed issue in #354 by making HystrixContextScheduler conform with RxJava's Worker contract. See also #593.

To avoid code duplication, it uses the rx.internal.schedulers.ScheduledAction. I don't know how close Hystrix follows RxJava releases, but I'll make steps to have ScheduledAction part of the stable RxJava API eventually.

@cloudbees-pull-request-builder

Hystrix-pull-requests #256 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

NetflixOSS » Hystrix » Hystrix-pull-requests #17 SUCCESS
This pull request looks good

@mattrjacobs
Copy link
Contributor

This is great @akarnokd , thanks for the contribution!

The other piece of this issue is that a HystrixCommand has configuration to optionally perform a thread interruption on some unsubscribes. From what I can tell SchedulerAction.FutureCompleter already has a unsubscribe policy where thread interrupts happen if and only if the thread is different from the current. The policy I'm describing for Hystrix would be different from that, and determined at schedule-time.

I'm sure I could whip up a different hack for that - but is there a better way to make the thread-interruption policy more configurable?

@akarnokd
Copy link
Contributor Author

akarnokd commented Feb 2, 2015

I'm currently proposing a change for ScheduledAction in ReactiveX/RxJava#2579 which adds a system-property to enable/disable interruption globally. I'll add some interrupt-support there. But note if you get a ScheduledAction back from a wrapped worker, it is too late.

@mattrjacobs
Copy link
Contributor

I'm merging this in. I'll open up a separate issue for the configurability of the thread-interrupt behavior. Then, depending on how RxJava and Hystrix release cycles line up, I'll likely just use the new behavior in RxJava (https://github.com/ReactiveX/RxJava/pull/2579/files).

mattrjacobs added a commit that referenced this pull request Feb 2, 2015
Fixed HystrixContextScheduler to conform with RxJava Worker contract
@mattrjacobs mattrjacobs merged commit 1465c0a into Netflix:master Feb 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants