Repository navigation
Fix replay determinism for retry timeouts - #326
Open
andystaples wants to merge 2 commits into
Open
andystaples wants to merge 2 commits into
andystaples wants to merge 2 commits into
Conversation
Use orchestration time for activity and sub-orchestration retry expiration, normalize UTC timestamps, and cover replay and deadline boundaries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The datetime checks mishandle timestamps whose tzinfo is non-None but utcoffset() returns None, causing host-dependent conversions that can prematurely exhaust retry timeouts.
1 open issue
What changed in this PR
This PR addresses #316 by using replay-safe orchestration time for activity and sub-orchestration retry timeouts, preventing delayed history replay from incorrectly suppressing retries.
Changes:
- Passes logical time into retry calculations and normalizes timezone-aware timestamps while retaining the direct-call wall-clock fallback.
- Adds regression coverage for replay, timeout boundaries, timezone handling, and successful completion after retries.
- Documents the fix and upgrade precautions across all three packages without changing provider versions or dependency minimums.
| File | Description |
|---|---|
| tests/durabletask/test_retry_timeout.py | Adds timeout and replay regression tests. |
| durabletask/worker.py | Supplies orchestration time to both retry paths. |
| durabletask/task.py | Adds explicit retry time and timestamp normalization. |
| durabletask-azuremanaged/CHANGELOG.md | Documents the provider impact and core upgrade guidance. |
| CHANGELOG.md | Records the fix and warns about existing history compatibility. |
| azure-functions-durable/CHANGELOG.md | Documents the provider impact and core upgrade guidance. |
Review effort: Balanced
Comment on lines
+841
to
+845
| if current_time.tzinfo is not None: | ||
| current_time = current_time.astimezone(UTC).replace(tzinfo=None) | ||
| start_time = self._start_time | ||
| if start_time.tzinfo is not None: | ||
| start_time = start_time.astimezone(UTC).replace(tzinfo=None) |
Verify expiration on the fifth failure after the 14-second deadline, with a higher attempt limit so timeout exhaustion is exercised independently. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Activity and sub-orchestration retry timeout decisions now use the orchestration's replay-safe logical clock instead of wall-clock time. Replaying a historical failure after the real-time deadline no longer incorrectly suppresses its retry.
RetryableTask.compute_next_delay()accepts an optional current timestamp, normalizes timezone-aware inputs to UTC, and treats naive inputs as UTC. Direct calls without a timestamp retain their wall-clock fallback. Retry attempt limits, backoff, interval caps, and the existing expiration boundary are preserved; an absent timeout remains unbounded.Regression coverage exercises activity and sub-orchestration history replay, recorded retry timers through successful completion, deadline boundaries, timezone offsets, and calls without an explicit timestamp.
Upgrade and release coordination
Histories that recorded timeout failures based on wall-clock time can replay differently under the corrected behavior, including orchestrations that caught those failures. Drain affected instances on the previous SDK before upgrading. This change does not recover already-failed instances.
The core, Azure Managed, and Azure Functions changelogs document the fix. Provider versions and dependency minimums remain unchanged; coordinate provider releases after the corrected core SDK is released.
Fixes #316