Skip to content

Commit 8ea44a2

Browse files
Bernd VerstCopilot
andcommitted
Performance: reduce orchestration history replay allocations
Long orchestration histories were copied and rescanned several times per work item before and during replay: - `TaskHubGrpcWorker._execute_orchestrator()` built `list(req.pastEvents) + list(req.newEvents)` (three throwaway lists sized by the full history) purely to locate the first `executionStarted` event. It now iterates `itertools.chain(...)` and still breaks at the first match. - `_OrchestrationExecutor.execute()` materialized every `executionStarted` event in the committed history but only read element `[0]`. It now uses `next(...)` over a generator, short-circuiting at the first match and falling back to `"<unknown>"` exactly as before. - `_OrchestrationExecutor._build_rewind_result()` built a combined `list(old_events) + list(new_events)` for its two passes. Both inputs are sequences, so each pass now chains them lazily in the same order. Replay order, rewind handling, non-determinism detection, log messages and exception types/messages are all unchanged; this is an allocation-only change with no observable behavior difference. Adds regression tests covering both branches of the orchestration-name lookup (resolved from committed history, and the "<unknown>" fallback). Fixes #185 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6a6454c-5bdc-4bc7-9059-a2789a65e271
1 parent 4d87430 commit 8ea44a2

2 files changed

Lines changed: 59 additions & 10 deletions

File tree

‎durabletask/worker.py‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import asyncio
55
import inspect
6+
import itertools
67
import json
78
import logging
89
import os
@@ -1132,10 +1133,12 @@ def _execute_orchestrator(
11321133
if self._payload_store is not None:
11331134
payload_helpers.deexternalize_payloads(req, self._payload_store)
11341135

1135-
# Extract parent trace context from executionStarted event
1136+
# Extract parent trace context from executionStarted event.
1137+
# Chain the two event lists lazily so that long histories are not
1138+
# copied just to locate the first executionStarted event.
11361139
parent_trace_ctx = None
11371140
orchestration_name = "<unknown>"
1138-
for e in list(req.pastEvents) + list(req.newEvents):
1141+
for e in itertools.chain(req.pastEvents, req.newEvents):
11391142
if e.HasField("executionStarted"):
11401143
orchestration_name = e.executionStarted.name
11411144
if e.executionStarted.HasField("parentTraceContext"):
@@ -2132,10 +2135,12 @@ def execute(
21322135
old_events: Sequence[pb.HistoryEvent],
21332136
new_events: Sequence[pb.HistoryEvent],
21342137
) -> ExecutionResults:
2135-
orchestration_name = "<unknown>"
2136-
orchestration_started_events = [e for e in old_events if e.HasField("executionStarted")]
2137-
if len(orchestration_started_events) >= 1:
2138-
orchestration_name = orchestration_started_events[0].executionStarted.name
2138+
# Only the first executionStarted event is needed, so stop at the
2139+
# first match instead of materializing every matching event.
2140+
orchestration_name = next(
2141+
(e.executionStarted.name for e in old_events if e.HasField("executionStarted")),
2142+
"<unknown>",
2143+
)
21392144
self._orchestration_name = orchestration_name
21402145

21412146
self._logger.debug(
@@ -2286,21 +2291,22 @@ def _build_rewind_result(
22862291

22872292
rewind_event: pb.ExecutionRewoundEvent = new_events[1].executionRewound
22882293

2289-
all_events = list(old_events) + list(new_events)
22902294
# Generate a new execution ID for the rewound execution.
22912295
new_execution_id = uuid.uuid4().hex
22922296

22932297
# First pass: collect the task-scheduled IDs that correspond to
22942298
# failed activities so we can remove the matching taskScheduled
2295-
# events in the second pass.
2299+
# events in the second pass. Both arguments are sequences, so each
2300+
# pass chains them lazily (old events first, then new events)
2301+
# instead of copying the whole history into a combined list.
22962302
failed_task_ids: set[int] = set()
2297-
for event in all_events:
2303+
for event in itertools.chain(old_events, new_events):
22982304
if event.HasField("taskFailed"):
22992305
failed_task_ids.add(event.taskFailed.taskScheduledId)
23002306

23012307
# Second pass: build the clean history.
23022308
clean_history: list[pb.HistoryEvent] = []
2303-
for event in all_events:
2309+
for event in itertools.chain(old_events, new_events):
23042310
if event.HasField("taskFailed"):
23052311
continue
23062312
if event.HasField("taskScheduled") and event.eventId in failed_task_ids:

‎tests/durabletask/test_orchestration_executor.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,49 @@ def orchestrator(ctx: task.OrchestrationContext, _):
9393
assert observed["parent"] is None
9494

9595

96+
def test_orchestration_name_resolved_from_committed_history():
97+
"""The orchestration name is taken from the first executionStarted event in old events."""
98+
99+
def dummy_activity(ctx, _):
100+
pass
101+
102+
def orchestrator(ctx: task.OrchestrationContext, _):
103+
result = yield ctx.call_activity(dummy_activity, input=None)
104+
return result
105+
106+
registry = worker._Registry()
107+
name = registry.add_orchestrator(orchestrator)
108+
109+
# executionStarted is not the first event, so the whole prefix of the
110+
# committed history has to be scanned before the name is found.
111+
old_events = [
112+
helpers.new_orchestrator_started_event(),
113+
helpers.new_execution_started_event(name, TEST_INSTANCE_ID, encoded_input=None),
114+
helpers.new_task_scheduled_event(1, task.get_name(dummy_activity))]
115+
new_events = [helpers.new_task_completed_event(1, json.dumps("done!"))]
116+
117+
executor = worker._OrchestrationExecutor(registry, TEST_LOGGER, JsonDataConverter())
118+
executor.execute(TEST_INSTANCE_ID, old_events, new_events)
119+
120+
assert executor._orchestration_name == name
121+
122+
123+
def test_orchestration_name_unknown_without_committed_history():
124+
"""The orchestration name falls back to '<unknown>' when old events carry no executionStarted."""
125+
126+
def orchestrator(ctx: task.OrchestrationContext, _):
127+
return "done"
128+
129+
registry = worker._Registry()
130+
name = registry.add_orchestrator(orchestrator)
131+
132+
new_events = [helpers.new_execution_started_event(name, TEST_INSTANCE_ID, encoded_input=None)]
133+
executor = worker._OrchestrationExecutor(registry, TEST_LOGGER, JsonDataConverter())
134+
executor.execute(TEST_INSTANCE_ID, [], new_events)
135+
136+
assert executor._orchestration_name == "<unknown>"
137+
138+
96139
def test_complete_orchestration_actions():
97140
"""Tests the actions output for a completed orchestration"""
98141

0 commit comments

Comments
 (0)