Skip to content

Commit b4270ee

Browse files
berndverstBernd VerstCopilot
authored
Index sandbox activity overlap validation (#205)
`build_sandbox_worker_profiles()` validated activity ownership by scanning every previously registered activity with `activities_overlap()` for each new activity, making sandbox worker profile construction O(n^2) in the number of registered activities. Replace the linear scan with `_ActivityOwnerIndex`, which keys owners on the normalized (casefolded) activity name and on the exact (name, version) pair, with `None` representing an unversioned registration. Overlap semantics are unchanged: an unversioned activity overlaps every version of the same name, identical name+version pairs overlap, and the same name with different explicit versions does not. Registration order is retained so the reported conflicting profile is the same one a linear scan would report, keeping the exception type and message byte-identical. No public API or observable behavior change, so no changelog entry. Co-authored-by: Bernd Verst <beverst@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bb48ba8-473f-4632-9551-9c8d3c6615de
1 parent 6a6c79a commit b4270ee

2 files changed

Lines changed: 220 additions & 6 deletions

File tree

durabletask-azuremanaged/durabletask/azuremanaged/preview/sandboxes/profile_builder.py

Lines changed: 92 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
from durabletask.azuremanaged.internal import sandbox_service_pb2 as pb
88
from durabletask.azuremanaged.preview.sandboxes.helpers import (
99
SandboxActivity,
10-
activities_overlap,
1110
format_activity,
1211
normalize_required,
1312
resolve_activities,
@@ -91,22 +90,109 @@ def _build_sandbox_worker_profile(
9190
return worker_profile
9291

9392

93+
class _ActivityOwnerSlot:
94+
"""Earliest worker profiles recorded for a single activity overlap bucket.
95+
96+
Only two owners ever need to be retained to answer "which worker profile
97+
first claimed an activity in this bucket, ignoring `worker_profile_id`":
98+
the very first owner, plus the first owner that differs from it.
99+
"""
100+
101+
__slots__ = ("_first", "_first_other")
102+
103+
def __init__(self) -> None:
104+
self._first: Optional[tuple[int, str]] = None
105+
self._first_other: Optional[tuple[int, str]] = None
106+
107+
def add(self, order: int, worker_profile_id: str) -> None:
108+
if self._first is None:
109+
self._first = (order, worker_profile_id)
110+
elif self._first_other is None and worker_profile_id != self._first[1]:
111+
self._first_other = (order, worker_profile_id)
112+
113+
def first_owner_other_than(self, worker_profile_id: str) -> Optional[tuple[int, str]]:
114+
if self._first is not None and self._first[1] != worker_profile_id:
115+
return self._first
116+
return self._first_other
117+
118+
119+
class _ActivityOwnerIndex:
120+
"""Indexes activity ownership so overlap checks cost O(1) per activity.
121+
122+
Reproduces the semantics of
123+
:func:`durabletask.azuremanaged.preview.sandboxes.helpers.activities_overlap`
124+
exactly: activity names are compared case insensitively, an unversioned
125+
activity overlaps every version of the same name, and two activities with
126+
the same name overlap when their explicit versions are equal. Registration
127+
order is tracked so the reported conflict matches the first overlapping
128+
activity, exactly as a linear scan would report it.
129+
"""
130+
131+
def __init__(self) -> None:
132+
self._registration_count = 0
133+
self._by_name: dict[str, _ActivityOwnerSlot] = {}
134+
self._by_name_and_version: dict[tuple[str, Optional[str]], _ActivityOwnerSlot] = {}
135+
136+
def find_conflicting_profile(
137+
self,
138+
activity: SandboxActivity,
139+
worker_profile_id: str) -> Optional[str]:
140+
"""Return the profile that first claimed an overlapping activity, if any."""
141+
name_key = activity.name.casefold()
142+
if activity.version is None:
143+
# An unversioned activity overlaps every version of the same name.
144+
owner = _first_owner_other_than(self._by_name.get(name_key), worker_profile_id)
145+
else:
146+
# A versioned activity overlaps the same version plus any
147+
# unversioned registration of the same name.
148+
owner = _earlier_owner(
149+
_first_owner_other_than(
150+
self._by_name_and_version.get((name_key, None)), worker_profile_id),
151+
_first_owner_other_than(
152+
self._by_name_and_version.get((name_key, activity.version)), worker_profile_id))
153+
return None if owner is None else owner[1]
154+
155+
def add(self, activity: SandboxActivity, worker_profile_id: str) -> None:
156+
"""Record `worker_profile_id` as an owner of `activity`."""
157+
name_key = activity.name.casefold()
158+
order = self._registration_count
159+
self._registration_count += 1
160+
self._by_name.setdefault(name_key, _ActivityOwnerSlot()).add(order, worker_profile_id)
161+
self._by_name_and_version.setdefault(
162+
(name_key, activity.version), _ActivityOwnerSlot()).add(order, worker_profile_id)
163+
164+
165+
def _first_owner_other_than(
166+
slot: Optional[_ActivityOwnerSlot],
167+
worker_profile_id: str) -> Optional[tuple[int, str]]:
168+
return None if slot is None else slot.first_owner_other_than(worker_profile_id)
169+
170+
171+
def _earlier_owner(
172+
left: Optional[tuple[int, str]],
173+
right: Optional[tuple[int, str]]) -> Optional[tuple[int, str]]:
174+
if left is None:
175+
return right
176+
if right is None:
177+
return left
178+
return left if left[0] <= right[0] else right
179+
180+
94181
def build_sandbox_worker_profiles() -> list[pb.SandboxWorkerProfile]:
95182
"""Build sandbox worker_profiles from worker profile configuration."""
96183
worker_profiles: list[pb.SandboxWorkerProfile] = []
97-
activity_owners: list[tuple[SandboxActivity, str]] = []
184+
activity_owners = _ActivityOwnerIndex()
98185
for profile in registered_sandbox_worker_profiles():
99186
activities = resolve_activities(profile.activities)
100187

101188
for activity in activities:
102-
existing_profile = next((owner_profile for owner_activity, owner_profile in activity_owners
103-
if activities_overlap(owner_activity, activity)
104-
and owner_profile != profile.worker_profile_id), None)
189+
existing_profile = activity_owners.find_conflicting_profile(
190+
activity, profile.worker_profile_id)
105191
if existing_profile:
106192
raise ValueError(
107193
f"Sandbox activity '{format_activity(activity)}' is assigned to both worker profile "
108194
f"'{existing_profile}' and '{profile.worker_profile_id}'.")
109-
activity_owners.append((activity, profile.worker_profile_id))
195+
activity_owners.add(activity, profile.worker_profile_id)
110196

111197
worker_profiles.append(_build_sandbox_worker_profile(
112198
activities=activities,

tests/durabletask-azuremanaged/test_sandboxes_extension.py

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
# Licensed under the MIT License.
33

44
import inspect
5+
import random
56
import threading
67

78
import grpc
@@ -20,6 +21,7 @@
2021
SandboxWorkerProfileImageOptions,
2122
)
2223
from durabletask.azuremanaged.preview.sandboxes.profile_builder import (
24+
_ActivityOwnerIndex,
2325
_build_sandbox_worker_profile,
2426
build_sandbox_worker_profiles,
2527
)
@@ -28,6 +30,7 @@
2830
build_sandbox_worker_start,
2931
)
3032
from durabletask.azuremanaged.preview.sandboxes.helpers import resolve_activities
33+
from durabletask.azuremanaged.preview.sandboxes.helpers import activities_overlap
3134
from durabletask.azuremanaged.preview.sandboxes.helpers import SandboxActivity
3235
from durabletask.azuremanaged.internal import sandbox_service_pb2 as pb
3336
from durabletask.azuremanaged.internal import sandbox_service_pb2_grpc as stubs
@@ -206,6 +209,9 @@ def configure(self, options: SandboxWorkerProfileOptions) -> None:
206209
try:
207210
build_sandbox_worker_profiles()
208211
except ValueError as ex:
212+
assert str(ex) == (
213+
"Sandbox activity 'pytestoverlapremotehello' is assigned to both worker profile "
214+
"'pytest-overlap-profile-a' and 'pytest-overlap-profile-b'.")
209215
assert "pytestoverlapremotehello" in str(ex)
210216
assert "pytest-overlap-profile-a" in str(ex)
211217
assert "pytest-overlap-profile-b" in str(ex)
@@ -252,6 +258,128 @@ def configure(self, options: SandboxWorkerProfileOptions) -> None:
252258
sandbox_worker_profiles._worker_profiles.pop("pytest-version-profile-b", None)
253259

254260

261+
def test_build_sandbox_worker_profiles_rejects_versioned_activity_overlapping_unversioned_owner() -> None:
262+
@sandbox_worker_profile("pytest-unversioned-owner-profile-a")
263+
class PytestUnversionedOwnerProfileA(SandboxWorkerProfile):
264+
def configure(self, options: SandboxWorkerProfileOptions) -> None:
265+
options.image.image_ref = "example.azurecr.io/python-worker-a:v1"
266+
options.image.managed_identity_client_id = "image-pull-client-id"
267+
options.scheduler_managed_identity_client_id = "scheduler-client-id"
268+
options.add_activity("PytestUnversionedOwner", version=None)
269+
270+
@sandbox_worker_profile("pytest-unversioned-owner-profile-b")
271+
class PytestUnversionedOwnerProfileB(SandboxWorkerProfile):
272+
def configure(self, options: SandboxWorkerProfileOptions) -> None:
273+
options.image.image_ref = "example.azurecr.io/python-worker-b:v1"
274+
options.image.managed_identity_client_id = "image-pull-client-id"
275+
options.scheduler_managed_identity_client_id = "scheduler-client-id"
276+
options.add_activity("pytestunversionedowner", version="v9")
277+
278+
try:
279+
try:
280+
build_sandbox_worker_profiles()
281+
except ValueError as ex:
282+
assert str(ex) == (
283+
"Sandbox activity 'pytestunversionedowner@v9' is assigned to both worker profile "
284+
"'pytest-unversioned-owner-profile-a' and 'pytest-unversioned-owner-profile-b'.")
285+
else:
286+
raise AssertionError(
287+
"Expected an unversioned sandbox activity to overlap every version of the same name.")
288+
finally:
289+
sandbox_worker_profiles._worker_profiles.pop("pytest-unversioned-owner-profile-a", None)
290+
sandbox_worker_profiles._worker_profiles.pop("pytest-unversioned-owner-profile-b", None)
291+
292+
293+
def test_build_sandbox_worker_profiles_rejects_unversioned_activity_overlapping_versioned_owner() -> None:
294+
@sandbox_worker_profile("pytest-versioned-owner-profile-a")
295+
class PytestVersionedOwnerProfileA(SandboxWorkerProfile):
296+
def configure(self, options: SandboxWorkerProfileOptions) -> None:
297+
options.image.image_ref = "example.azurecr.io/python-worker-a:v1"
298+
options.image.managed_identity_client_id = "image-pull-client-id"
299+
options.scheduler_managed_identity_client_id = "scheduler-client-id"
300+
options.add_activity("PytestVersionedOwner", version="v9")
301+
302+
@sandbox_worker_profile("pytest-versioned-owner-profile-b")
303+
class PytestVersionedOwnerProfileB(SandboxWorkerProfile):
304+
def configure(self, options: SandboxWorkerProfileOptions) -> None:
305+
options.image.image_ref = "example.azurecr.io/python-worker-b:v1"
306+
options.image.managed_identity_client_id = "image-pull-client-id"
307+
options.scheduler_managed_identity_client_id = "scheduler-client-id"
308+
options.add_activity("pytestversionedowner", version=None)
309+
310+
try:
311+
try:
312+
build_sandbox_worker_profiles()
313+
except ValueError as ex:
314+
assert str(ex) == (
315+
"Sandbox activity 'pytestversionedowner' is assigned to both worker profile "
316+
"'pytest-versioned-owner-profile-a' and 'pytest-versioned-owner-profile-b'.")
317+
else:
318+
raise AssertionError(
319+
"Expected an unversioned sandbox activity to overlap an existing versioned owner.")
320+
finally:
321+
sandbox_worker_profiles._worker_profiles.pop("pytest-versioned-owner-profile-a", None)
322+
sandbox_worker_profiles._worker_profiles.pop("pytest-versioned-owner-profile-b", None)
323+
324+
325+
def test_build_sandbox_worker_profiles_rejects_identical_activity_versions() -> None:
326+
@sandbox_worker_profile("pytest-same-version-profile-a")
327+
class PytestSameVersionProfileA(SandboxWorkerProfile):
328+
def configure(self, options: SandboxWorkerProfileOptions) -> None:
329+
options.image.image_ref = "example.azurecr.io/python-worker-a:v1"
330+
options.image.managed_identity_client_id = "image-pull-client-id"
331+
options.scheduler_managed_identity_client_id = "scheduler-client-id"
332+
options.add_activity("PytestSameVersionActivity", version="v3")
333+
334+
@sandbox_worker_profile("pytest-same-version-profile-b")
335+
class PytestSameVersionProfileB(SandboxWorkerProfile):
336+
def configure(self, options: SandboxWorkerProfileOptions) -> None:
337+
options.image.image_ref = "example.azurecr.io/python-worker-b:v1"
338+
options.image.managed_identity_client_id = "image-pull-client-id"
339+
options.scheduler_managed_identity_client_id = "scheduler-client-id"
340+
options.add_activity("pytestsameversionactivity", version="v3")
341+
342+
try:
343+
try:
344+
build_sandbox_worker_profiles()
345+
except ValueError as ex:
346+
assert str(ex) == (
347+
"Sandbox activity 'pytestsameversionactivity@v3' is assigned to both worker profile "
348+
"'pytest-same-version-profile-a' and 'pytest-same-version-profile-b'.")
349+
else:
350+
raise AssertionError("Expected identical sandbox activity versions to overlap.")
351+
finally:
352+
sandbox_worker_profiles._worker_profiles.pop("pytest-same-version-profile-a", None)
353+
sandbox_worker_profiles._worker_profiles.pop("pytest-same-version-profile-b", None)
354+
355+
356+
def test_activity_owner_index_matches_linear_overlap_scan() -> None:
357+
registrations = [
358+
(SandboxActivity(name, version), worker_profile_id)
359+
for name in ("Alpha", "alpha", "BETA", "Gamma")
360+
for version in (None, "v1", "v2", "V1")
361+
for worker_profile_id in ("profile-a", "profile-b", "profile-c")
362+
]
363+
364+
for seed in range(25):
365+
shuffled = list(registrations)
366+
random.Random(seed).shuffle(shuffled)
367+
368+
index = _ActivityOwnerIndex()
369+
owners: list[tuple[SandboxActivity, str]] = []
370+
for activity, worker_profile_id in shuffled:
371+
expected = next(
372+
(owner_profile for owner_activity, owner_profile in owners
373+
if activities_overlap(owner_activity, activity)
374+
and owner_profile != worker_profile_id),
375+
None)
376+
377+
assert index.find_conflicting_profile(activity, worker_profile_id) == expected
378+
379+
owners.append((activity, worker_profile_id))
380+
index.add(activity, worker_profile_id)
381+
382+
255383
def test_profile_options_add_activity_accepts_callable() -> None:
256384
def pytest_callable_remote_hello(_ctx, value):
257385
return value

0 commit comments

Comments
 (0)