Skip to content

Commit

Permalink
Only create the HystrixContextRunnable for running the timeout fallba…
Browse files Browse the repository at this point in the history
…ck when the timeout actually occurs (not eagerly)
  • Loading branch information
mattrjacobs committed May 8, 2017
1 parent f0d5482 commit 951c6f9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
22 changes: 11 additions & 11 deletions hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -1132,17 +1132,8 @@ public Subscriber<? super R> call(final Subscriber<? super R> child) {
// if the child unsubscribes we unsubscribe our parent as well
child.add(s);

/*
* Define the action to perform on timeout outside of the TimerListener to it can capture the HystrixRequestContext

This comment has been minimized.

Copy link
@whiskeysierra

whiskeysierra May 31, 2018

Before it didn't just capture the HystrixRequestContext but also any other thread-local state that the concurrency strategy propagates. This is broken now.

* of the calling thread which doesn't exist on the Timer thread.
*/
final HystrixContextRunnable timeoutRunnable = new HystrixContextRunnable(originalCommand.concurrencyStrategy, new Runnable() {

@Override
public void run() {
child.onError(new HystrixTimeoutException());
}
});
//capture the HystrixRequestContext upfront so that we can use it in the timeout thread later
final HystrixRequestContext hystrixRequestContext = HystrixRequestContext.getContextForCurrentThread();

TimerListener listener = new TimerListener() {

Expand All @@ -1157,6 +1148,15 @@ public void tick() {
// shut down the original request
s.unsubscribe();

final HystrixContextRunnable timeoutRunnable = new HystrixContextRunnable(originalCommand.concurrencyStrategy, hystrixRequestContext, new Runnable() {

@Override
public void run() {
child.onError(new HystrixTimeoutException());
}
});


timeoutRunnable.run();
//if it did not start, then we need to mark a command start for concurrency metrics, and then issue the timeout
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ public HystrixContextRunnable(Runnable actual) {
}

public HystrixContextRunnable(HystrixConcurrencyStrategy concurrencyStrategy, final Runnable actual) {
this(concurrencyStrategy, HystrixRequestContext.getContextForCurrentThread(), actual);
}

public HystrixContextRunnable(final HystrixConcurrencyStrategy concurrencyStrategy, final HystrixRequestContext hystrixRequestContext, final Runnable actual) {
this.actual = concurrencyStrategy.wrapCallable(new Callable<Void>() {

@Override
Expand All @@ -43,7 +47,7 @@ public Void call() throws Exception {
}

});
this.parentThreadState = HystrixRequestContext.getContextForCurrentThread();
this.parentThreadState = hystrixRequestContext;
}

@Override
Expand Down

0 comments on commit 951c6f9

Please sign in to comment.