Skip to content

Commit

Permalink
Use binary backoff for errors by default. Add builder for WorkflowSet…
Browse files Browse the repository at this point in the history
…tings.
  • Loading branch information
gmokki committed Sep 12, 2014
1 parent 6d86fc2 commit 2ed54ab
Show file tree
Hide file tree
Showing 14 changed files with 111 additions and 147 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ private int busyLoopPrevention(WorkflowSettings settings,
int subsequentStateExecutions, StateExecutionImpl execution) {
if (subsequentStateExecutions++ >= MAX_SUBSEQUENT_STATE_EXECUTIONS && execution.getNextActivation() != null) {
logger.warn("Executed {} times without delay, forcing short transition delay", MAX_SUBSEQUENT_STATE_EXECUTIONS);
execution.setNextActivation(execution.getNextActivation().plusMillis(settings.getShortTransitionDelay()));
execution.setNextActivation(execution.getNextActivation().plusMillis(settings.shortTransitionDelay));
}
return subsequentStateExecutions;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.util.Map;
import java.util.Set;

import org.joda.time.DateTime;
import org.springframework.util.Assert;

import com.nitorcreations.nflow.engine.internal.workflow.StateExecutionImpl;
Expand All @@ -29,7 +30,7 @@ public abstract class AbstractWorkflowDefinition<S extends WorkflowState> {
private Map<String, WorkflowStateMethod> stateMethods;

protected AbstractWorkflowDefinition(String type, S initialState, S errorState) {
this(type, initialState, errorState, new WorkflowSettings(null));
this(type, initialState, errorState, new WorkflowSettings.Builder().build());
}

protected AbstractWorkflowDefinition(String type, S initialState, S errorState, WorkflowSettings settings) {
Expand Down Expand Up @@ -120,20 +121,20 @@ void requireStateMethodExists(S state) {
}

public void handleRetry(StateExecutionImpl execution) {
if (execution.getRetries() >= getSettings().getMaxRetries()) {
if (execution.getRetries() >= getSettings().maxRetries) {
execution.setRetry(false);
WorkflowState failureState = failureTransitions.get(execution.getCurrentStateName());
if (failureState != null) {
execution.setNextState(failureState);
execution.setNextStateReason("Max retry count exceeded");
execution.setNextActivation(getSettings().getErrorTransitionActivation());
execution.setNextActivation(new DateTime());
} else {
execution.setNextState(errorState);
execution.setNextStateReason("Max retry count exceeded, no failure state defined");
execution.setNextActivation(null);
}
} else {
execution.setNextActivation(getSettings().getErrorTransitionActivation());
execution.setNextActivation(getSettings().getErrorTransitionActivation(execution.getRetries()));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public abstract class WorkflowDefinition<S extends Enum<S> & WorkflowState> exte

@SuppressFBWarnings(value = "BC_UNCONFIRMED_CAST", justification = "findbugs does not understand that S extends both WorkflowState end Enum")
protected WorkflowDefinition(String type, S initialState, S errorState) {
this(type, initialState, errorState, new WorkflowSettings(null));
this(type, initialState, errorState, new WorkflowSettings.Builder().build());
}

@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,63 +1,95 @@
package com.nitorcreations.nflow.engine.workflow.definition;

import static java.util.concurrent.TimeUnit.HOURS;
import static java.lang.Math.min;
import static java.util.concurrent.TimeUnit.DAYS;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.joda.time.DateTime.now;

import javax.inject.Inject;

import org.joda.time.DateTime;
import org.springframework.core.env.Environment;
import org.springframework.stereotype.Component;

@Component
public class WorkflowSettings {
public final int minErrorTransitionDelay;
public final int maxErrorTransitionDelay;
public final int shortTransitionDelay;
public final int immediateTransitionDelay;
public final int maxRetries;

private final Environment env;
private WorkflowSettings(Builder builder) {
this.minErrorTransitionDelay = builder.minErrorTransitionDelay;
this.maxErrorTransitionDelay = builder.maxErrorTransitionDelay;
this.shortTransitionDelay = builder.shortTransitionDelay;
this.immediateTransitionDelay = builder.immediateTransitionDelay;
this.maxRetries = builder.maxRetries;
}

private final int errorTransitionDelay;
private final int shortTransitionDelay;
private final int immediateTransitionDelay;
private final int maxRetries;
public static class Builder {
public int maxErrorTransitionDelay;
public int minErrorTransitionDelay;
public int shortTransitionDelay;
public int immediateTransitionDelay;
public int maxRetries;

@Inject
public WorkflowSettings(Environment env) {
this.env = env;
errorTransitionDelay = getIntegerProperty("nflow.transition.delay.waiterror.ms", (int) HOURS.toMillis(2));
shortTransitionDelay = getIntegerProperty("nflow.transition.delay.waitshort.ms", (int) SECONDS.toMillis(30));
immediateTransitionDelay = getIntegerProperty("nflow.transition.delay.immediate.ms", 0);
maxRetries = getIntegerProperty("nflow.max.state.retries", 3);
}
public Builder() {
this(null);
}

public DateTime getErrorTransitionActivation() {
return now().plusMillis(getErrorTransitionDelay());
}
public Builder(Environment env) {
minErrorTransitionDelay = getIntegerProperty(env, "nflow.transition.delay.error.min.ms", (int) MINUTES.toMillis(1));
maxErrorTransitionDelay = getIntegerProperty(env, "nflow.transition.delay.error.max.ms", (int) DAYS.toMillis(1));
shortTransitionDelay = getIntegerProperty(env, "nflow.transition.delay.waitshort.ms", (int) SECONDS.toMillis(30));
immediateTransitionDelay = getIntegerProperty(env, "nflow.transition.delay.immediate.ms", 0);
maxRetries = getIntegerProperty(env, "nflow.max.state.retries", 17);
}

public int getErrorTransitionDelay() {
return errorTransitionDelay;
}
public Builder setMaxErrorTransitionDelay(int maxErrorTransitionDelay) {
this.maxErrorTransitionDelay = maxErrorTransitionDelay;
return this;
}

public DateTime getShortTransitionActivation() {
return now().plusMillis(getShortTransitionDelay());
}
public Builder setMinErrorTransitionDelay(int minErrorTransitionDelay) {
this.minErrorTransitionDelay = minErrorTransitionDelay;
return this;
}

public int getShortTransitionDelay() {
return shortTransitionDelay;
}
public Builder setShortTransitionDelay(int shortTransitionDelay) {
this.shortTransitionDelay = shortTransitionDelay;
return this;
}

public Builder setImmediateTransitionDelay(int immediateTransitionDelay) {
this.immediateTransitionDelay = immediateTransitionDelay;
return this;
}

public int getImmediateTransitionDelay() {
return immediateTransitionDelay;
public Builder setMaxRetries(int maxRetries) {
this.maxRetries = maxRetries;
return this;
}

private int getIntegerProperty(Environment env, String key, int defaultValue) {
if (env != null) {
return env.getProperty(key, Integer.class, defaultValue);
}
return defaultValue;
}

public WorkflowSettings build() {
return new WorkflowSettings(this);
}
}

public int getMaxRetries() {
return maxRetries;
public DateTime getErrorTransitionActivation(int retryCount) {
return now()
.plusMillis(calculateBinaryBackoffDelay(retryCount + 1, minErrorTransitionDelay, maxErrorTransitionDelay));
}

private int getIntegerProperty(String key, int defaultValue) {
if (env != null) {
return env.getRequiredProperty(key, Integer.class);
}
return defaultValue;
protected int calculateBinaryBackoffDelay(int retryCount, int minDelay, int maxDelay) {
return min(minDelay * (1 << retryCount), maxDelay);
}

public DateTime getShortTransitionActivation() {
return now().plusMillis(shortTransitionDelay);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,14 @@ public void runWorkflowThroughToFailureState() {
Mockito.doReturn(wf).when(workflowDefinitions).getWorkflowDefinition(eq("test"));
WorkflowInstance instance = constructWorkflowInstanceBuilder()
.setType("test").setId(Integer.valueOf(1)).setProcessing(true)
.setState("start").setRetries(wf.getSettings().getMaxRetries()).build();
.setState("start").setRetries(wf.getSettings().maxRetries).build();
when(workflowInstances.getWorkflowInstance(eq(instance.id))).thenReturn(instance);
doNothing().when(workflowInstances).updateWorkflowInstance(update.capture(), action.capture());
executor.run();
verify(workflowInstances).updateWorkflowInstance(argThat(matchesWorkflowInstance(FailingTestWorkflow.State.failure, 0, false)),
argThat(matchesWorkflowInstanceAction(FailingTestWorkflow.State.start, wf.getSettings().getMaxRetries())));
assertThat(update.getAllValues().get(0), matchesWorkflowInstance(FailingTestWorkflow.State.failure, 0, true));
assertThat(update.getAllValues().get(1), matchesWorkflowInstance(FailingTestWorkflow.State.error, 0, true));
assertThat(action.getAllValues().get(0), matchesWorkflowInstanceAction(FailingTestWorkflow.State.start, wf.getSettings().maxRetries));
assertThat(action.getAllValues().get(1), matchesWorkflowInstanceAction(FailingTestWorkflow.State.failure, 0));
}

@Test
Expand Down Expand Up @@ -423,7 +426,7 @@ public String getDescription() {
}

public NextAction start(StateExecution execution) {
return moveToStateAfter(State.process, now().plusMillis(getSettings().getErrorTransitionDelay()), "Process after delay");
return moveToStateAfter(State.process, getSettings().getErrorTransitionActivation(0), "Process after delay");
}

public NextAction process(StateExecution execution, @StateVar("string") String s, @StateVar("int") int i, @StateVar("pojo") Pojo pojo, @StateVar(value="nullPojo", instantiateNull=true) Pojo pojo2, @StateVar(value="immutablePojo", readOnly=true) Pojo unmodifiablePojo, @StateVar("nullInt") int zero, @StateVar("mutableString") Mutable<String> mutableString) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void handleRetryMaxRetriesExceededNotHaveFailureState() {
private StateExecutionImpl handleRetryMaxRetriesExceeded(TestState currentState) {
TestDefinition def = new TestDefinition("x", TestState.start);
StateExecutionImpl execution = mock(StateExecutionImpl.class);
when(execution.getRetries()).thenReturn(def.getSettings().getMaxRetries());
when(execution.getRetries()).thenReturn(def.getSettings().maxRetries);
when(execution.getCurrentStateName()).thenReturn(currentState.name());
def.handleRetry(execution);
return execution;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.nitorcreations.nflow.engine.workflow.definition;

import static org.joda.time.DateTimeUtils.currentTimeMillis;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.joda.time.DateTimeUtils.currentTimeMillis;
import static org.junit.Assert.assertThat;

import org.junit.Test;
Expand All @@ -12,9 +12,9 @@
public class WorkflowSettingsTest {
@Test
public void verifyConsantDefaultValues() {
WorkflowSettings s = new WorkflowSettings(null);
assertThat(s.getImmediateTransitionDelay(), is(0));
assertThat(s.getShortTransitionDelay(), is(30000));
WorkflowSettings s = new WorkflowSettings.Builder().build();
assertThat(s.immediateTransitionDelay, is(0));
assertThat(s.shortTransitionDelay, is(30000));
long delta = s.getShortTransitionActivation().getMillis() - currentTimeMillis() - 30000;
assertThat(delta, greaterThanOrEqualTo(-1000L));
assertThat(delta, lessThanOrEqualTo(0L));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@ public ListWorkflowDefinitionResponse convert(WorkflowDefinition<? extends Workf

WorkflowSettings workflowSettings = definition.getSettings();
TransitionDelays transitionDelays = new TransitionDelays();
transitionDelays.immediate = workflowSettings.getImmediateTransitionDelay();
transitionDelays.waitShort = workflowSettings.getShortTransitionDelay();
transitionDelays.waitError = workflowSettings.getErrorTransitionDelay();
transitionDelays.immediate = workflowSettings.immediateTransitionDelay;
transitionDelays.waitShort = workflowSettings.shortTransitionDelay;
transitionDelays.minErrorWait = workflowSettings.minErrorTransitionDelay;
transitionDelays.maxErrorWait = workflowSettings.maxErrorTransitionDelay;
Settings settings = new Settings();
settings.transitionDelaysInMilliseconds = transitionDelays;
settings.maxRetries = workflowSettings.getMaxRetries();
settings.maxRetries = workflowSettings.maxRetries;
resp.settings = settings;

return resp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,11 @@ public static class TransitionDelays {
@ApiModelProperty(value = "Short delay between transitions", required=true)
public long waitShort;

@ApiModelProperty(value = "Maximum retries for a state before moving to failure", required=true)
public long waitError;
@ApiModelProperty(value = "First retry delay after failure", required=true)
public long minErrorWait;

@ApiModelProperty(value = "Maximum delay between failure retries", required=true)
public long maxErrorWait;

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
import static com.nitorcreations.nflow.rest.v1.DummyTestWorkflow.State.error;
import static com.nitorcreations.nflow.rest.v1.DummyTestWorkflow.State.start;

import org.springframework.core.env.Environment;

import com.nitorcreations.nflow.engine.workflow.definition.NextAction;
import com.nitorcreations.nflow.engine.workflow.definition.StateExecution;
import com.nitorcreations.nflow.engine.workflow.definition.WorkflowDefinition;
Expand Down Expand Up @@ -49,7 +47,7 @@ public String getDescription() {
}

public DummyTestWorkflow() {
super("dummy", start, error, new DummyTestSettings(null));
super("dummy", start, error, new WorkflowSettings.Builder().setMinErrorTransitionDelay(300).setMaxErrorTransitionDelay(1000).setShortTransitionDelay(200).setImmediateTransitionDelay(100).setMaxRetries(10).build());
permit(start, end, error);
permit(start, error);
permit(error, end);
Expand All @@ -66,33 +64,4 @@ public NextAction error(StateExecution execution) {
public NextAction end(StateExecution execution) {
return stopInState(end, "Finished in end state");
}

public static class DummyTestSettings extends WorkflowSettings {

public DummyTestSettings(Environment env) {
super(env);
}

@Override
public int getErrorTransitionDelay() {
return 300;
}

@Override
public int getShortTransitionDelay() {
return 200;
}

@Override
public int getImmediateTransitionDelay() {
return 100;
}

@Override
public int getMaxRetries() {
return 10;
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ public void convertWorks() {
reflectEquals(getResponseState(end, Collections.<String>emptyList(), null)),
reflectEquals(getResponseState(error, asList(end.name()), null)),
reflectEquals(getResponseState(start, asList(end.name(), error.name()), error.name()))));
assertThat((int)resp.settings.transitionDelaysInMilliseconds.immediate, is(def.getSettings().getImmediateTransitionDelay()));
assertThat((int)resp.settings.transitionDelaysInMilliseconds.waitShort, is(def.getSettings().getShortTransitionDelay()));
assertThat((int)resp.settings.transitionDelaysInMilliseconds.waitError, is(def.getSettings().getErrorTransitionDelay()));
assertThat(resp.settings.maxRetries, is(def.getSettings().getMaxRetries()));
assertThat((int)resp.settings.transitionDelaysInMilliseconds.immediate, is(def.getSettings().immediateTransitionDelay));
assertThat((int)resp.settings.transitionDelaysInMilliseconds.waitShort, is(def.getSettings().shortTransitionDelay));
assertThat((int)resp.settings.transitionDelaysInMilliseconds.minErrorWait, is(def.getSettings().minErrorTransitionDelay));
assertThat((int)resp.settings.transitionDelaysInMilliseconds.maxErrorWait, is(def.getSettings().maxErrorTransitionDelay));
assertThat(resp.settings.maxRetries, is(def.getSettings().maxRetries));
}

private State getResponseState(DummyTestWorkflow.State workflowState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public String getDescription() {
}

public CreditApplicationWorkflow() {
super("creditApplicationProcess", createCreditApplication, error, new CreditApplicationWorkflowSettings());
super("creditApplicationProcess", createCreditApplication, error, new WorkflowSettings.Builder().setMinErrorTransitionDelay(0).setMaxErrorTransitionDelay(0).setShortTransitionDelay(0).setMaxRetries(3).build());
permit(createCreditApplication, acceptCreditApplication);
permit(acceptCreditApplication, grantLoan);
permit(acceptCreditApplication, finishCreditApplication);
Expand Down Expand Up @@ -132,22 +132,4 @@ public CreditApplication(String customerId, BigDecimal amount) {
public static class WorkflowInfo {
public String applicationId;
}

public static class CreditApplicationWorkflowSettings extends WorkflowSettings {

public CreditApplicationWorkflowSettings() {
super(null);
}

@Override
public int getErrorTransitionDelay() {
return 0;
}

@Override
public int getShortTransitionDelay() {
return 0;
}
}

}
Loading

0 comments on commit 2ed54ab

Please sign in to comment.