Skip to content

Commit

Permalink
Remove Strategy Injection on HystrixCommand
Browse files Browse the repository at this point in the history 
#34
  • Loading branch information
@benjchristensen
benjchristensen committed Dec 4, 2012
1 parent 8b76e2f commit bfb6696
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 243 deletions.
40 changes: 5 additions & 35 deletions hystrix-core/src/main/java/com/netflix/hystrix/HystrixCollapser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@ protected HystrixCollapser(HystrixCollapserKey collapserKey) {
* Fluent interface for constructor arguments
*/
protected HystrixCollapser(Setter setter) {
this(setter.collapserKey, setter.scope, new RealCollapserTimer(), setter.propertiesStrategy, setter.propertiesSetter, setter.concurrencyStrategy);
this(setter.collapserKey, setter.scope, new RealCollapserTimer(), setter.propertiesSetter);
}

private HystrixCollapser(HystrixCollapserKey collapserKey, Scope scope, CollapserTimer timer, HystrixPropertiesStrategy propertiesFactory, HystrixCollapserProperties.Setter propertiesBuilder, HystrixConcurrencyStrategy concurrencyStrategy) {
private HystrixCollapser(HystrixCollapserKey collapserKey, Scope scope, CollapserTimer timer, HystrixCollapserProperties.Setter propertiesBuilder) {
/* strategy: ConcurrencyStrategy */
this.concurrencyStrategy = HystrixPlugins.getInstance().getConcurrencyStrategy(concurrencyStrategy);
this.concurrencyStrategy = HystrixPlugins.getInstance().getConcurrencyStrategy();

this.timer = timer;
this.scope = scope;
Expand All @@ -152,7 +152,7 @@ private HystrixCollapser(HystrixCollapserKey collapserKey, Scope scope, Collapse
this.collapserKey = collapserKey;
}
this.requestCache = HystrixRequestCache.getInstance(this.collapserKey, this.concurrencyStrategy);
this.properties = HystrixPropertiesFactory.getCollapserProperties(propertiesFactory, this.collapserKey, propertiesBuilder);
this.properties = HystrixPropertiesFactory.getCollapserProperties(this.collapserKey, propertiesBuilder);
}

/**
Expand Down Expand Up @@ -1010,9 +1010,7 @@ private static String getDefaultNameFromClass(@SuppressWarnings("rawtypes") Clas
public static class Setter {
private final HystrixCollapserKey collapserKey;
private Scope scope = Scope.REQUEST; // default if nothing is set
private HystrixPropertiesStrategy propertiesStrategy;
private HystrixCollapserProperties.Setter propertiesSetter;
private HystrixConcurrencyStrategy concurrencyStrategy;

private Setter(HystrixCollapserKey collapserKey) {
this.collapserKey = collapserKey;
Expand Down Expand Up @@ -1043,20 +1041,6 @@ public Setter andScope(Scope scope) {
return this;
}

/**
* @param propertiesStrategy
* {@link HystrixPropertiesStrategy} implementation to override the default behavior.
* <p>
* See JavaDoc on {@link HystrixPropertiesStrategy} class header for more information.
* <p>
* Will use default if left NULL.
* @return Setter for fluent interface via method chaining
*/
public Setter andPropertiesStrategy(HystrixPropertiesStrategy propertiesStrategy) {
this.propertiesStrategy = propertiesStrategy;
return this;
}

/**
* @param propertiesSetter
* {@link HystrixCollapserProperties.Setter} that allows instance specific property overrides (which can then be overridden by dynamic properties, see
Expand All @@ -1071,20 +1055,6 @@ public Setter andCollapserPropertiesDefaults(HystrixCollapserProperties.Setter p
return this;
}

/**
*
* @param concurrencyStrategy
* {@link HystrixConcurrencyStrategy} implementation to override the default behavior.
* <p>
* See JavaDoc on {@link HystrixConcurrencyStrategy} class header for more information.
* <p>
* Will use default if left NULL.
* @return Setter for fluent interface via method chaining
*/
public Setter andConcurrencyStrategy(HystrixConcurrencyStrategy concurrencyStrategy) {
this.concurrencyStrategy = concurrencyStrategy;
return this;
}
}

// this is a micro-optimization but saves about 1-2microseconds (on 2011 MacBook Pro)
Expand Down Expand Up @@ -1819,7 +1789,7 @@ public TestRequestCollapser(TestCollapserTimer timer, AtomicInteger counter, Str
public TestRequestCollapser(Scope scope, TestCollapserTimer timer, AtomicInteger counter, String value, int defaultMaxRequestsInBatch, int defaultTimerDelayInMilliseconds, ConcurrentLinkedQueue<HystrixCommand<List<String>>> executionLog) {
// use a CollapserKey based on the CollapserTimer object reference so it's unique for each timer as we don't want caching
// of properties to occur and we're using the default HystrixProperty which typically does caching
super(collapserKeyFromString(timer), scope, timer, null, HystrixCollapserProperties.Setter().withMaxRequestsInBatch(defaultMaxRequestsInBatch).withTimerDelayInMilliseconds(defaultTimerDelayInMilliseconds), null);
super(collapserKeyFromString(timer), scope, timer, HystrixCollapserProperties.Setter().withMaxRequestsInBatch(defaultMaxRequestsInBatch).withTimerDelayInMilliseconds(defaultTimerDelayInMilliseconds));
this.count = counter;
this.value = value;
this.commandsExecuted = executionLog;
Expand Down
116 changes: 25 additions & 91 deletions hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import com.netflix.hystrix.strategy.concurrency.HystrixContextRunnable;
import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext;
import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier;
import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisher;
import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisherFactory;
import com.netflix.hystrix.strategy.properties.HystrixPropertiesFactory;
import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy;
Expand Down Expand Up @@ -129,6 +128,7 @@ public abstract class HystrixCommand<R> implements HystrixExecutable<R> {
/**
* Construct a {@link HystrixCommand} with defined {@link HystrixCommandGroupKey}.
* <p>
* The {@link HystrixCommandKey} will be derived from the implementing class name.
*
* @param group
* {@link HystrixCommandGroupKey} used to group together multiple {@link HystrixCommand} objects.
Expand All @@ -142,17 +142,18 @@ protected HystrixCommand(HystrixCommandGroupKey group) {
}

/**
* Construct a {@link HystrixCommand} with defined {@link Setter} that allows
* injecting property and strategy overrides and other optional arguments.
* Construct a {@link HystrixCommand} with defined {@link Setter} that allows injecting property and strategy overrides and other optional arguments.
* <p>
* Null values on everything except 'group' will result in the default being used.
* NOTE: The {@link HystrixCommandKey} is used to associate a {@link HystrixCommand} with {@link HystrixCircuitBreaker}, {@link HystrixCommandMetrics} and other objects.
* <p>
* Do not create multiple {@link HystrixCommand} implementations with the same {@link HystrixCommandKey} but different injected default properties as the first instantiated will win.
*
* @param setter
* Fluent interface for constructor arguments
*/
protected HystrixCommand(Setter setter) {
// use 'null' to specify use the default
this(setter.groupKey, setter.commandKey, setter.threadPoolKey, null, null, setter.propertiesStrategy, setter.commandPropertiesDefaults, setter.threadPoolPropertiesDefaults, setter.notifier, setter.concurrencyStrategy, null, setter.metricsPublisher, null, null);
this(setter.groupKey, setter.commandKey, setter.threadPoolKey, null, null, setter.commandPropertiesDefaults, setter.threadPoolPropertiesDefaults, null, null, null);
}

/**
Expand All @@ -163,8 +164,8 @@ protected HystrixCommand(Setter setter) {
* Most of the args will revert to a valid default if 'null' is passed in.
*/
private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, HystrixThreadPoolKey threadPoolKey, HystrixCircuitBreaker circuitBreaker, HystrixThreadPool threadPool,
HystrixPropertiesStrategy propertiesFactory, HystrixCommandProperties.Setter commandPropertiesDefaults, HystrixThreadPoolProperties.Setter threadPoolPropertiesDefaults, HystrixEventNotifier notifier,
HystrixConcurrencyStrategy concurrencyStrategy, HystrixCommandMetrics metrics, HystrixMetricsPublisher metricsPublisher, TryableSemaphore fallbackSemaphore, TryableSemaphore executionSemaphore) {
HystrixCommandProperties.Setter commandPropertiesDefaults, HystrixThreadPoolProperties.Setter threadPoolPropertiesDefaults,
HystrixCommandMetrics metrics, TryableSemaphore fallbackSemaphore, TryableSemaphore executionSemaphore) {
/*
* CommandGroup initialization
*/
Expand All @@ -187,7 +188,7 @@ private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, Hyst
/*
* Properties initialization
*/
this.properties = HystrixPropertiesFactory.getCommandProperties(propertiesFactory, this.commandKey, commandPropertiesDefaults);
this.properties = HystrixPropertiesFactory.getCommandProperties(this.commandKey, commandPropertiesDefaults);

/*
* ThreadPoolKey
Expand All @@ -212,15 +213,17 @@ private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, Hyst
}

/* strategy: HystrixEventNotifier */
this.eventNotifier = HystrixPlugins.getInstance().getEventNotifier(notifier);
this.eventNotifier = HystrixPlugins.getInstance().getEventNotifier();

/* strategy: HystrixConcurrentStrategy */
this.concurrencyStrategy = HystrixPlugins.getInstance().getConcurrencyStrategy(concurrencyStrategy);
this.concurrencyStrategy = HystrixPlugins.getInstance().getConcurrencyStrategy();

/*
* Metrics initialization
*/
if (metrics == null) {
// TODO this caches the first time it's loaded and will thus miss changes to threadPoolKey, properties and eventNotifier
// We need a better way of handling this now that we have HystrixPlugins
this.metrics = HystrixCommandMetrics.getInstance(this.commandKey, this.commandGroup, this.threadPoolKey, this.properties, this.eventNotifier);
} else {
this.metrics = metrics;
Expand All @@ -232,6 +235,8 @@ private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, Hyst
if (this.properties.circuitBreakerEnabled().get()) {
if (circuitBreaker == null) {
// get the default implementation of HystrixCircuitBreaker
// TODO this caches the first time it's loaded and will thus miss changes to properties
// We need a better way of handling this now that we have HystrixPlugins
this.circuitBreaker = HystrixCircuitBreaker.Factory.getInstance(this.commandKey, this.commandGroup, this.properties, this.metrics);
} else {
this.circuitBreaker = circuitBreaker;
Expand All @@ -241,14 +246,16 @@ private HystrixCommand(HystrixCommandGroupKey group, HystrixCommandKey key, Hyst
}

/* strategy: HystrixMetricsPublisherCommand */
HystrixMetricsPublisherFactory.createOrRetrievePublisherForCommand(metricsPublisher, this.commandKey, this.commandGroup, this.metrics, this.circuitBreaker, this.properties);
// TODO this caches the first time it's loaded and will thus miss changes to properties
// We need a better way of handling this now that we have HystrixPlugins
HystrixMetricsPublisherFactory.createOrRetrievePublisherForCommand(this.commandKey, this.commandGroup, this.metrics, this.circuitBreaker, this.properties);

/*
* ThreadPool initialization
*/
if (threadPool == null) {
// get the default implementation of HystrixThreadPool
this.threadPool = HystrixThreadPool.Factory.getInstance(this.threadPoolKey, this.concurrencyStrategy, metricsPublisher, propertiesFactory, threadPoolPropertiesDefaults);
this.threadPool = HystrixThreadPool.Factory.getInstance(this.threadPoolKey, threadPoolPropertiesDefaults);
} else {
this.threadPool = threadPool;
}
Expand Down Expand Up @@ -1547,12 +1554,8 @@ public static class Setter {
private final HystrixCommandGroupKey groupKey;
private HystrixCommandKey commandKey;
private HystrixThreadPoolKey threadPoolKey;
private HystrixPropertiesStrategy propertiesStrategy;
private HystrixCommandProperties.Setter commandPropertiesDefaults;
private HystrixThreadPoolProperties.Setter threadPoolPropertiesDefaults;
private HystrixEventNotifier notifier;
private HystrixConcurrencyStrategy concurrencyStrategy;
private HystrixMetricsPublisher metricsPublisher;

/**
* Setter factory method containing required values.
Expand Down Expand Up @@ -1618,18 +1621,6 @@ public Setter andThreadPoolKey(HystrixThreadPoolKey threadPoolKey) {
return this;
}

/**
* @param propertiesStrategy
* {@link HystrixPropertiesStrategy} implementation used to retrieve {@link HystrixCommandProperties} and {@link HystrixThreadPoolProperties}.
* <p>
* See the {@link HystrixPropertiesStrategy} JavaDocs for more information.
* @return Setter for fluent interface via method chaining
*/
public Setter andPropertiesStrategy(HystrixPropertiesStrategy propertiesStrategy) {
this.propertiesStrategy = propertiesStrategy;
return this;
}

/**
* Optional
*
Expand Down Expand Up @@ -1658,42 +1649,6 @@ public Setter andThreadPoolPropertiesDefaults(HystrixThreadPoolProperties.Setter
return this;
}

/**
* @param notifier
* {@link HystrixEventNotifier} implementation used to perform event notification.
* <p>
* See the {@link HystrixEventNotifier} JavaDocs for more information.
* @return Setter for fluent interface via method chaining
*/
public Setter andEventNotifier(HystrixEventNotifier notifier) {
this.notifier = notifier;
return this;
}

/**
* @param concurrencyStrategy
* {@link HystrixConcurrencyStrategy} implementation used for concurrency related behavior.
* <p>
* See {@link HystrixConcurrencyStrategy} JavaDocs for more information.
* @return Setter for fluent interface via method chaining
*/
public Setter andConcurrencyStrategy(HystrixConcurrencyStrategy concurrencyStrategy) {
this.concurrencyStrategy = concurrencyStrategy;
return this;
}

/**
* @param metricsPublisher
* {@link HystrixMetricsPublisher} implementation used for publishing metrics.
* <p>
* See {@link HystrixMetricsPublisher} JavaDocs for more information.
* @return Setter for fluent interface via method chaining
*/
public Setter andMetricsPublisher(HystrixMetricsPublisher metricsPublisher) {
this.metricsPublisher = metricsPublisher;
return this;
}

}

public static class UnitTest {
Expand All @@ -1702,6 +1657,7 @@ public static class UnitTest {
public void prepareForTest() {
/* we must call this to simulate a new request lifecycle running and clearing caches */
HystrixRequestContext.initializeContext();
HystrixPlugins.getInstance().registerPropertiesStrategy(TEST_PROPERTIES_FACTORY);
}

@After
Expand All @@ -1714,6 +1670,8 @@ public void cleanup() {

// force properties to be clean as well
ConfigurationManager.getConfigInstance().clear();

HystrixPlugins.getInstance().registerPropertiesStrategy(null);
}

/**
Expand Down Expand Up @@ -4121,9 +4079,9 @@ public void testBadRequestExceptionViaQueueInSemaphore() {
final TestCommandBuilder builder;

TestHystrixCommand(TestCommandBuilder builder) {
super(builder.owner, builder.dependencyKey, builder.threadPoolKey, builder.circuitBreaker, builder.threadPool, builder.propertiesFactory,
builder.commandPropertiesDefaults, builder.threadPoolPropertiesDefaults, builder.notifier, builder.concurrencyStrategy, builder.metrics,
builder.metricsPublisher, builder.fallbackSemaphore, builder.executionSemaphore);
super(builder.owner, builder.dependencyKey, builder.threadPoolKey, builder.circuitBreaker, builder.threadPool,
builder.commandPropertiesDefaults, builder.threadPoolPropertiesDefaults, builder.metrics,
builder.fallbackSemaphore, builder.executionSemaphore);
this.builder = builder;
}

Expand All @@ -4138,13 +4096,9 @@ static class TestCommandBuilder {
HystrixThreadPoolKey threadPoolKey = null;
HystrixCircuitBreaker circuitBreaker = _cb;
HystrixThreadPool threadPool = null;
HystrixPropertiesStrategy propertiesFactory = TEST_PROPERTIES_FACTORY;
HystrixCommandProperties.Setter commandPropertiesDefaults = HystrixCommandProperties.Setter.getUnitTestPropertiesSetter();
HystrixThreadPoolProperties.Setter threadPoolPropertiesDefaults = HystrixThreadPoolProperties.Setter.getUnitTestPropertiesBuilder();
HystrixEventNotifier notifier = null;
HystrixConcurrencyStrategy concurrencyStrategy = null;
HystrixCommandMetrics metrics = _cb.metrics;
HystrixMetricsPublisher metricsPublisher = null;
TryableSemaphore fallbackSemaphore = null;
TryableSemaphore executionSemaphore = null;

Expand Down Expand Up @@ -4173,11 +4127,6 @@ TestCommandBuilder setThreadPool(HystrixThreadPool threadPool) {
return this;
}

TestCommandBuilder setPropertiesFactory(HystrixPropertiesStrategy propertiesFactory) {
this.propertiesFactory = propertiesFactory;
return this;
}

TestCommandBuilder setCommandPropertiesDefaults(HystrixCommandProperties.Setter commandPropertiesDefaults) {
this.commandPropertiesDefaults = commandPropertiesDefaults;
return this;
Expand All @@ -4188,26 +4137,11 @@ TestCommandBuilder setThreadPoolPropertiesDefaults(HystrixThreadPoolProperties.S
return this;
}

TestCommandBuilder setNotifier(HystrixEventNotifier notifier) {
this.notifier = notifier;
return this;
}

TestCommandBuilder setConcurrencyStrategy(HystrixConcurrencyStrategy concurrencyStrategy) {
this.concurrencyStrategy = concurrencyStrategy;
return this;
}

TestCommandBuilder setMetrics(HystrixCommandMetrics metrics) {
this.metrics = metrics;
return this;
}

TestCommandBuilder setMetricsPublisher(HystrixMetricsPublisher metricsPublisher) {
this.metricsPublisher = metricsPublisher;
return this;
}

TestCommandBuilder setFallbackSemaphore(TryableSemaphore fallbackSemaphore) {
this.fallbackSemaphore = fallbackSemaphore;
return this;
Expand Down
Loading

0 comments on commit bfb6696

Please sign in to comment.