Skip to content

Commit b4d28e3

Browse files
pablomhcursoragent
andcommitted
Fixes: #3322 - Narrow distribution task locks for unchanged base_path
Only reserve the domain-wide distributions resource for operations that can affect base_path overlap validation. Distribution creates, deletes, and updates that change base_path continue to reserve `pdrn:<domain>:distributions`, while updates that leave base_path unchanged now reserve only the distribution instance itself. This reduces unnecessary serialization of `ageneral_update` tasks for ordinary distribution updates, including partial PATCH requests that omit base_path or send the existing base_path unchanged. Add a functional test covering the reserved resources used for create, partial update without base_path, partial update with unchanged base_path, partial update with changed base_path, and delete. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 9e56a1e commit b4d28e3

3 files changed

Lines changed: 84 additions & 5 deletions

File tree

CHANGES/7896.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Reduced lock contention for distribution updates that leave `base_path` unchanged.

pulpcore/app/viewsets/publication.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -527,8 +527,26 @@ def get_queryset(self):
527527
return qs
528528

529529
def async_reserved_resources(self, instance):
530-
"""Return resource that locks all Distributions."""
531-
return [f"pdrn:{get_domain().pulp_id}:distributions"]
530+
"""
531+
Reserve the narrowest safe lock for async distribution operations.
532+
533+
Creates, deletes, and base_path changes still lock the domain-wide distributions resource
534+
because base_path overlap validation is domain scoped. Other updates only need to lock the
535+
specific distribution instance.
536+
"""
537+
domain_distributions = f"pdrn:{get_domain().pulp_id}:distributions"
538+
if instance is None:
539+
return [domain_distributions]
540+
541+
if getattr(self, "action", "") == "destroy":
542+
return [instance, domain_distributions]
543+
544+
request_data = getattr(getattr(self, "request", None), "data", {})
545+
requested_base_path = request_data.get("base_path", instance.base_path)
546+
if requested_base_path == instance.base_path:
547+
return [instance]
548+
549+
return [instance, domain_distributions]
532550

533551

534552
class ListDistributionViewSet(BaseDistributionViewSet, mixins.ListModelMixin):
@@ -567,9 +585,9 @@ class DistributionViewSet(
567585
LabelsMixin,
568586
):
569587
"""
570-
Provides read and list methods and also provides asynchronous CUD methods to dispatch tasks
571-
with reservation that lock all Distributions preventing race conditions during base_path
572-
checking.
588+
Provides read and list methods plus asynchronous CUD methods that reserve the narrowest safe
589+
distribution locks, only taking the domain-wide lock when base_path overlap validation or
590+
base_path release needs it.
573591
"""
574592

575593

pulpcore/tests/functional/api/using_plugin/test_distributions.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,66 @@ def test_distribution_base_path(
167167
assert json.loads(exc.value.body)["base_path"] is not None
168168

169169

170+
@pytest.mark.parallel
171+
def test_distribution_update_task_reservations(
172+
file_bindings,
173+
monitor_task,
174+
):
175+
create_task = monitor_task(
176+
file_bindings.DistributionsFileApi.create(
177+
{"name": str(uuid4()), "base_path": str(uuid4())}
178+
).task
179+
)
180+
assert any(
181+
resource.endswith(":distributions") for resource in create_task.reserved_resources_record
182+
)
183+
distribution = file_bindings.DistributionsFileApi.read(create_task.created_resources[0])
184+
assert distribution.prn not in create_task.reserved_resources_record
185+
186+
no_base_path_update_task = monitor_task(
187+
file_bindings.DistributionsFileApi.partial_update(
188+
distribution.pulp_href,
189+
{"name": str(uuid4())},
190+
).task
191+
)
192+
assert distribution.prn in no_base_path_update_task.reserved_resources_record
193+
assert not any(
194+
resource.endswith(":distributions")
195+
for resource in no_base_path_update_task.reserved_resources_record
196+
)
197+
198+
unchanged_base_path_update_task = monitor_task(
199+
file_bindings.DistributionsFileApi.partial_update(
200+
distribution.pulp_href,
201+
{"name": str(uuid4()), "base_path": distribution.base_path},
202+
).task
203+
)
204+
assert distribution.prn in unchanged_base_path_update_task.reserved_resources_record
205+
assert not any(
206+
resource.endswith(":distributions")
207+
for resource in unchanged_base_path_update_task.reserved_resources_record
208+
)
209+
210+
base_path_update_task = monitor_task(
211+
file_bindings.DistributionsFileApi.partial_update(
212+
distribution.pulp_href, {"base_path": str(uuid4())}
213+
).task
214+
)
215+
assert distribution.prn in base_path_update_task.reserved_resources_record
216+
assert any(
217+
resource.endswith(":distributions")
218+
for resource in base_path_update_task.reserved_resources_record
219+
)
220+
221+
delete_task = monitor_task(
222+
file_bindings.DistributionsFileApi.delete(distribution.pulp_href).task
223+
)
224+
assert distribution.prn in delete_task.reserved_resources_record
225+
assert any(
226+
resource.endswith(":distributions") for resource in delete_task.reserved_resources_record
227+
)
228+
229+
170230
@pytest.mark.parallel
171231
def test_distribution_filtering(
172232
file_bindings,

0 commit comments

Comments
 (0)