-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Version 1.3 - RxJava Observable Integration #151
Version 1.3 - RxJava Observable Integration #151
Conversation
#123 - refactored HystrixCommand to use Observable inside the implementation to be non-blocking and callback driven - observe() and toObservable() public methods added - HystrixCollapser not yet changed
- pick up concurrency fix to ScheduledObserver ReactiveX/RxJava#269
- it is relying on scheduling of the collapser timer - this is an example so specifically should not be refactored to use artifical time
…ead while other threads working of the Cached wrapper are already waiting on the result
Conflicts: gradle.properties hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java
Hystrix-pull-requests #22 ABORTED |
After running on a production canary for a couple days a deadlock occurred, so I've introduced a concurrency bug somewhere. Thus it's going to be a while longer before this is ready ... I'm going to refactor the Collapser code as it has become somewhat unwieldy and difficult to reason through. |
I need to look at how an Observable handles multiple subscriptions when requestCache is disabled. Right now |
…plify reasoning … done while hunting down concurrency bug ...
A short-cut return was before the try/finally. With thread interleaving one thread could get the readLock and never release it and then the execute/shutdown methods would block indefinitely trying to get the writeBlock. I don't know for sure that this was the bug I've been hunting but the symptoms that could occur from this exactly match what happened.
Hystrix-pull-requests #26 ABORTED |
- change logger.error to logger.debug on noisy logs - improve wording on message where a command has failed and the fallback failed - logger.error is not needed since everywhere that it would be valuable throws a HystrixRuntimeException which will be handled or logged elsewhere
Hystrix-pull-requests #27 ABORTED |
This functionality of aggressive starts from a blocking request has removed for now since we've moved to a completely async model. If testing shows we still need/want this for performance optimizations we can add it back in.
Hystrix-pull-requests #28 FAILURE |
Conflicts: gradle.properties
Hystrix-pull-requests #29 ABORTED |
- added Javadoc to formalize this contract (it was always there) - reverted back to 1.2 behavior where IllegalStateException is thrown directly, not wrapped inside a HystrixRuntimeException
Hystrix-pull-requests #30 FAILURE |
Hystrix-pull-requests #31 FAILURE |
Hystrix-pull-requests #33 FAILURE |
Testing has looked good in production canaries for the past 2 days so merging this into trunk. Will then proceed with a larger scale canary test in Netflix production with binaries created through the formal build process. I will not release that to Maven Central until I have validated further. |
Version 1.3 - RxJava Observable Integration
This pull request completes issue #123 "Support Asynchronous Callbacks with RxJava Integration"
Async execution can now be done with
observe()
and it will callback when the value is received:Here is the simplest possible example of subscribing to the value (using a lambda instead of anonymous inner class in this example):
More can be learned about RxJava and the composition features at https://github.com/Netflix/RxJava/wiki
This pull request is being canary tested at Netflix and will not be merged or released until we are comfortable with deploying this to our full production traffic. Since this is a non-trivial change I will probably leave it in our canary environment for around a week before proceeding (I have already done several smaller canary tests at various development stages). If any issues are found this pull request will be updated with the fix.
I am posting this pull request before our testing is complete to give everyone a heads up and an opportunity to provide feedback.