This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit 3be0143191ea659cfad402565c13d2531b95e7de Author: Alex Heneveld <[email protected]> AuthorDate: Tue Mar 21 10:57:00 2023 +0000 fix bug where backoff max wasn't checked for retries --- .../core/workflow/steps/flow/RetryWorkflowStep.java | 5 +++++ .../brooklyn/core/workflow/WorkflowRetryTest.java | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/RetryWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/RetryWorkflowStep.java index 8770336e5f..e35574707f 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/RetryWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/RetryWorkflowStep.java @@ -284,6 +284,11 @@ public class RetryWorkflowStep extends WorkflowStepDefinition { if (backoff.increase !=null) delay = delay.add(backoff.increase.multiply(exponent)); if (backoff.jitter!=null) delay = delay.multiply(1 + Math.random()*backoff.jitter); + if (backoff.max!=null && delay.isLongerThan(backoff.max)) { + delay = backoff.max; + // also apply a sigmoidal heuristic if jitter requested + if (backoff.jitter!=null) delay = delay.multiply(1 / (1+Math.random()*backoff.jitter)); + } if (delay.isPositive()) { Duration ddelay = delay; diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowRetryTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowRetryTest.java index 6f538cf274..ef3b500f36 100644 --- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowRetryTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowRetryTest.java @@ -278,6 +278,27 @@ public class WorkflowRetryTest extends RebindTestFixture<BasicApplication> { Asserts.assertThat(Duration.of(sw), d -> d.isShorterThan(Duration.millis(EXPECTED_DELAY + GRACE))); } + @Test(groups="Integration") // because slow + public void testRetryWithBackoffUpToAndLimit() { + Stopwatch sw = Stopwatch.createStarted(); + try { + Task<?> lastInvocation = runSteps(MutableList.of( + "let integer x = ${entity.sensor.x} ?? 0", + "let x = ${x} + 1", + "set-sensor x = ${x}", + "retry from start limit 1s backoff 1ms increasing 2x up to 32ms"), null); + Asserts.shouldHaveFailedPreviously("Instead got "+lastInvocation.getUnchecked()); + } catch (Exception e) { + Asserts.expectedFailure(e); + Asserts.assertNotNull(Exceptions.getFirstThrowableOfType(e, RetryWorkflowStep.RetriesExceeded.class), "Exception "+e); + } + long EXPECTED_DELAY = 999; + Asserts.assertThat(Duration.of(sw), d -> d.isLongerThan(Duration.millis(EXPECTED_DELAY))); + Integer x = app.sensors().get(Sensors.newIntegerSensor("x")); + Asserts.assertThat(x, xx -> xx > 12); // shouldn't keep doubling beyond 32 ms + Asserts.assertThat(x, xx -> xx < 40); // but should increase + } + Task<?> lastInvocation; @Test
