From 7a779676af79c5d99749b86f3bfec794b28f6f34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juha=20Syrj=C3=A4l=C3=A4?= Date: Wed, 22 Oct 2014 22:18:25 +0300 Subject: [PATCH] Fix binary backoff calculation for error transition delay. --- .../workflow/definition/WorkflowSettings.java | 15 ++++++-- .../definition/WorkflowSettingsTest.java | 34 +++++++++++++++++-- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/nflow-engine/src/main/java/com/nitorcreations/nflow/engine/workflow/definition/WorkflowSettings.java b/nflow-engine/src/main/java/com/nitorcreations/nflow/engine/workflow/definition/WorkflowSettings.java index 1100eacd4..883ebcb2f 100644 --- a/nflow-engine/src/main/java/com/nitorcreations/nflow/engine/workflow/definition/WorkflowSettings.java +++ b/nflow-engine/src/main/java/com/nitorcreations/nflow/engine/workflow/definition/WorkflowSettings.java @@ -1,5 +1,6 @@ package com.nitorcreations.nflow.engine.workflow.definition; +import static java.lang.Math.max; import static java.lang.Math.min; import static java.util.concurrent.TimeUnit.DAYS; import static java.util.concurrent.TimeUnit.MINUTES; @@ -9,6 +10,8 @@ import org.joda.time.DateTime; import org.springframework.core.env.Environment; +import java.math.BigInteger; + /** * Configuration for the workflow execution. */ @@ -144,7 +147,7 @@ public WorkflowSettings build() { * @return Next activation time. */ public DateTime getErrorTransitionActivation(int retryCount) { - return now().plusMillis(calculateBinaryBackoffDelay(retryCount + 1, minErrorTransitionDelay, maxErrorTransitionDelay)); + return now().plus(calculateBinaryBackoffDelay(retryCount + 1, minErrorTransitionDelay, maxErrorTransitionDelay)); } /** @@ -158,8 +161,14 @@ public DateTime getErrorTransitionActivation(int retryCount) { * Maximum retry delay. * @return Delay in milliseconds. */ - protected int calculateBinaryBackoffDelay(int retryCount, int minDelay, int maxDelay) { - return min(minDelay * (1 << retryCount), maxDelay); + protected long calculateBinaryBackoffDelay(int retryCount, long minDelay, long maxDelay) { + BigInteger delay = BigInteger.valueOf(minDelay).multiply(BigInteger.valueOf(2).pow(retryCount)); + if(!BigInteger.valueOf(delay.longValue()).equals(delay)) { + // got overflow in delay calculation + // Java 1.8 has delay.longValueExact() + return maxDelay; + } + return max(minDelay, min(delay.longValue(), maxDelay)); } /** diff --git a/nflow-engine/src/test/java/com/nitorcreations/nflow/engine/workflow/definition/WorkflowSettingsTest.java b/nflow-engine/src/test/java/com/nitorcreations/nflow/engine/workflow/definition/WorkflowSettingsTest.java index 5094d493b..4bb2816eb 100644 --- a/nflow-engine/src/test/java/com/nitorcreations/nflow/engine/workflow/definition/WorkflowSettingsTest.java +++ b/nflow-engine/src/test/java/com/nitorcreations/nflow/engine/workflow/definition/WorkflowSettingsTest.java @@ -6,12 +6,27 @@ import static org.joda.time.DateTimeUtils.currentTimeMillis; import static org.junit.Assert.assertThat; +import org.joda.time.DateTime; +import org.joda.time.DateTimeUtils; +import org.junit.After; +import org.junit.Before; import org.junit.Test; - public class WorkflowSettingsTest { + DateTime now = new DateTime(2014, 10, 22, 20, 44, 0); + + @Before + public void setup() { + DateTimeUtils.setCurrentMillisFixed(now.getMillis()); + } + + @After + public void teardown() { + DateTimeUtils.setCurrentMillisSystem(); + } + @Test - public void verifyConsantDefaultValues() { + public void verifyConstantDefaultValues() { WorkflowSettings s = new WorkflowSettings.Builder().build(); assertThat(s.immediateTransitionDelay, is(0)); assertThat(s.shortTransitionDelay, is(30000)); @@ -19,4 +34,19 @@ public void verifyConsantDefaultValues() { assertThat(delta, greaterThanOrEqualTo(-1000L)); assertThat(delta, lessThanOrEqualTo(0L)); } + + @Test + public void errorTransitionDelayIsBetweenMinAndMaxDelay() { + int maxDelay = 1_000_000; + int minDelay = 1000; + WorkflowSettings s = new WorkflowSettings.Builder().setMinErrorTransitionDelay(minDelay).setMaxErrorTransitionDelay(maxDelay).build(); + long prevDelay = 0; + for(int retryCount = 0 ; retryCount < 100 ; retryCount++) { + long delay = s.getErrorTransitionActivation(retryCount).getMillis() - now.getMillis(); + assertThat(delay, greaterThanOrEqualTo((long)minDelay)); + assertThat(delay, lessThanOrEqualTo((long)maxDelay)); + assertThat(delay, greaterThanOrEqualTo(prevDelay)); + prevDelay = delay; + } + } }