Skip to content

Commit 6573546

Browse files
committed
Stop actors when dispatch merge identities change
Remember each running dispatch's routed identity. If an update changes it, stop the old actor unless another running dispatch still uses it. Use the merge strategy's existing filter for managed actors and clear stale retries. Signed-off-by: Mathias L. Baumann <mathias.baumann@frequenz.com>
1 parent f43adbf commit 6573546

6 files changed

Lines changed: 441 additions & 32 deletions

File tree

‎RELEASE_NOTES.md‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
11
# Dispatch Highlevel Interface Release Notes
22

3-
## Enhancements
3+
## Summary
44

5-
- Improved docstring documentation across the project.
5+
<!-- Here goes a general summary of what this release is about -->
6+
7+
## Upgrading
8+
9+
<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
10+
11+
## New Features
12+
13+
<!-- Here goes the main new features and examples or instructions on how to use them -->
614

715
## Bug Fixes
816

9-
* Fixed documentation cross-references to dependencies by updating the mkdocstrings inventories to match the minimum dependency versions and adding the missing `frequenz-client-base` inventory.
10-
* Declared the missing direct dependencies `frequenz-client-common`, `frequenz-core` and `grpcio`, which were previously only pulled in indirectly through `frequenz-client-dispatch`.
17+
* Fixed actors being left running when an update changes a dispatch's merge identity.

‎src/frequenz/dispatch/_actor_dispatcher.py‎

Lines changed: 89 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66
import asyncio
77
import logging
8-
from collections.abc import Callable
8+
from collections.abc import Callable, Mapping
9+
from copy import copy
910
from dataclasses import dataclass
1011
from datetime import timedelta
1112
from typing import Any, Awaitable
@@ -19,6 +20,7 @@
1920
from typing_extensions import deprecated
2021

2122
from ._dispatch import Dispatch
23+
from ._merge_helpers import _other_running_dispatches
2224

2325
_logger = logging.getLogger(__name__)
2426

