Skip to content

Commit 519b868

Browse files
committed
Address PR review feedback; enable DTS rewind e2e tests
- Add async rewind_orchestration parity on AsyncTaskHubGrpcClient - in-memory backend: stamp ExecutionRewound with current time; avoid duplicate executionRewound on post-rewind replay - worker: fix rewind comments, guard parentInstance, drop dead sub-orchestration branch - tests: free-port fixture, clarify docstrings, un-skip DTS rewind e2e (emulator now supports RewindInstance) - azuremanaged CHANGELOG entry
1 parent 199a701 commit 519b868

7 files changed

Lines changed: 83 additions & 43 deletions

File tree

durabletask-azuremanaged/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

88
## Unreleased
99

10+
- Added `rewind_orchestration()` to `DurableTaskSchedulerClient` and `AsyncDurableTaskSchedulerClient` (inherited from the base clients) to rewind a failed orchestration instance to its last known good state.
11+
1012
## v1.7.2
1113

1214
- Updates base dependency to durabletask v1.7.2.

durabletask/client.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,9 @@ async def SuspendInstance(self, request: pb.SuspendRequest) -> pb.SuspendRespons
384384
async def ResumeInstance(self, request: pb.ResumeRequest) -> pb.ResumeResponse:
385385
...
386386

387+
async def RewindInstance(self, request: pb.RewindInstanceRequest) -> pb.RewindInstanceResponse:
388+
...
389+
387390
async def RestartInstance(self, request: pb.RestartInstanceRequest) -> pb.RestartInstanceResponse:
388391
...
389392

@@ -786,7 +789,7 @@ def resume_orchestration(self, instance_id: str) -> None:
786789
self._stub.ResumeInstance(req)
787790

788791
def rewind_orchestration(self, instance_id: str, *,
789-
reason: str | None = None):
792+
reason: str | None = None) -> None:
790793
"""Rewinds a failed orchestration instance to its last known good state.
791794
792795
Rewind removes failed task and sub-orchestration results from the
@@ -1290,6 +1293,26 @@ async def resume_orchestration(self, instance_id: str) -> None:
12901293
self._logger.info(f"Resuming instance '{instance_id}'.")
12911294
await self._stub.ResumeInstance(req)
12921295

1296+
async def rewind_orchestration(self, instance_id: str, *,
1297+
reason: str | None = None) -> None:
1298+
"""Rewinds a failed orchestration instance to its last known good state.
1299+
1300+
Rewind removes failed task and sub-orchestration results from the
1301+
orchestration history and replays the orchestration from the last
1302+
successful checkpoint. Activities that previously succeeded are
1303+
not re-executed; only failed work is retried.
1304+
1305+
Args:
1306+
instance_id: The ID of the orchestration instance to rewind.
1307+
reason: An optional reason string describing why the orchestration is being rewound.
1308+
"""
1309+
req = pb.RewindInstanceRequest(
1310+
instanceId=instance_id,
1311+
reason=helpers.get_string_value(reason))
1312+
1313+
self._logger.info(f"Rewinding instance '{instance_id}'.")
1314+
await self._stub.RewindInstance(req)
1315+
12931316
async def restart_orchestration(self, instance_id: str, *,
12941317
restart_with_new_instance_id: bool = False) -> str:
12951318
"""Restarts an existing orchestration instance.

