Skip to content

Commit 0fa93f9

Browse files
berndverstBernd VerstCopilot
authored
Performance [P1]: Bound async worker task creation (#200)
* Bound async worker task creation to the concurrency limit `_AsyncWorkerManager._consume_queue()` created an `asyncio.Task` for every dequeued work item and only acquired the concurrency semaphore later, inside `_process_work_item()`. The semaphore therefore throttled execution but not allocation, so during a burst with slow handlers the number of pending task objects grew with the queue depth rather than with the configured concurrency. The loop also rescanned the entire `running_tasks` set on every iteration. The consumer now acquires a semaphore permit *before* dequeuing an item and hands that permit off to the task it creates, which releases it once the work item is done. The queue stays the bounded backlog and in-flight work item tasks never exceed the configured `ConcurrencyOptions` limits. Completed tasks are removed from `running_tasks` via `task.add_done_callback(set.discard)` instead of an O(n) sweep. Semantics are preserved: there is still no await between `queue.get()` returning and `asyncio.create_task()`, so an item can never be dropped by cancellation; the permit is released if the dequeue times out or raises; and `queue.task_done()` is still called exactly once per item, before the permit is released, on both the success and failure paths. Fixes #193 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 010b6832-802b-425d-957c-cf29b7143392 * Clarify that the queue-readiness helper returns a coroutine The second element of the tuple returned by _start_manager_and_wait_for_queues is an already-created coroutine object, not a callable. Naming the local wait_for_queues made �wait wait_for_queues read like a missing call -- it misled an automated reviewer and would mislead a human too. Rename the local to queues_ready and say so in the docstring. No behavior change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 010b6832-802b-425d-957c-cf29b7143392 --------- Co-authored-by: Bernd Verst <beverst@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 010b6832-802b-425d-957c-cf29b7143392
1 parent fc288ad commit 0fa93f9

3 files changed

Lines changed: 172 additions & 20 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ CHANGED
1717
- **Breaking:** `FailureDetails.error_type` — and the `errorType` value sent over the wire — is now the fully-qualified type name (`module.ClassName`, e.g. `builtins.ValueError`, `durabletask.task.TaskFailedError`) instead of the bare class name, matching the .NET and Java SDKs. Code that compared `error_type` against a bare name (for example `== "ValueError"`) must be updated to the qualified name or, preferably, switched to `FailureDetails.is_caused_by()`. Because this value is persisted and crosses the orchestration boundary, failures produced by older workers may still carry a bare name; `is_caused_by()` accepts both.
1818
- **Breaking:** Retired the process-global history-export context. `durabletask.extensions.history_export.bind_context()` and `clear_context()` have been removed; the export activities now resolve their `HistoryExportContext` per invocation via a resolver captured at registration. `ExportHistoryClient.register_worker()` continues to wire this up automatically, so most callers are unaffected. Code that called `bind_context(HistoryExportContext(client, writer))` directly should instead register the activities with `durabletask.extensions.history_export.activities.register(worker, lambda: HistoryExportContext(client, writer))` (or a lazier resolver), or use `ExportHistoryClient.register_worker()`.
1919

20+
FIXED
21+
22+
- Fixed the worker allocating one `asyncio` task per queued work item before applying the concurrency limit, which made memory use and event-loop scheduling overhead grow with the queue backlog during bursts. In-flight work item tasks are now bounded by the configured `ConcurrencyOptions` limits.
23+
2024
## v1.8.0
2125

2226
ADDED

durabletask/worker.py

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3440,43 +3440,57 @@ async def run(self) -> None:
34403440
self._pool_is_shutdown = True
34413441

34423442
async def _consume_queue(self, queue: asyncio.Queue[_WorkItem], semaphore: asyncio.Semaphore) -> None:
3443-
# List to track running tasks
3443+
# Set of running tasks. Completed tasks remove themselves through a done
3444+
# callback so that this loop never has to scan the whole set.
34443445
running_tasks: set[asyncio.Task[Any]] = set()
34453446

34463447
while True:
3447-
# Clean up completed tasks
3448-
done_tasks = {task for task in running_tasks if task.done()}
3449-
running_tasks -= done_tasks
3450-
34513448
# Exit if shutdown is set and the queue is empty and no tasks are running
34523449
if self._shutdown and queue.empty() and not running_tasks:
34533450
break
34543451

3452+
# Reserve concurrency capacity *before* dequeuing a work item so that
3453+
# the number of allocated tasks stays bounded by the configured
3454+
# concurrency limit instead of growing with the queue depth. The
3455+
# permit is handed off to the task created below, which releases it
3456+
# once the work item has been processed.
3457+
await semaphore.acquire()
3458+
permit_owned = True
34553459
try:
3456-
work = await asyncio.wait_for(queue.get(), timeout=1.0)
3457-
except asyncio.TimeoutError:
3458-
continue
3460+
try:
3461+
work = await asyncio.wait_for(queue.get(), timeout=1.0)
3462+
except asyncio.TimeoutError:
3463+
continue
34593464

3460-
func, cancellation_func, args, kwargs = work
3461-
# Create a concurrent task for processing
3462-
task = asyncio.create_task(
3463-
self._process_work_item(semaphore, queue, func, cancellation_func, args, kwargs)
3464-
)
3465-
running_tasks.add(task)
3465+
func, cancellation_func, args, kwargs = work
3466+
# Create a concurrent task for processing
3467+
task = asyncio.create_task(
3468+
self._process_work_item(semaphore, queue, func, cancellation_func, args, kwargs)
3469+
)
3470+
permit_owned = False
3471+
running_tasks.add(task)
3472+
task.add_done_callback(running_tasks.discard)
3473+
finally:
3474+
if permit_owned:
3475+
semaphore.release()
34663476

34673477
async def _process_work_item(
34683478
self, semaphore: asyncio.Semaphore, queue: asyncio.Queue[_WorkItem],
34693479
func: Callable[..., Any], cancellation_func: Callable[..., Any],
34703480
args: tuple[Any, ...], kwargs: dict[str, Any]
34713481
) -> None:
3472-
async with semaphore:
3482+
# The semaphore permit is acquired by ``_consume_queue`` before this task
3483+
# is created and is released here once the work item is done.
3484+
try:
3485+
await self._run_func(func, *args, **kwargs)
3486+
except Exception as work_exception:
3487+
self._logger.error(f"Uncaught error while processing work item, item will be abandoned: {work_exception}")
3488+
await self._run_func(cancellation_func, *args, **kwargs)
3489+
finally:
34733490
try:
3474-
await self._run_func(func, *args, **kwargs)
3475-
except Exception as work_exception:
3476-
self._logger.error(f"Uncaught error while processing work item, item will be abandoned: {work_exception}")
3477-
await self._run_func(cancellation_func, *args, **kwargs)
3478-
finally:
34793491
queue.task_done()
3492+
finally:
3493+
semaphore.release()
34803494

34813495
async def _run_func(self, func: Callable[..., Any], *args: Any, **kwargs: Any) -> Any:
34823496
if inspect.iscoroutinefunction(func):

tests/durabletask/test_worker_concurrency_loop_async.py

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,137 @@ async def run_test():
9595
await worker_task
9696
asyncio.run(run_test())
9797
asyncio.run(run_test())
98+
99+
100+
def _start_manager_and_wait_for_queues(manager):
101+
"""Start the manager loop and return an awaitable that resolves once its queues exist.
102+
103+
The second element of the returned tuple is an already-created coroutine
104+
object (a readiness awaitable), not a callable -- await it directly.
105+
"""
106+
worker_task = asyncio.create_task(manager.run())
107+
108+
async def wait_for_queues():
109+
for _ in range(100):
110+
if manager.activity_queue is not None:
111+
return
112+
await asyncio.sleep(0.01)
113+
raise RuntimeError("Worker manager queues were never initialized")
114+
115+
return worker_task, wait_for_queues()
116+
117+
118+
def test_async_worker_manager_bounds_task_allocation():
119+
"""Work item tasks must be allocated only up to the concurrency limit.
120+
121+
The concurrency semaphore is acquired before a task is created, so a burst
122+
of queued work items must not allocate one asyncio.Task per queued item.
123+
"""
124+
limit = 2
125+
total_items = 25
126+
options = ConcurrencyOptions(
127+
maximum_concurrent_activity_work_items=limit,
128+
maximum_concurrent_orchestration_work_items=limit,
129+
maximum_concurrent_entity_work_items=limit,
130+
maximum_thread_pool_workers=limit,
131+
)
132+
manager = TaskHubGrpcWorker(concurrency_options=options)._async_worker_manager
133+
134+
state = {"allocated": 0, "peak_allocated": 0, "running": 0, "peak_running": 0}
135+
completed = []
136+
original_process_work_item = manager._process_work_item
137+
138+
def counting_process_work_item(*args, **kwargs):
139+
# The coroutine object is created synchronously by asyncio.create_task,
140+
# so this counts allocated work item tasks, not just started ones.
141+
state["allocated"] += 1
142+
state["peak_allocated"] = max(state["peak_allocated"], state["allocated"])
143+
inner = original_process_work_item(*args, **kwargs)
144+
145+
async def tracked():
146+
try:
147+
return await inner
148+
finally:
149+
state["allocated"] -= 1
150+
151+
return tracked()
152+
153+
manager._process_work_item = counting_process_work_item
154+
155+
async def slow_work(idx):
156+
state["running"] += 1
157+
state["peak_running"] = max(state["peak_running"], state["running"])
158+
await asyncio.sleep(0.02)
159+
state["running"] -= 1
160+
completed.append(idx)
161+
162+
async def cancel_work(idx):
163+
pass
164+
165+
async def run_test():
166+
manager.prepare_for_run()
167+
worker_task, queues_ready = _start_manager_and_wait_for_queues(manager)
168+
await queues_ready
169+
assert manager.activity_queue is not None
170+
for i in range(total_items):
171+
manager.submit_activity(slow_work, cancel_work, i)
172+
await asyncio.wait_for(manager.activity_queue.join(), timeout=60)
173+
manager.shutdown()
174+
await asyncio.wait_for(worker_task, timeout=60)
175+
176+
asyncio.run(run_test())
177+
178+
assert sorted(completed) == list(range(total_items))
179+
assert state["peak_allocated"] <= limit, (
180+
f"Expected at most {limit} concurrently allocated work item tasks, "
181+
f"got {state['peak_allocated']}"
182+
)
183+
# The concurrency limit must still be fully usable.
184+
assert state["peak_running"] == limit, (
185+
f"Expected {limit} concurrently running work items, got {state['peak_running']}"
186+
)
187+
assert state["allocated"] == 0
188+
189+
190+
def test_async_worker_manager_calls_task_done_exactly_once_on_failure():
191+
"""Failing work items must still mark the queue item done exactly once."""
192+
total_items = 6
193+
options = ConcurrencyOptions(
194+
maximum_concurrent_activity_work_items=2,
195+
maximum_concurrent_orchestration_work_items=2,
196+
maximum_concurrent_entity_work_items=2,
197+
maximum_thread_pool_workers=2,
198+
)
199+
manager = TaskHubGrpcWorker(concurrency_options=options)._async_worker_manager
200+
cancelled = []
201+
task_done_calls = []
202+
203+
async def failing_work(idx):
204+
raise RuntimeError(f"boom {idx}")
205+
206+
async def cancel_work(idx):
207+
cancelled.append(idx)
208+
209+
async def run_test():
210+
manager.prepare_for_run()
211+
worker_task, queues_ready = _start_manager_and_wait_for_queues(manager)
212+
await queues_ready
213+
queue = manager.activity_queue
214+
assert queue is not None
215+
original_task_done = queue.task_done
216+
217+
def counting_task_done():
218+
task_done_calls.append(1)
219+
original_task_done()
220+
221+
queue.task_done = counting_task_done # type: ignore[method-assign]
222+
for i in range(total_items):
223+
manager.submit_activity(failing_work, cancel_work, i)
224+
await asyncio.wait_for(queue.join(), timeout=60)
225+
manager.shutdown()
226+
await asyncio.wait_for(worker_task, timeout=60)
227+
228+
asyncio.run(run_test())
229+
230+
assert sorted(cancelled) == list(range(total_items))
231+
assert len(task_done_calls) == total_items

0 commit comments

Comments
 (0)