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

Only fire onFallbackStart/onFallbackError execution hooks if there is a user-supplied fallback #754

Merged
merged 1 commit into from
Apr 13, 2015
Merged

Only fire onFallbackStart/onFallbackError execution hooks if there is a user-supplied fallback #754

Show file tree
Hide file tree
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
43 changes: 41 additions & 2 deletions hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.netflix.hystrix;

import java.lang.ref.Reference;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -121,6 +122,8 @@ protected static enum TimedOutStatus {
// on the repetitive string processing that will occur on the same classes over and over again
private static ConcurrentHashMap<Class<?>, String> defaultNameCache = new ConcurrentHashMap<Class<?>, String>();

private static ConcurrentHashMap<HystrixCommandKey, Boolean> commandContainsFallback = new ConcurrentHashMap<HystrixCommandKey, Boolean>();

/* package */static String getDefaultNameFromClass(Class<?> cls) {
String fromCache = defaultNameCache.get(cls);
if (fromCache != null) {
Expand Down Expand Up @@ -721,7 +724,9 @@ private Observable<R> getFallbackOrThrowException(final HystrixEventType eventTy

// acquire a permit
if (fallbackSemaphore.tryAcquire()) {
executionHook.onFallbackStart(this);
if (isFallbackUserSupplied(this)) {
executionHook.onFallbackStart(this);
}

try {
fallbackExecutionChain = getFallbackObservable();
Expand Down Expand Up @@ -1055,6 +1060,36 @@ protected TryableSemaphore getExecutionSemaphore() {
}
}

/**
* Each concrete implementation of AbstractCommand should return the name of the fallback method as a String
* This will be used to determine if the fallback "exists" for firing the onFallbackStart/onFallbackError hooks
* @return method name of fallback
*/
protected abstract String getFallbackMethodName();

/**
* For the given command instance, does it define an actual fallback method?
* @param cmd command instance
* @return true iff there is a user-supplied fallback method on the given command instance
*/
/*package-private*/ static boolean isFallbackUserSupplied(final AbstractCommand<?> cmd) {
HystrixCommandKey commandKey = cmd.commandKey;
Boolean containsFromMap = commandContainsFallback.get(commandKey);
if (containsFromMap != null) {
return containsFromMap;
} else {
Boolean toInsertIntoMap;
try {
cmd.getClass().getDeclaredMethod(cmd.getFallbackMethodName());
toInsertIntoMap = true;
} catch (NoSuchMethodException nsme) {
toInsertIntoMap = false;
}
commandContainsFallback.put(commandKey, toInsertIntoMap);
return toInsertIntoMap;
}
}

protected static class ObservableCommand<R> extends Observable<R> {
private final AbstractCommand<R> command;

Expand Down Expand Up @@ -1471,7 +1506,11 @@ private Exception wrapWithOnExecutionErrorHook(Throwable t) {
private Exception wrapWithOnFallbackErrorHook(Throwable t) {
Exception e = getExceptionFromThrowable(t);
try {
return executionHook.onFallbackError(this, e);
if (isFallbackUserSupplied(this)) {
return executionHook.onFallbackError(this, e);
} else {
return e;
}
} catch (Throwable hookEx) {
logger.warn("Error calling ExecutionHook.onFallbackError", hookEx);
return e;
Expand Down
5 changes: 5 additions & 0 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 @@ -405,4 +405,9 @@ public Future<R> queue() {
return f;
}

@Override
protected String getFallbackMethodName() {
return "getFallback";
}

}
5 changes: 5 additions & 0 deletions hystrix-core/src/main/java/com/netflix/hystrix/HystrixObservableCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ protected boolean shouldOutputOnNextEvents() {
return true;
}

@Override
protected String getFallbackMethodName() {
return "resumeWithFallback";
}

/**
* Construct a {@link HystrixObservableCommand} with defined {@link Setter} that allows injecting property and strategy overrides and other optional arguments.
* <p>
Expand Down
62 changes: 31 additions & 31 deletions hystrix-core/src/test/java/com/netflix/hystrix/CommonHystrixCommandTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,11 +354,11 @@ public void call(C command) {
TestableExecutionHook hook = command.getBuilder().executionHook;
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(RuntimeException.class, hook.getCommandException().getClass());
assertEquals(RuntimeException.class, hook.getExecutionException().getClass());
assertEquals(UnsupportedOperationException.class, hook.getFallbackException().getClass());
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onExecutionError - !onRunError - onFallbackStart - onFallbackError - onError - onThreadComplete - ", hook.executionSequence.toString());
assertNull(hook.getFallbackException());
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onExecutionError - !onRunError - onError - onThreadComplete - ", hook.executionSequence.toString());
}
});
}
Expand Down Expand Up @@ -451,10 +451,10 @@ public void call(C command) {
TestableExecutionHook hook = command.getBuilder().executionHook;
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 0, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(TimeoutException.class, hook.getCommandException().getClass());
assertEquals(UnsupportedOperationException.class, hook.getFallbackException().getClass());
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onFallbackStart - onFallbackError - onError - ", hook.executionSequence.toString());
assertNull(hook.getFallbackException());
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onError - ", hook.executionSequence.toString());

try {
Thread.sleep(300);
Expand All @@ -464,8 +464,8 @@ public void call(C command) {

assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(1, 0, 1));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onFallbackStart - onFallbackError - onError - onExecutionEmit - !onRunSuccess - onExecutionSuccess - onThreadComplete - ", hook.executionSequence.toString());
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onError - onExecutionEmit - !onRunSuccess - onExecutionSuccess - onThreadComplete - ", hook.executionSequence.toString());

}
});
Expand Down Expand Up @@ -580,10 +580,10 @@ public void call(C command) {
TestableExecutionHook hook = command.getBuilder().executionHook;
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 0, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(TimeoutException.class, hook.getCommandException().getClass());
assertEquals(UnsupportedOperationException.class, hook.getFallbackException().getClass());
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onFallbackStart - onFallbackError - onError - ", hook.executionSequence.toString());
assertNull(hook.getFallbackException());
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onError - ", hook.executionSequence.toString());

try {
Thread.sleep(300);
Expand All @@ -592,9 +592,9 @@ public void call(C command) {
}
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(RuntimeException.class, hook.getExecutionException().getClass());
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onFallbackStart - onFallbackError - onError - onExecutionError - !onRunError - onThreadComplete - ", hook.executionSequence.toString());
assertEquals("onStart - onThreadStart - !onRunStart - onExecutionStart - onError - onExecutionError - !onRunError - onThreadComplete - ", hook.executionSequence.toString());

}
});
Expand Down Expand Up @@ -718,10 +718,10 @@ public void call(C command) {
TestableExecutionHook hook = command.getBuilder().executionHook;
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 0, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(RejectedExecutionException.class, hook.getCommandException().getClass());
assertEquals(UnsupportedOperationException.class, hook.getFallbackException().getClass());
assertEquals("onStart - onFallbackStart - onFallbackError - onError - ", hook.executionSequence.toString());
assertNull(hook.getFallbackException());
assertEquals("onStart - onError - ", hook.executionSequence.toString());
}
});
}
Expand Down Expand Up @@ -837,10 +837,10 @@ public void call(C command) {
TestableExecutionHook hook = command.getBuilder().executionHook;
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 0, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(RejectedExecutionException.class, hook.getCommandException().getClass());
assertEquals(UnsupportedOperationException.class, hook.getFallbackException().getClass());
assertEquals("onStart - onFallbackStart - onFallbackError - onError - ", hook.executionSequence.toString());
assertNull(hook.getFallbackException());
assertEquals("onStart - onError - ", hook.executionSequence.toString());
}
});
}
Expand Down Expand Up @@ -940,10 +940,10 @@ public void call(C command) {
TestableExecutionHook hook = command.getBuilder().executionHook;
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 0, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(RuntimeException.class, hook.getCommandException().getClass());
assertEquals(UnsupportedOperationException.class, hook.getFallbackException().getClass());
assertEquals("onStart - onFallbackStart - onFallbackError - onError - ", hook.executionSequence.toString());
assertNull(hook.getFallbackException());
assertEquals("onStart - onError - ", hook.executionSequence.toString());
}
});
}
Expand Down Expand Up @@ -1116,11 +1116,11 @@ public void call(C command) {
TestableExecutionHook hook = command.getBuilder().executionHook;
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(RuntimeException.class, hook.getCommandException().getClass());
assertEquals(RuntimeException.class, hook.getExecutionException().getClass());
assertEquals(UnsupportedOperationException.class, hook.getFallbackException().getClass());
assertEquals("onStart - !onRunStart - onExecutionStart - onExecutionError - !onRunError - onFallbackStart - onFallbackError - onError - ", hook.executionSequence.toString());
assertNull(hook.getFallbackException());
assertEquals("onStart - !onRunStart - onExecutionStart - onExecutionError - !onRunError - onError - ", hook.executionSequence.toString());
}
});
}
Expand Down Expand Up @@ -1232,10 +1232,10 @@ public void call(C command) {
TestableExecutionHook hook = command.getBuilder().executionHook;
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 0, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(RuntimeException.class, hook.getCommandException().getClass());
assertEquals(UnsupportedOperationException.class, hook.getFallbackException().getClass());
assertEquals("onStart - onFallbackStart - onFallbackError - onError - ", hook.executionSequence.toString());
assertNull(hook.getFallbackException());
assertEquals("onStart - onError - ", hook.executionSequence.toString());
}
});
}
Expand Down Expand Up @@ -1370,10 +1370,10 @@ public void call(C command) {
TestableExecutionHook hook = command.getBuilder().executionHook;
assertTrue(hook.commandEmissionsMatch(0, 1, 0));
assertTrue(hook.executionEventsMatch(0, 0, 0));
assertTrue(hook.fallbackEventsMatch(0, 1, 0));
assertTrue(hook.fallbackEventsMatch(0, 0, 0));
assertEquals(RuntimeException.class, hook.getCommandException().getClass());
assertEquals(UnsupportedOperationException.class, hook.getFallbackException().getClass());
assertEquals("onStart - onFallbackStart - onFallbackError - onError - ", hook.executionSequence.toString());
assertNull(hook.getFallbackException());
assertEquals("onStart - onError - ", hook.executionSequence.toString());
}
});
}
Expand Down
Loading