From 170663b25d59d9d136aaaf2281bcdeb384086e92 Mon Sep 17 00:00:00 2001 From: Javier Aliaga Date: Tue, 3 Mar 2026 18:16:09 +0100 Subject: [PATCH 1/6] fix: MaxRetryInterval was not propagate Signed-off-by: Javier Aliaga --- .../runtime/DefaultWorkflowContext.java | 3 ++ .../workflows/DefaultWorkflowContextTest.java | 40 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java b/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java index 164fd3b41..507fadc93 100644 --- a/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java +++ b/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java @@ -281,6 +281,9 @@ private RetryPolicy toRetryPolicy(WorkflowTaskRetryPolicy workflowTaskRetryPolic ); retryPolicy.setBackoffCoefficient(workflowTaskRetryPolicy.getBackoffCoefficient()); + if (workflowTaskRetryPolicy.getMaxRetryInterval() != null) { + retryPolicy.setMaxRetryInterval(workflowTaskRetryPolicy.getMaxRetryInterval()); + } if (workflowTaskRetryPolicy.getRetryTimeout() != null) { retryPolicy.setRetryTimeout(workflowTaskRetryPolicy.getRetryTimeout()); } diff --git a/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java b/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java index 18f31e507..f1a54690b 100644 --- a/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java +++ b/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java @@ -523,4 +523,44 @@ public void workflowRetryPolicyRetryThrowIllegalArgumentWhenRetryTimeoutIsLessTh .setRetryTimeout(Duration.ofSeconds(9)) .build()); } + + @Test + public void callActivityRetryPolicyMaxRetryIntervalShouldBePropagated() { + String expectedName = "TestActivity"; + String expectedInput = "TestInput"; + Duration expectedMaxRetryInterval = Duration.ofSeconds(60); + WorkflowTaskRetryPolicy retryPolicy = WorkflowTaskRetryPolicy.newBuilder() + .setMaxNumberOfAttempts(5) + .setFirstRetryInterval(Duration.ofSeconds(1)) + .setMaxRetryInterval(expectedMaxRetryInterval) + .build(); + WorkflowTaskOptions options = new WorkflowTaskOptions(retryPolicy); + ArgumentCaptor captor = ArgumentCaptor.forClass(TaskOptions.class); + + context.callActivity(expectedName, expectedInput, options, String.class); + + verify(mockInnerContext, times(1)) + .callActivity(eq(expectedName), eq(expectedInput), captor.capture(), eq(String.class)); + + assertEquals(expectedMaxRetryInterval, captor.getValue().getRetryPolicy().getMaxRetryInterval()); + } + + @Test + public void callActivityRetryPolicyDefaultMaxRetryIntervalShouldBeZeroWhenNotSet() { + String expectedName = "TestActivity"; + String expectedInput = "TestInput"; + WorkflowTaskRetryPolicy retryPolicy = WorkflowTaskRetryPolicy.newBuilder() + .setMaxNumberOfAttempts(5) + .setFirstRetryInterval(Duration.ofSeconds(1)) + .build(); + WorkflowTaskOptions options = new WorkflowTaskOptions(retryPolicy); + ArgumentCaptor captor = ArgumentCaptor.forClass(TaskOptions.class); + + context.callActivity(expectedName, expectedInput, options, String.class); + + verify(mockInnerContext, times(1)) + .callActivity(eq(expectedName), eq(expectedInput), captor.capture(), eq(String.class)); + + assertEquals(Duration.ZERO, captor.getValue().getRetryPolicy().getMaxRetryInterval()); + } } From 215de45d19a6cca09db606b5044e8da02ebb265e Mon Sep 17 00:00:00 2001 From: Javier Aliaga Date: Wed, 4 Mar 2026 15:59:14 +0100 Subject: [PATCH 2/6] feat: Add jitter to retryPolicy Signed-off-by: Javier Aliaga --- .../java/io/dapr/durabletask/RetryPolicy.java | 33 +++++++++++++++++ .../TaskOrchestrationExecutor.java | 17 +++++++-- .../workflows/WorkflowTaskRetryPolicy.java | 36 +++++++++++++++++-- .../runtime/DefaultWorkflowContext.java | 1 + 4 files changed, 82 insertions(+), 5 deletions(-) diff --git a/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java b/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java index 9efd912b1..1d26c2b16 100644 --- a/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java +++ b/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java @@ -27,6 +27,7 @@ public final class RetryPolicy { private double backoffCoefficient = 1.0; private Duration maxRetryInterval = Duration.ZERO; private Duration retryTimeout = Duration.ZERO; + private double jitterFactor = 0.0; /** * Creates a new {@code RetryPolicy} object. @@ -173,4 +174,36 @@ public Duration getMaxRetryInterval() { public Duration getRetryTimeout() { return this.retryTimeout; } + + /** + * Sets the jitter factor applied to the computed retry delay. + * + *

A value between 0.0 (no jitter) and 1.0 (up to 100% reduction). For each retry, the delay + * is reduced by a random fraction in the range {@code [0, jitterFactor]}, using a deterministic + * seed derived from the first-attempt timestamp and the attempt number. The seed must be + * deterministic: the delay drives the {@code finalFireAt} of a durable timer, and if replay + * computes a different value, the timer-chain check may create spurious sub-timers that shift + * subsequent sequence IDs and cause a NonDeterministicOrchestratorException. + * This desynchronizes concurrent workflow retries and avoids thundering herd behaviour.

+ * + * @param jitterFactor the jitter factor; must be between 0.0 and 1.0 inclusive + * @return this retry policy object + * @throws IllegalArgumentException if {@code jitterFactor} is outside [0.0, 1.0] + */ + public RetryPolicy setJitterFactor(double jitterFactor) { + if (jitterFactor < 0.0 || jitterFactor > 1.0) { + throw new IllegalArgumentException("The value for jitterFactor must be between 0.0 and 1.0 inclusive."); + } + this.jitterFactor = jitterFactor; + return this; + } + + /** + * Gets the configured jitter factor. + * + * @return the configured jitter factor + */ + public double getJitterFactor() { + return this.jitterFactor; + } } diff --git a/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java b/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java index 1ecdcde7e..54ac109e0 100644 --- a/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java +++ b/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java @@ -38,6 +38,7 @@ import java.util.List; import java.util.Map; import java.util.Queue; +import java.util.Random; import java.util.Set; import java.util.UUID; import java.util.concurrent.CancellationException; @@ -1513,10 +1514,20 @@ private Duration getNextDelay() { // NOTE: A max delay of zero or less is interpreted to mean no max delay if (nextDelayInMillis > maxDelayInMillis && maxDelayInMillis > 0) { - return this.policy.getMaxRetryInterval(); - } else { - return Duration.ofMillis(nextDelayInMillis); + nextDelayInMillis = maxDelayInMillis; } + + // Apply jitter: reduce delay by a random fraction in [0, jitterFactor]. + // Seed is deterministic so that replay computes the same finalFireAt, preventing + // the createTimerChain callback from creating spurious extra sub-timers. + double jitterFactor = this.policy.getJitterFactor(); + if (jitterFactor > 0.0) { + long seed = this.firstAttempt.toEpochMilli() + this.attemptNumber; + double reduction = new Random(seed).nextDouble() * jitterFactor; + nextDelayInMillis = (long) (nextDelayInMillis * (1.0 - reduction)); + } + + return Duration.ofMillis(nextDelayInMillis); } // If there's no declarative retry policy defined, then the custom code retry handler diff --git a/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java b/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java index b0e72f917..4a1e2cf7a 100644 --- a/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java +++ b/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java @@ -24,6 +24,7 @@ public final class WorkflowTaskRetryPolicy { private final Double backoffCoefficient; private final Duration maxRetryInterval; private final Duration retryTimeout; + private final Double jitterFactor; /** * Constructor for WorkflowTaskRetryPolicy. @@ -32,19 +33,24 @@ public final class WorkflowTaskRetryPolicy { * @param backoffCoefficient Coefficient to increase the retry interval. * @param maxRetryInterval Maximum interval to wait between retries. * @param retryTimeout Timeout for the whole retry process. + * @param jitterFactor Jitter factor between 0.0 and 1.0; reduces each retry delay by a random + * fraction in [0, jitterFactor] to desynchronize concurrent retries. + * 0.0 disables jitter (default). */ public WorkflowTaskRetryPolicy( Integer maxNumberOfAttempts, Duration firstRetryInterval, Double backoffCoefficient, Duration maxRetryInterval, - Duration retryTimeout + Duration retryTimeout, + Double jitterFactor ) { this.maxNumberOfAttempts = maxNumberOfAttempts; this.firstRetryInterval = firstRetryInterval; this.backoffCoefficient = backoffCoefficient; this.maxRetryInterval = maxRetryInterval; this.retryTimeout = retryTimeout; + this.jitterFactor = jitterFactor; } public int getMaxNumberOfAttempts() { @@ -67,6 +73,10 @@ public Duration getRetryTimeout() { return retryTimeout; } + public double getJitterFactor() { + return jitterFactor != null ? jitterFactor : 0.0; + } + public static Builder newBuilder() { return new Builder(); } @@ -78,6 +88,7 @@ public static class Builder { private Double backoffCoefficient = 1.0; private Duration maxRetryInterval; private Duration retryTimeout; + private Double jitterFactor = 0.0; private Builder() { } @@ -92,7 +103,8 @@ public WorkflowTaskRetryPolicy build() { this.firstRetryInterval, this.backoffCoefficient, this.maxRetryInterval, - this.retryTimeout + this.retryTimeout, + this.jitterFactor ); } @@ -176,6 +188,26 @@ public Builder setRetryTimeout(Duration retryTimeout) { return this; } + + /** + * Set the jitter factor applied to the computed retry delay. + * + *

A value between 0.0 (no jitter, default) and 1.0 (up to 100% reduction). For each retry, + * the computed delay is reduced by a random fraction in [0, jitterFactor]. + * This desynchronizes concurrent workflow retries and avoids thundering herd behaviour.

+ * + * @param jitterFactor Jitter factor between 0.0 and 1.0 inclusive + * @return This builder + */ + public Builder setJitterFactor(double jitterFactor) { + if (jitterFactor < 0.0 || jitterFactor > 1.0) { + throw new IllegalArgumentException("The value for jitterFactor must be between 0.0 and 1.0 inclusive."); + } + + this.jitterFactor = jitterFactor; + + return this; + } } } diff --git a/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java b/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java index 507fadc93..4d5ef5e34 100644 --- a/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java +++ b/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java @@ -287,6 +287,7 @@ private RetryPolicy toRetryPolicy(WorkflowTaskRetryPolicy workflowTaskRetryPolic if (workflowTaskRetryPolicy.getRetryTimeout() != null) { retryPolicy.setRetryTimeout(workflowTaskRetryPolicy.getRetryTimeout()); } + retryPolicy.setJitterFactor(workflowTaskRetryPolicy.getJitterFactor()); return retryPolicy; } From eef0bbf1d09cdc4e4980d060af12d4a82dd7eb08 Mon Sep 17 00:00:00 2001 From: Javier Aliaga Date: Wed, 4 Mar 2026 16:34:45 +0100 Subject: [PATCH 3/6] chore: Tests Signed-off-by: Javier Aliaga --- .../io/dapr/durabletask/RetryPolicyTest.java | 139 ++++++++++++++++++ .../WorkflowTaskRetryPolicyTest.java | 114 ++++++++++++++ 2 files changed, 253 insertions(+) create mode 100644 durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java create mode 100644 sdk-workflows/src/test/java/io/dapr/workflows/WorkflowTaskRetryPolicyTest.java diff --git a/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java b/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java new file mode 100644 index 000000000..5cbe13c74 --- /dev/null +++ b/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java @@ -0,0 +1,139 @@ +/* + * Copyright 2026 The Dapr Authors + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and +limitations under the License. +*/ + +package io.dapr.durabletask; + +import org.junit.jupiter.api.Test; + +import java.time.Duration; +import java.util.Random; + +import static org.junit.jupiter.api.Assertions.*; + +public class RetryPolicyTest { + + // ---- default value ---- + + @Test + void jitterFactorDefaultsToZero() { + RetryPolicy policy = new RetryPolicy(3, Duration.ofSeconds(1)); + assertEquals(0.0, policy.getJitterFactor()); + } + + // ---- valid boundary values ---- + + @Test + void jitterFactorZeroIsAccepted() { + RetryPolicy policy = new RetryPolicy(3, Duration.ofSeconds(1)); + RetryPolicy returned = policy.setJitterFactor(0.0); + assertEquals(0.0, policy.getJitterFactor()); + assertSame(policy, returned, "setJitterFactor should return this for chaining"); + } + + @Test + void jitterFactorOneIsAccepted() { + RetryPolicy policy = new RetryPolicy(3, Duration.ofSeconds(1)); + policy.setJitterFactor(1.0); + assertEquals(1.0, policy.getJitterFactor()); + } + + @Test + void jitterFactorMidRangeIsAccepted() { + RetryPolicy policy = new RetryPolicy(3, Duration.ofSeconds(1)); + policy.setJitterFactor(0.5); + assertEquals(0.5, policy.getJitterFactor()); + } + + // ---- invalid values ---- + + @Test + void jitterFactorBelowZeroThrows() { + RetryPolicy policy = new RetryPolicy(3, Duration.ofSeconds(1)); + assertThrows(IllegalArgumentException.class, () -> policy.setJitterFactor(-0.1)); + } + + @Test + void jitterFactorAboveOneThrows() { + RetryPolicy policy = new RetryPolicy(3, Duration.ofSeconds(1)); + assertThrows(IllegalArgumentException.class, () -> policy.setJitterFactor(1.1)); + } + + // ---- deterministic delay formula ---- + + /** + * Verifies that the jitter reduction formula is deterministic: given the same + * firstAttempt epoch millis and attempt number (which together form the seed), + * the reduced delay must always equal the pre-computed expected value. + * + *

This mirrors the logic in TaskOrchestrationExecutor.RetriableTask.getNextDelay(): + * 

+   *   seed      = firstAttempt.toEpochMilli() + attemptNumber
+   *   reduction = new Random(seed).nextDouble() * jitterFactor
+   *   delay     = (long)(baseDelayMillis * (1.0 - reduction))
+   *
+ */ + @Test + void jitterDelayIsDeterministicForGivenSeed() { + long firstAttemptEpochMillis = 1_700_000_000_000L; + int attemptNumber = 1; + long baseDelayMillis = 1000L; + double jitterFactor = 0.5; + + long seed = firstAttemptEpochMillis + attemptNumber; + double reduction = new Random(seed).nextDouble() * jitterFactor; + long expected = (long) (baseDelayMillis * (1.0 - reduction)); + + // Calling with the same seed twice must produce the same result. + long seed2 = firstAttemptEpochMillis + attemptNumber; + double reduction2 = new Random(seed2).nextDouble() * jitterFactor; + long result2 = (long) (baseDelayMillis * (1.0 - reduction2)); + + assertEquals(expected, result2); + } + + /** + * Verifies that with jitterFactor=0.5 the reduced delay is always between + * 50% and 100% of the base delay (i.e. never negative or exceeding the base). + */ + @Test + void jitterReducedDelayIsWithinExpectedBounds() { + long baseDelayMillis = 2000L; + double jitterFactor = 0.5; + + for (int attempt = 1; attempt <= 10; attempt++) { + long seed = System.currentTimeMillis() + attempt; + double reduction = new Random(seed).nextDouble() * jitterFactor; + long reduced = (long) (baseDelayMillis * (1.0 - reduction)); + + assertTrue(reduced >= (long) (baseDelayMillis * (1.0 - jitterFactor)), + "Reduced delay should be >= base * (1 - jitterFactor)"); + assertTrue(reduced <= baseDelayMillis, + "Reduced delay should not exceed base delay"); + } + } + + /** + * With jitterFactor=0 the delay must be unchanged. + */ + @Test + void zeroJitterLeavesDelayUnchanged() { + long baseDelayMillis = 3000L; + double jitterFactor = 0.0; + + long seed = 42L; + double reduction = new Random(seed).nextDouble() * jitterFactor; + long reduced = (long) (baseDelayMillis * (1.0 - reduction)); + + assertEquals(baseDelayMillis, reduced); + } +} diff --git a/sdk-workflows/src/test/java/io/dapr/workflows/WorkflowTaskRetryPolicyTest.java b/sdk-workflows/src/test/java/io/dapr/workflows/WorkflowTaskRetryPolicyTest.java new file mode 100644 index 000000000..831d5168d --- /dev/null +++ b/sdk-workflows/src/test/java/io/dapr/workflows/WorkflowTaskRetryPolicyTest.java @@ -0,0 +1,114 @@ +/* + * Copyright 2025 The Dapr Authors + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +package io.dapr.workflows; + +import org.junit.jupiter.api.Test; + +import java.time.Duration; + +import static org.junit.jupiter.api.Assertions.*; + +public class WorkflowTaskRetryPolicyTest { + + // ---- default value ---- + + @Test + void jitterFactorDefaultsToZero() { + WorkflowTaskRetryPolicy policy = WorkflowTaskRetryPolicy.newBuilder().build(); + assertEquals(0.0, policy.getJitterFactor()); + } + + /** + * When the policy is constructed via the all-args constructor with a null + * jitterFactor (e.g. deserialisation path), getJitterFactor() must still + * return 0.0 rather than throw a NullPointerException. + */ + @Test + void jitterFactorNullInConstructorReturnsZero() { + WorkflowTaskRetryPolicy policy = new WorkflowTaskRetryPolicy( + 3, + Duration.ofSeconds(1), + 1.0, + null, + null, + null // jitterFactor = null + ); + assertEquals(0.0, policy.getJitterFactor()); + } + + // ---- valid boundary values ---- + + @Test + void jitterFactorZeroIsAccepted() { + WorkflowTaskRetryPolicy policy = WorkflowTaskRetryPolicy.newBuilder() + .setJitterFactor(0.0) + .build(); + assertEquals(0.0, policy.getJitterFactor()); + } + + @Test + void jitterFactorOneIsAccepted() { + WorkflowTaskRetryPolicy policy = WorkflowTaskRetryPolicy.newBuilder() + .setJitterFactor(1.0) + .build(); + assertEquals(1.0, policy.getJitterFactor()); + } + + @Test + void jitterFactorMidRangeIsAccepted() { + WorkflowTaskRetryPolicy policy = WorkflowTaskRetryPolicy.newBuilder() + .setJitterFactor(0.3) + .build(); + assertEquals(0.3, policy.getJitterFactor()); + } + + // ---- invalid values ---- + + @Test + void jitterFactorBelowZeroThrows() { + WorkflowTaskRetryPolicy.Builder builder = WorkflowTaskRetryPolicy.newBuilder(); + assertThrows(IllegalArgumentException.class, () -> builder.setJitterFactor(-0.1)); + } + + @Test + void jitterFactorAboveOneThrows() { + WorkflowTaskRetryPolicy.Builder builder = WorkflowTaskRetryPolicy.newBuilder(); + assertThrows(IllegalArgumentException.class, () -> builder.setJitterFactor(1.1)); + } + + // ---- builder chaining ---- + + @Test + void builderReturnsItself() { + WorkflowTaskRetryPolicy.Builder builder = WorkflowTaskRetryPolicy.newBuilder(); + assertSame(builder, builder.setJitterFactor(0.5)); + } + + // ---- coexistence with other fields ---- + + @Test + void jitterFactorDoesNotAffectOtherFields() { + WorkflowTaskRetryPolicy policy = WorkflowTaskRetryPolicy.newBuilder() + .setMaxNumberOfAttempts(5) + .setFirstRetryInterval(Duration.ofSeconds(2)) + .setBackoffCoefficient(2.0) + .setJitterFactor(0.25) + .build(); + + assertEquals(5, policy.getMaxNumberOfAttempts()); + assertEquals(Duration.ofSeconds(2), policy.getFirstRetryInterval()); + assertEquals(2.0, policy.getBackoffCoefficient()); + assertEquals(0.25, policy.getJitterFactor()); + } +} From 987919ec14381098bb6f0964ee0df636634bef9e Mon Sep 17 00:00:00 2001 From: Javier Aliaga Date: Wed, 4 Mar 2026 17:08:11 +0100 Subject: [PATCH 4/6] chore: Fix review comments Signed-off-by: Javier Aliaga --- .../java/io/dapr/durabletask/RetryPolicy.java | 4 +- .../TaskOrchestrationExecutor.java | 4 +- .../io/dapr/durabletask/RetryPolicyTest.java | 12 ++++++ .../workflows/WorkflowTaskRetryPolicy.java | 25 ++++++++++-- .../workflows/DefaultWorkflowContextTest.java | 40 +++++++++++++++++++ .../WorkflowTaskRetryPolicyTest.java | 12 ++++++ 6 files changed, 90 insertions(+), 7 deletions(-) diff --git a/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java b/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java index 1d26c2b16..ca33914ed 100644 --- a/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java +++ b/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java @@ -179,7 +179,7 @@ public Duration getRetryTimeout() { * Sets the jitter factor applied to the computed retry delay. * *

A value between 0.0 (no jitter) and 1.0 (up to 100% reduction). For each retry, the delay - * is reduced by a random fraction in the range {@code [0, jitterFactor]}, using a deterministic + * is reduced by a random fraction in the range {@code [0, jitterFactor)}, using a deterministic * seed derived from the first-attempt timestamp and the attempt number. The seed must be * deterministic: the delay drives the {@code finalFireAt} of a durable timer, and if replay * computes a different value, the timer-chain check may create spurious sub-timers that shift @@ -191,7 +191,7 @@ public Duration getRetryTimeout() { * @throws IllegalArgumentException if {@code jitterFactor} is outside [0.0, 1.0] */ public RetryPolicy setJitterFactor(double jitterFactor) { - if (jitterFactor < 0.0 || jitterFactor > 1.0) { + if (!Double.isFinite(jitterFactor) || jitterFactor < 0.0 || jitterFactor > 1.0) { throw new IllegalArgumentException("The value for jitterFactor must be between 0.0 and 1.0 inclusive."); } this.jitterFactor = jitterFactor; diff --git a/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java b/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java index 54ac109e0..3ac170c55 100644 --- a/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java +++ b/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java @@ -1504,7 +1504,7 @@ private Duration getNextDelay() { (long) Helpers.powExact(this.policy.getBackoffCoefficient(), this.attemptNumber)); } catch (ArithmeticException overflowException) { if (maxDelayInMillis > 0) { - return this.policy.getMaxRetryInterval(); + nextDelayInMillis = maxDelayInMillis; } else { // If no maximum is specified, just throw throw new ArithmeticException("The retry policy calculation resulted in an arithmetic " @@ -1517,7 +1517,7 @@ private Duration getNextDelay() { nextDelayInMillis = maxDelayInMillis; } - // Apply jitter: reduce delay by a random fraction in [0, jitterFactor]. + // Apply jitter: reduce delay by a random fraction in [0, jitterFactor). // Seed is deterministic so that replay computes the same finalFireAt, preventing // the createTimerChain callback from creating spurious extra sub-timers. double jitterFactor = this.policy.getJitterFactor(); diff --git a/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java b/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java index 5cbe13c74..6e328c89e 100644 --- a/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java +++ b/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java @@ -68,6 +68,18 @@ void jitterFactorAboveOneThrows() { assertThrows(IllegalArgumentException.class, () -> policy.setJitterFactor(1.1)); } + @Test + void jitterFactorNaNThrows() { + RetryPolicy policy = new RetryPolicy(3, Duration.ofSeconds(1)); + assertThrows(IllegalArgumentException.class, () -> policy.setJitterFactor(Double.NaN)); + } + + @Test + void jitterFactorPositiveInfinityThrows() { + RetryPolicy policy = new RetryPolicy(3, Duration.ofSeconds(1)); + assertThrows(IllegalArgumentException.class, () -> policy.setJitterFactor(Double.POSITIVE_INFINITY)); + } + // ---- deterministic delay formula ---- /** diff --git a/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java b/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java index 4a1e2cf7a..575d4afdb 100644 --- a/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java +++ b/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java @@ -26,6 +26,25 @@ public final class WorkflowTaskRetryPolicy { private final Duration retryTimeout; private final Double jitterFactor; + /** + * Constructor for WorkflowTaskRetryPolicy (without jitter). + * @param maxNumberOfAttempts Maximum number of attempts to retry the workflow. + * @param firstRetryInterval Interval to wait before the first retry. + * @param backoffCoefficient Coefficient to increase the retry interval. + * @param maxRetryInterval Maximum interval to wait between retries. + * @param retryTimeout Timeout for the whole retry process. + */ + public WorkflowTaskRetryPolicy( + Integer maxNumberOfAttempts, + Duration firstRetryInterval, + Double backoffCoefficient, + Duration maxRetryInterval, + Duration retryTimeout + ) { + this(maxNumberOfAttempts, firstRetryInterval, backoffCoefficient, + maxRetryInterval, retryTimeout, null); + } + /** * Constructor for WorkflowTaskRetryPolicy. * @param maxNumberOfAttempts Maximum number of attempts to retry the workflow. @@ -34,7 +53,7 @@ public final class WorkflowTaskRetryPolicy { * @param maxRetryInterval Maximum interval to wait between retries. * @param retryTimeout Timeout for the whole retry process. * @param jitterFactor Jitter factor between 0.0 and 1.0; reduces each retry delay by a random - * fraction in [0, jitterFactor] to desynchronize concurrent retries. + * fraction in [0, jitterFactor) to desynchronize concurrent retries. * 0.0 disables jitter (default). */ public WorkflowTaskRetryPolicy( @@ -193,14 +212,14 @@ public Builder setRetryTimeout(Duration retryTimeout) { * Set the jitter factor applied to the computed retry delay. * *

A value between 0.0 (no jitter, default) and 1.0 (up to 100% reduction). For each retry, - * the computed delay is reduced by a random fraction in [0, jitterFactor]. + * the computed delay is reduced by a random fraction in [0, jitterFactor). * This desynchronizes concurrent workflow retries and avoids thundering herd behaviour.

* * @param jitterFactor Jitter factor between 0.0 and 1.0 inclusive * @return This builder */ public Builder setJitterFactor(double jitterFactor) { - if (jitterFactor < 0.0 || jitterFactor > 1.0) { + if (!Double.isFinite(jitterFactor) || jitterFactor < 0.0 || jitterFactor > 1.0) { throw new IllegalArgumentException("The value for jitterFactor must be between 0.0 and 1.0 inclusive."); } diff --git a/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java b/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java index f1a54690b..eb9b8ee5d 100644 --- a/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java +++ b/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java @@ -563,4 +563,44 @@ public void callActivityRetryPolicyDefaultMaxRetryIntervalShouldBeZeroWhenNotSet assertEquals(Duration.ZERO, captor.getValue().getRetryPolicy().getMaxRetryInterval()); } + + @Test + public void callActivityRetryPolicyJitterFactorShouldBePropagated() { + String expectedName = "TestActivity"; + String expectedInput = "TestInput"; + double expectedJitterFactor = 0.5; + WorkflowTaskRetryPolicy retryPolicy = WorkflowTaskRetryPolicy.newBuilder() + .setMaxNumberOfAttempts(5) + .setFirstRetryInterval(Duration.ofSeconds(1)) + .setJitterFactor(expectedJitterFactor) + .build(); + WorkflowTaskOptions options = new WorkflowTaskOptions(retryPolicy); + ArgumentCaptor captor = ArgumentCaptor.forClass(TaskOptions.class); + + context.callActivity(expectedName, expectedInput, options, String.class); + + verify(mockInnerContext, times(1)) + .callActivity(eq(expectedName), eq(expectedInput), captor.capture(), eq(String.class)); + + assertEquals(expectedJitterFactor, captor.getValue().getRetryPolicy().getJitterFactor()); + } + + @Test + public void callActivityRetryPolicyDefaultJitterFactorShouldBeZeroWhenNotSet() { + String expectedName = "TestActivity"; + String expectedInput = "TestInput"; + WorkflowTaskRetryPolicy retryPolicy = WorkflowTaskRetryPolicy.newBuilder() + .setMaxNumberOfAttempts(5) + .setFirstRetryInterval(Duration.ofSeconds(1)) + .build(); + WorkflowTaskOptions options = new WorkflowTaskOptions(retryPolicy); + ArgumentCaptor captor = ArgumentCaptor.forClass(TaskOptions.class); + + context.callActivity(expectedName, expectedInput, options, String.class); + + verify(mockInnerContext, times(1)) + .callActivity(eq(expectedName), eq(expectedInput), captor.capture(), eq(String.class)); + + assertEquals(0.0, captor.getValue().getRetryPolicy().getJitterFactor()); + } } diff --git a/sdk-workflows/src/test/java/io/dapr/workflows/WorkflowTaskRetryPolicyTest.java b/sdk-workflows/src/test/java/io/dapr/workflows/WorkflowTaskRetryPolicyTest.java index 831d5168d..f4c36f8e2 100644 --- a/sdk-workflows/src/test/java/io/dapr/workflows/WorkflowTaskRetryPolicyTest.java +++ b/sdk-workflows/src/test/java/io/dapr/workflows/WorkflowTaskRetryPolicyTest.java @@ -87,6 +87,18 @@ void jitterFactorAboveOneThrows() { assertThrows(IllegalArgumentException.class, () -> builder.setJitterFactor(1.1)); } + @Test + void jitterFactorNaNThrows() { + WorkflowTaskRetryPolicy.Builder builder = WorkflowTaskRetryPolicy.newBuilder(); + assertThrows(IllegalArgumentException.class, () -> builder.setJitterFactor(Double.NaN)); + } + + @Test + void jitterFactorPositiveInfinityThrows() { + WorkflowTaskRetryPolicy.Builder builder = WorkflowTaskRetryPolicy.newBuilder(); + assertThrows(IllegalArgumentException.class, () -> builder.setJitterFactor(Double.POSITIVE_INFINITY)); + } + // ---- builder chaining ---- @Test From e4c3cb327b2ba83dd47d8738eaa784d7957a7896 Mon Sep 17 00:00:00 2001 From: Javier Aliaga Date: Wed, 4 Mar 2026 17:35:47 +0100 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Javier Aliaga --- .../io/dapr/durabletask/RetryPolicyTest.java | 52 ------------------- 1 file changed, 52 deletions(-) diff --git a/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java b/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java index 6e328c89e..349cba1c8 100644 --- a/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java +++ b/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java @@ -80,61 +80,9 @@ void jitterFactorPositiveInfinityThrows() { assertThrows(IllegalArgumentException.class, () -> policy.setJitterFactor(Double.POSITIVE_INFINITY)); } - // ---- deterministic delay formula ---- /** - * Verifies that the jitter reduction formula is deterministic: given the same - * firstAttempt epoch millis and attempt number (which together form the seed), - * the reduced delay must always equal the pre-computed expected value. - * - *

This mirrors the logic in TaskOrchestrationExecutor.RetriableTask.getNextDelay(): - * 

-   *   seed      = firstAttempt.toEpochMilli() + attemptNumber
-   *   reduction = new Random(seed).nextDouble() * jitterFactor
-   *   delay     = (long)(baseDelayMillis * (1.0 - reduction))
-   *
- */ - @Test - void jitterDelayIsDeterministicForGivenSeed() { - long firstAttemptEpochMillis = 1_700_000_000_000L; - int attemptNumber = 1; - long baseDelayMillis = 1000L; - double jitterFactor = 0.5; - - long seed = firstAttemptEpochMillis + attemptNumber; - double reduction = new Random(seed).nextDouble() * jitterFactor; - long expected = (long) (baseDelayMillis * (1.0 - reduction)); - - // Calling with the same seed twice must produce the same result. - long seed2 = firstAttemptEpochMillis + attemptNumber; - double reduction2 = new Random(seed2).nextDouble() * jitterFactor; - long result2 = (long) (baseDelayMillis * (1.0 - reduction2)); - - assertEquals(expected, result2); - } - /** - * Verifies that with jitterFactor=0.5 the reduced delay is always between - * 50% and 100% of the base delay (i.e. never negative or exceeding the base). - */ - @Test - void jitterReducedDelayIsWithinExpectedBounds() { - long baseDelayMillis = 2000L; - double jitterFactor = 0.5; - - for (int attempt = 1; attempt <= 10; attempt++) { - long seed = System.currentTimeMillis() + attempt; - double reduction = new Random(seed).nextDouble() * jitterFactor; - long reduced = (long) (baseDelayMillis * (1.0 - reduction)); - - assertTrue(reduced >= (long) (baseDelayMillis * (1.0 - jitterFactor)), - "Reduced delay should be >= base * (1 - jitterFactor)"); - assertTrue(reduced <= baseDelayMillis, - "Reduced delay should not exceed base delay"); - } - } - - /** * With jitterFactor=0 the delay must be unchanged. */ @Test From f4996592269913dd322601f30aaacac209e32445 Mon Sep 17 00:00:00 2001 From: Javier Aliaga Date: Wed, 11 Mar 2026 12:14:18 +0100 Subject: [PATCH 6/6] chore: Nextdelay must be greater than 0 Signed-off-by: Javier Aliaga --- .../java/io/dapr/durabletask/RetryPolicy.java | 21 ++++++++++++++++- .../TaskOrchestrationExecutor.java | 10 +++----- .../io/dapr/durabletask/RetryPolicyTest.java | 23 ++++++++++++------- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java b/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java index ca33914ed..ba7b34a90 100644 --- a/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java +++ b/durabletask-client/src/main/java/io/dapr/durabletask/RetryPolicy.java @@ -14,8 +14,9 @@ package io.dapr.durabletask; import javax.annotation.Nullable; + import java.time.Duration; -import java.util.Objects; +import java.util.Random; /** * A declarative retry policy that can be configured for activity or sub-orchestration calls. @@ -206,4 +207,22 @@ public RetryPolicy setJitterFactor(double jitterFactor) { public double getJitterFactor() { return this.jitterFactor; } + + /** + * Applies jitter to a delay value, reducing it by a deterministic random fraction + * in [0, jitterFactor). The result is guaranteed to be at least 1ms. + * + * @param delayInMillis the base delay in milliseconds (must be positive) + * @param jitterFactor the jitter factor in [0.0, 1.0] + * @param seed deterministic seed for the random number generator + * @return the jittered delay in milliseconds, always >= 1 + */ + static long applyJitter(long delayInMillis, double jitterFactor, long seed) { + if (jitterFactor > 0.0) { + double reduction = new Random(seed).nextDouble() * jitterFactor; + delayInMillis = (long) (delayInMillis * (1.0 - reduction)); + return Math.max(delayInMillis, 1); + } + return delayInMillis; + } } diff --git a/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java b/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java index 3ac170c55..a12514b70 100644 --- a/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java +++ b/durabletask-client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java @@ -38,7 +38,6 @@ import java.util.List; import java.util.Map; import java.util.Queue; -import java.util.Random; import java.util.Set; import java.util.UUID; import java.util.concurrent.CancellationException; @@ -1520,12 +1519,9 @@ private Duration getNextDelay() { // Apply jitter: reduce delay by a random fraction in [0, jitterFactor). // Seed is deterministic so that replay computes the same finalFireAt, preventing // the createTimerChain callback from creating spurious extra sub-timers. - double jitterFactor = this.policy.getJitterFactor(); - if (jitterFactor > 0.0) { - long seed = this.firstAttempt.toEpochMilli() + this.attemptNumber; - double reduction = new Random(seed).nextDouble() * jitterFactor; - nextDelayInMillis = (long) (nextDelayInMillis * (1.0 - reduction)); - } + long seed = this.firstAttempt.toEpochMilli() + this.attemptNumber; + nextDelayInMillis = RetryPolicy.applyJitter( + nextDelayInMillis, this.policy.getJitterFactor(), seed); return Duration.ofMillis(nextDelayInMillis); } diff --git a/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java b/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java index 349cba1c8..e0c4b6bc9 100644 --- a/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java +++ b/durabletask-client/src/test/java/io/dapr/durabletask/RetryPolicyTest.java @@ -16,7 +16,6 @@ import org.junit.jupiter.api.Test; import java.time.Duration; -import java.util.Random; import static org.junit.jupiter.api.Assertions.*; @@ -82,18 +81,26 @@ void jitterFactorPositiveInfinityThrows() { /** - * With jitterFactor=0 the delay must be unchanged. */ @Test void zeroJitterLeavesDelayUnchanged() { long baseDelayMillis = 3000L; - double jitterFactor = 0.0; - - long seed = 42L; - double reduction = new Random(seed).nextDouble() * jitterFactor; - long reduced = (long) (baseDelayMillis * (1.0 - reduction)); + assertEquals(baseDelayMillis, RetryPolicy.applyJitter(baseDelayMillis, 0.0, 42L)); + } - assertEquals(baseDelayMillis, reduced); + /** + * With jitterFactor=1.0 the delay must never drop below 1ms, + * even when nextDouble() approaches 1.0. + */ + @Test + void jitterWithMaxFactorNeverProducesZeroDelay() { + long baseDelayMillis = 1L; // smallest meaningful delay + + for (int seed = 0; seed < 1000; seed++) { + long result = RetryPolicy.applyJitter(baseDelayMillis, 1.0, seed); + assertTrue(result >= 1, + "Delay must be at least 1ms, but was " + result + " for seed " + seed); + } } }