durabletask/testing/in_memory_backend.py

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -613,8 +613,12 @@ def GetWorkItems(self, request: pb.GetWorkItemsRequest, context: grpc.ServicerCo
613613
instance.pending_events.clear()
614614

615615
# Add OrchestratorStarted for re-dispatches so that
616-
# ctx.current_utc_datetime advances correctly
617-
if instance.history:
616+
# ctx.current_utc_datetime advances correctly, unless
617+
# the pending batch already begins with one (e.g. a
618+
# post-rewind replay supplies its own).
619+
if instance.history and not (
620+
instance.dispatched_events
621+
and instance.dispatched_events[0].HasField("orchestratorStarted")):
618622
now = datetime.now(timezone.utc)
619623
orch_started = helpers.new_orchestrator_started_event(now)
620624
instance.dispatched_events.insert(0, orch_started)
@@ -1901,10 +1905,15 @@ def _prepare_rewind(self, instance: OrchestrationInstance,
19011905
# Clear any stale dispatched events.
19021906
instance.dispatched_events.clear()
19031907

1904-
# Add the ExecutionRewound event as a new pending event.
1908+
# Add the ExecutionRewound event as a new pending event. Stamp it
1909+
# with the current time (rather than the default epoch) so the
1910+
# audit event carries a meaningful timestamp, consistent with
1911+
# other backend-generated events.
1912+
rewind_timestamp = timestamp_pb2.Timestamp()
1913+
rewind_timestamp.FromDatetime(datetime.now(timezone.utc))
19051914
rewind_event = pb.HistoryEvent(
19061915
eventId=-1,
1907-
timestamp=timestamp_pb2.Timestamp(),
1916+
timestamp=rewind_timestamp,
19081917
executionRewound=pb.ExecutionRewoundEvent(
19091918
reason=wrappers_pb2.StringValue(value=reason) if reason else None,
19101919
),
@@ -1977,22 +1986,18 @@ def _process_rewind_orchestration_action(
19771986
instance.instance_id, sub_instance_id, task_id)
19781987

19791988
# Re-enqueue so the orchestration replays with the clean history.
1980-
# The executionRewound event is added to pending_events so the
1981-
# worker can see it in new_events; the worker uses the presence
1982-
# of executionRewound in old_events (history) to distinguish
1983-
# this normal post-rewind replay from the initial rewind
1984-
# short-circuit. Note: we do NOT add orchestratorStarted here
1985-
# because the work-item dispatch loop already inserts one when
1986-
# the instance has non-empty history.
1987-
rewind_event = None
1988-
for event in new_history:
1989-
if event.HasField("executionRewound"):
1990-
rewind_event = event
1991-
break
1989+
# The executionRewound event is already present in the clean
1990+
# history (it was kept by the SDK's rewind rewrite), so it must
1991+
# NOT be re-sent as a pending event — doing so would duplicate it
1992+
# in the instance history once the dispatched events are
1993+
# committed. A lone orchestratorStarted is enough to make the
1994+
# instance dispatchable for the post-rewind replay; the worker
1995+
# replays normally because executionCompleted is no longer in the
1996+
# history.
19921997
instance.pending_events.clear()
19931998
instance.dispatched_events.clear()
1994-
if rewind_event is not None:
1995-
instance.pending_events.append(rewind_event)
1999+
instance.pending_events.append(
2000+
helpers.new_orchestrator_started_event(datetime.now(timezone.utc)))
19962001
instance.completion_token = self._next_completion_token
19972002
self._next_completion_token += 1
19982003
self._orchestration_in_flight.discard(instance.instance_id)

durabletask/worker.py

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2142,8 +2142,8 @@ def execute(
21422142

21432143
# Check for rewind BEFORE replay. A rewind is indicated by an
21442144
# executionRewound event in new_events. We look for an
2145-
# executionCompleted event anywhere in the history (old or new
2146-
# events) to decide whether to rewind or replay:
2145+
# executionCompleted event in the committed history (old_events)
2146+
# to decide whether to rewind or replay:
21472147
# 1. executionCompleted IS present → the orchestration reached a
21482148
# terminal state (e.g. failed). This is a *new* rewind that
21492149
# the worker must short-circuit by building clean history.
@@ -2252,6 +2252,14 @@ def _build_rewind_result(
22522252
event is kept so the backend can identify which sub-orchestration
22532253
instances to recursively rewind.
22542254
2255+
Known limitation (shared with the Core/.NET rewind): timer events
2256+
(``timerCreated`` / ``timerFired``) emitted between retry attempts
2257+
of an activity scheduled with a ``RetryPolicy`` are not removed.
2258+
On the next replay the regenerated ``scheduleTask`` action may not
2259+
line up with those retained timer events, which can surface as a
2260+
non-determinism mismatch. Rewinding an activity that used a retry
2261+
policy is therefore not currently supported.
2262+
22552263
WARNING!!!:
22562264
If any changes are made to how this method modifies the orchestration's history, then corresponding changes *must*
22572265
be made in the backend implementations that rely on this method for executing a rewind.
@@ -2272,14 +2280,12 @@ def _build_rewind_result(
22722280
new_execution_id = uuid.uuid4().hex
22732281

22742282
# First pass: collect the task-scheduled IDs that correspond to
2275-
# failed activities / sub-orchestrations so we can remove the
2276-
# matching taskScheduled events in the second pass.
2283+
# failed activities so we can remove the matching taskScheduled
2284+
# events in the second pass.
22772285
failed_task_ids: set[int] = set()
22782286
for event in all_events:
22792287
if event.HasField("taskFailed"):
22802288
failed_task_ids.add(event.taskFailed.taskScheduledId)
2281-
elif event.HasField("subOrchestrationInstanceFailed"):
2282-
failed_task_ids.add(event.subOrchestrationInstanceFailed.taskScheduledId)
22832289

22842290
# Second pass: build the clean history.
22852291
clean_history: list[pb.HistoryEvent] = []
@@ -2301,10 +2307,15 @@ def _build_rewind_result(
23012307
event_copy.CopyFrom(event)
23022308
event_copy.executionStarted.orchestrationInstance.executionId.CopyFrom(
23032309
ph.get_string_value_or_empty(new_execution_id))
2304-
if rewind_event.HasField("parentExecutionId"):
2305-
if rewind_event.parentExecutionId.value:
2306-
event_copy.executionStarted.parentInstance.orchestrationInstance.executionId.CopyFrom(
2307-
rewind_event.parentExecutionId)
2310+
# Only update the parent's execution ID when this is
2311+
# actually a sub-orchestration (its executionStarted has a
2312+
# parentInstance). Writing through parentInstance for a
2313+
# top-level orchestration would materialize an empty one.
2314+
if (rewind_event.HasField("parentExecutionId")
2315+
and rewind_event.parentExecutionId.value
2316+
and event_copy.executionStarted.HasField("parentInstance")):
2317+
event_copy.executionStarted.parentInstance.orchestrationInstance.executionId.CopyFrom(
2318+
rewind_event.parentExecutionId)
23082319
clean_history.append(event_copy)
23092320
continue
23102321

tests/durabletask-azuremanaged/test_dts_rewind_e2e.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
# docker run -i -p 8080:8080 -p 8082:8082 -d mcr.microsoft.com/dts/dts-emulator:latest
1515
pytestmark = [
1616
pytest.mark.dts,
17-
pytest.mark.skip(reason="Rewind support is not yet available in the public DTS emulator"),
1817
]
1918

2019
# Read the environment variables

tests/durabletask/test_build_rewind_result.py

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@
1313
``executionStarted`` copy is updated accordingly.
1414
* ``taskFailed`` events and their corresponding ``taskScheduled``
1515
events are removed.
16-
* ``subOrchestrationInstanceFailed`` events and their corresponding
17-
``taskScheduled`` events are removed.
16+
* ``subOrchestrationInstanceFailed`` events are removed. Sub-orchestrations
17+
are represented by ``subOrchestrationInstanceCreated`` (not
18+
``taskScheduled``), so no ``taskScheduled`` event is dropped for them.
1819
* ``subOrchestrationInstanceCreated`` events for failed sub-orchestrations
1920
are kept so the backend can recursively rewind them.
2021
* ``executionCompleted`` events are removed.
@@ -344,8 +345,8 @@ def test_rewind_preserves_successful_activity():
344345

345346

346347
def test_rewind_removes_failed_sub_orch_events():
347-
"""subOrchestrationInstanceFailed and its corresponding taskScheduled
348-
should be removed, but subOrchestrationInstanceCreated is kept."""
348+
"""subOrchestrationInstanceFailed should be removed, but
349+
subOrchestrationInstanceCreated is kept."""
349350
executor = _make_executor()
350351

351352
old_events = [
@@ -373,14 +374,10 @@ def test_rewind_removes_failed_sub_orch_events():
373374
assert not any(
374375
e.HasField("subOrchestrationInstanceFailed") for e in clean
375376
)
376-
# The corresponding taskScheduled (eventId == 1) used by
377-
# subOrchestrationInstanceFailed should also be removed, since
378-
# _build_rewind_result collects the taskScheduledId from
379-
# subOrchestrationInstanceFailed too.
380-
# Note: subOrchestrationInstanceCreated uses a separate event with
381-
# its own eventId and is NOT removed.
377+
# Sub-orchestrations are represented by subOrchestrationInstanceCreated,
378+
# not taskScheduled, so there is no taskScheduled event to remove here.
382379
assert not any(
383-
e.HasField("taskScheduled") and e.eventId == 1 for e in clean
380+
e.HasField("taskScheduled") for e in clean
384381
)
385382
# The subOrchestrationInstanceCreated event should be preserved
386383
# so the backend can identify which sub-orchestration to rewind.

tests/durabletask/test_rewind_e2e.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@
88
from durabletask import client, task, worker
99
from durabletask.testing import create_test_backend
1010

11-
HOST = "localhost:50055"
11+
from tests.durabletask._port_utils import find_free_port
12+
13+
PORT = find_free_port()
14+
HOST = f"localhost:{PORT}"
1215

1316

1417
@pytest.fixture(autouse=True)
1518
def backend():
1619
"""Create an in-memory backend for testing."""
17-
b = create_test_backend(port=50055)
20+
b = create_test_backend(port=PORT)
1821
yield b
1922
b.stop()
2023
b.reset()

0 commit comments

Comments
 (0)