@@ -102,6 +104,7 @@ def __init__(
102104
object.__setattr__(self, "_src", _src)
103105

104106

107+
# pylint: disable=too-many-instance-attributes
105108
class ActorDispatcher(BackgroundService):
106109
"""A helper class to manage actors based on dispatches.
107110
@@ -246,16 +249,34 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen
246249
self._actors: dict[DispatchActorId, ActorDispatcher.ActorAndChannel] = {}
247250
self._failed_dispatches: dict[DispatchActorId, Dispatch] = {}
248251
"""Failed dispatches that will be retried later."""
252+
self._dispatch_identities: dict[DispatchId, DispatchActorId] = {}
253+
"""The identity to which each running dispatch was last routed."""
254+
self._running_dispatches: dict[DispatchId, Dispatch] = {}
255+
"""The latest version of each running dispatch."""
256+
self._dispatch_filter: (
257+
Callable[[Mapping[DispatchId, Dispatch], Dispatch], bool] | None
258+
) = None
259+
self._known_dispatches: Mapping[DispatchId, Dispatch] | None = None
249260

250261
def start(self) -> None:
251262
"""Start the background service."""
252263
self._tasks.add(asyncio.create_task(self._run()))
253264

254-
async def _start_actor(self, dispatch: Dispatch) -> None:
265+
def _set_merge_context(
266+
self,
267+
dispatch_filter: Callable[[Mapping[DispatchId, Dispatch], Dispatch], bool],
268+
known_dispatches: Mapping[DispatchId, Dispatch],
269+
) -> None:
270+
"""Set the merge filter and its complete dispatch context."""
271+
self._dispatch_filter = dispatch_filter
272+
self._known_dispatches = known_dispatches
273+
274+
async def _start_actor(self, dispatch: Dispatch, identity: DispatchActorId) -> None:
255275
"""Start the actor the given dispatch refers to.
256276
257277
Args:
258278
dispatch: The dispatch to start the actor for.
279+
identity: The identity to route the dispatch to.
259280
"""
260281
dispatch_update = DispatchInfo(
261282
target=dispatch.target,
@@ -264,7 +285,6 @@ async def _start_actor(self, dispatch: Dispatch) -> None:
264285
_src=dispatch,
265286
)
266287

267-
identity = self._dispatch_identity(dispatch)
268288
actor_and_channel = self._actors.get(identity)
269289

270290
if actor_and_channel:
@@ -301,15 +321,19 @@ async def _start_actor(self, dispatch: Dispatch) -> None:
301321
actor=actor, channel=channel, sender=channel.new_sender()
302322
)
303323

304-
async def _stop_actor(self, stopping_dispatch: Dispatch, msg: str) -> None:
324+
async def _stop_actor(
325+
self,
326+
stopping_dispatch: Dispatch,
327+
identity: DispatchActorId,
328+
msg: str,
329+
) -> None:
305330
"""Stop the actor for the given dispatch.
306331
307332
Args:
308333
stopping_dispatch: The dispatch that is stopping the actor.
334+
identity: The identity of the actor to stop.
309335
msg: The message to be passed to the actors being stopped.
310336
"""
311-
identity = self._dispatch_identity(stopping_dispatch)
312-
313337
if actor_and_channel := self._actors.pop(identity, None):
314338
_logger.info("Stopping actor for dispatch type %r", stopping_dispatch.type)
315339
await actor_and_channel.actor.stop(msg)
@@ -339,6 +363,27 @@ async def _run(self) -> None:
339363
elif self._dispatch_rx.triggered(selected):
340364
await self._handle_dispatch(selected.message)
341365

366+
def _should_stop_actor(
367+
self,
368+
dispatch: Dispatch,
369+
identity: DispatchActorId,
370+
*,
371+
check_local_dispatches: bool = False,
372+
) -> bool:
373+
"""Return whether the actor for an identity should stop."""
374+
if self._dispatch_filter is not None and self._known_dispatches is not None:
375+
return self._dispatch_filter(self._known_dispatches, dispatch)
376+
377+
if not check_local_dispatches:
378+
return True
379+
380+
return not _other_running_dispatches(
381+
self._running_dispatches,
382+
dispatch.id,
383+
identity,
384+
self._dispatch_identity,
385+
)
386+
342387
async def _handle_dispatch(self, dispatch: Dispatch) -> None:
343388
"""Process a dispatch to start, update, or stop an actor.
344389
@@ -349,11 +394,43 @@ async def _handle_dispatch(self, dispatch: Dispatch) -> None:
349394
Args:
350395
dispatch: The dispatch to handle.
351396
"""
352-
identity = self._dispatch_identity(dispatch)
353-
if identity in self._failed_dispatches:
354-
self._failed_dispatches.pop(identity)
355-
356397
if dispatch.started:
357-
await self._start_actor(dispatch)
398+
identity = self._dispatch_identity(dispatch)
399+
previous_identity = self._dispatch_identities.get(dispatch.id)
400+
401+
if previous_identity is not None and previous_identity != identity:
402+
previous_dispatch = self._running_dispatches[dispatch.id]
403+
failed_dispatch = self._failed_dispatches.get(previous_identity)
404+
if failed_dispatch is not None and failed_dispatch.id == dispatch.id:
405+
self._failed_dispatches.pop(previous_identity)
406+
407+
stopping_dispatch = copy(previous_dispatch)
408+
# pylint: disable-next=protected-access
409+
stopping_dispatch._set_deleted()
410+
if self._should_stop_actor(
411+
stopping_dispatch,
412+
previous_identity,
413+
check_local_dispatches=True,
414+
):
415+
await self._stop_actor(
416+
dispatch,
417+
previous_identity,
418+
"Dispatch identity changed",
419+
)
420+
421+
self._dispatch_identities[dispatch.id] = identity
422+
self._running_dispatches[dispatch.id] = dispatch
423+
self._failed_dispatches.pop(identity, None)
424+
await self._start_actor(dispatch, identity)
358425
else:
359-
await self._stop_actor(dispatch, "Dispatch stopped")
426+
identity = self._dispatch_identities.pop(
427+
dispatch.id, self._dispatch_identity(dispatch)
428+
)
429+
self._running_dispatches.pop(dispatch.id, None)
430+
431+
failed_dispatch = self._failed_dispatches.get(identity)
432+
if failed_dispatch is not None and failed_dispatch.id == dispatch.id:
433+
self._failed_dispatches.pop(identity)
434+
435+
if self._should_stop_actor(dispatch, identity):
436+
await self._stop_actor(dispatch, identity, "Dispatch stopped")

‎src/frequenz/dispatch/_dispatcher.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,14 +368,23 @@ def id_identity(dispatch: Dispatch) -> DispatchActorId:
368368
actor_factory=actor_factory,
369369
running_status_receiver=await self.new_running_state_event_receiver(
370370
dispatch_type,
371-
merge_strategy=merge_strategy,
371+
merge_strategy=None,
372372
),
373373
dispatch_identity=(
374374
id_identity if merge_strategy is None else merge_strategy.identity
375375
),
376376
retry_interval=retry_interval,
377377
)
378378

379+
if merge_strategy is not None:
380+
# The filter needs the same complete context used by public receivers.
381+
# pylint: disable=protected-access
382+
dispatcher._set_merge_context(
383+
merge_strategy.filter,
384+
self._bg_service._dispatches,
385+
)
386+
# pylint: enable=protected-access
387+
379388
self._actor_dispatchers[dispatch_type] = dispatcher
380389
dispatcher.start()
381390

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# License: MIT
2+
# Copyright © 2026 Frequenz Energy-as-a-Service GmbH
3+
4+
"""Helpers shared by dispatch merge handling."""
5+
6+
from collections.abc import Callable, Mapping
7+
from datetime import datetime, timezone
8+
9+
from frequenz.client.dispatch.types import DispatchId
10+
11+
from ._dispatch import Dispatch
12+
13+
14+
def _other_running_dispatches(
15+
dispatches: Mapping[DispatchId, Dispatch],
16+
dispatch_id: DispatchId,
17+
identity: object,
18+
dispatch_identity: Callable[[Dispatch], object],
19+
*,
20+
now: datetime | None = None,
21+
) -> list[Dispatch]:
22+
"""Return other running dispatches mapped to an identity."""
23+
if now is None:
24+
now = datetime.now(tz=timezone.utc)
25+
return [
26+
dispatch
27+
for dispatch in dispatches.values()
28+
if (
29+
dispatch.id != dispatch_id
30+
and dispatch.started_at(now)
31+
and dispatch_identity(dispatch) == identity
32+
)
33+
]

‎src/frequenz/dispatch/_merge_strategies.py‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from ._actor_dispatcher import DispatchActorId
1616
from ._bg_service import MergeStrategy
1717
from ._dispatch import Dispatch
18+
from ._merge_helpers import _other_running_dispatches
1819

1920
_logger = logging.getLogger(__name__)
2021

@@ -61,19 +62,14 @@ def filter(
6162
_logger.debug("Keeping start event %s", dispatch.id)
6263
return True
6364

64-
running_dispatch_list = [
65-
existing_dispatch
66-
for existing_dispatch in dispatches.values()
67-
if (
68-
self.identity(existing_dispatch) == self.identity(dispatch)
69-
and existing_dispatch.id != dispatch.id
70-
)
71-
]
72-
73-
other_dispatches_running = any(
74-
running_dispatch.started_at(now)
75-
for running_dispatch in running_dispatch_list
65+
running_dispatch_list = _other_running_dispatches(
66+
dispatches,
67+
dispatch.id,
68+
self.identity(dispatch),
69+
self.identity,
70+
now=now,
7671
)
72+
other_dispatches_running = bool(running_dispatch_list)
7773

7874
_logger.debug(
7975
"%s stop event %s because other_dispatches_running=%s",

0 commit comments

Comments
 (0)