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.

Sign up for GitHub

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

Jump to bottom

Do not overwrite user thread duration in case of timeouts #194

Merged
merged 1 commit into from
Nov 5, 2013
Merged

Do not overwrite user thread duration in case of timeouts #194

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,7 @@ private Subscription subscribeWithThreadIsolation(final Observer<? super R> obse

@Override
public R call() throws Exception {
boolean recordDuration = true;
try {
// assign 'callingThread' to our NFExceptionThreadingUtility ThreadLocal variable so that if we blow up
// anywhere along the way the exception knows who the calling thread is and can include it in the stacktrace
Expand All @@ -1164,7 +1165,6 @@ public R call() throws Exception {
try {
// store the command that is being run
Hystrix.startCurrentThreadExecutingCommand(getCommandKey());

// execute the command
R r = executeCommand();
// if we can go from NOT_EXECUTED to COMPLETED then we did not timeout
Expand All @@ -1174,14 +1174,16 @@ public R call() throws Exception {
// pass to the observer
observer.onNext(r);
// state changes before termination
preTerminationWork();
preTerminationWork(recordDuration);
/* now complete which releases the consumer */
observer.onCompleted();
return r;
} else {
// this means we lost the race and the timeout logic has or is being executed
// state changes before termination
preTerminationWork();
// do not recordDuration as this is a timeout and the tick would have set the duration already.
recordDuration = false;
preTerminationWork(recordDuration);
return null;
}
} finally {
Expand All @@ -1190,7 +1192,7 @@ public R call() throws Exception {
}
} catch (Exception e) {
// state changes before termination
preTerminationWork();
preTerminationWork(recordDuration);
// if we can go from NOT_EXECUTED to COMPLETED then we did not timeout
if (isCommandTimedOut.compareAndSet(TimedOutStatus.NOT_EXECUTED, TimedOutStatus.COMPLETED)) {
observer.onError(e);
Expand All @@ -1199,10 +1201,11 @@ public R call() throws Exception {
}
}

private void preTerminationWork() {
/* execution time (must occur before terminal state otherwise a race condition can occur if requested by client) */
recordTotalExecutionTime(invocationStartTime);

private void preTerminationWork(boolean recordDuration) {
if(recordDuration) {
/* execution time (must occur before terminal state otherwise a race condition can occur if requested by client) */
recordTotalExecutionTime(invocationStartTime);
}
threadPool.markThreadCompletion();

try {
Expand Down