From b4d28e30c6356bbc96505ab4a71dc5fa5497ac8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20M=C3=A9ndez=20Hern=C3=A1ndez?= Date: Wed, 22 Jul 2026 21:27:22 +0200 Subject: [PATCH] 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::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 --- CHANGES/7896.bugfix | 1 + pulpcore/app/viewsets/publication.py | 28 +++++++-- .../api/using_plugin/test_distributions.py | 60 +++++++++++++++++++ 3 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 CHANGES/7896.bugfix diff --git a/CHANGES/7896.bugfix b/CHANGES/7896.bugfix new file mode 100644 index 00000000000..a6f1876cf2e --- /dev/null +++ b/CHANGES/7896.bugfix @@ -0,0 +1 @@ +Reduced lock contention for distribution updates that leave `base_path` unchanged. diff --git a/pulpcore/app/viewsets/publication.py b/pulpcore/app/viewsets/publication.py index ae7e38a8e6d..e6031607abc 100644 --- a/pulpcore/app/viewsets/publication.py +++ b/pulpcore/app/viewsets/publication.py @@ -527,8 +527,26 @@ def get_queryset(self): return qs def async_reserved_resources(self, instance): - """Return resource that locks all Distributions.""" - return [f"pdrn:{get_domain().pulp_id}:distributions"] + """ + Reserve the narrowest safe lock for async distribution operations. + + Creates, deletes, and base_path changes still lock the domain-wide distributions resource + because base_path overlap validation is domain scoped. Other updates only need to lock the + specific distribution instance. + """ + domain_distributions = f"pdrn:{get_domain().pulp_id}:distributions" + if instance is None: + return [domain_distributions] + + if getattr(self, "action", "") == "destroy": + return [instance, domain_distributions] + + request_data = getattr(getattr(self, "request", None), "data", {}) + requested_base_path = request_data.get("base_path", instance.base_path) + if requested_base_path == instance.base_path: + return [instance] + + return [instance, domain_distributions] class ListDistributionViewSet(BaseDistributionViewSet, mixins.ListModelMixin): @@ -567,9 +585,9 @@ class DistributionViewSet( LabelsMixin, ): """ - Provides read and list methods and also provides asynchronous CUD methods to dispatch tasks - with reservation that lock all Distributions preventing race conditions during base_path - checking. + Provides read and list methods plus asynchronous CUD methods that reserve the narrowest safe + distribution locks, only taking the domain-wide lock when base_path overlap validation or + base_path release needs it. """ diff --git a/pulpcore/tests/functional/api/using_plugin/test_distributions.py b/pulpcore/tests/functional/api/using_plugin/test_distributions.py index 1870c86e462..9e724638c8c 100644 --- a/pulpcore/tests/functional/api/using_plugin/test_distributions.py +++ b/pulpcore/tests/functional/api/using_plugin/test_distributions.py @@ -167,6 +167,66 @@ def test_distribution_base_path( assert json.loads(exc.value.body)["base_path"] is not None +@pytest.mark.parallel +def test_distribution_update_task_reservations( + file_bindings, + monitor_task, +): + create_task = monitor_task( + file_bindings.DistributionsFileApi.create( + {"name": str(uuid4()), "base_path": str(uuid4())} + ).task + ) + assert any( + resource.endswith(":distributions") for resource in create_task.reserved_resources_record + ) + distribution = file_bindings.DistributionsFileApi.read(create_task.created_resources[0]) + assert distribution.prn not in create_task.reserved_resources_record + + no_base_path_update_task = monitor_task( + file_bindings.DistributionsFileApi.partial_update( + distribution.pulp_href, + {"name": str(uuid4())}, + ).task + ) + assert distribution.prn in no_base_path_update_task.reserved_resources_record + assert not any( + resource.endswith(":distributions") + for resource in no_base_path_update_task.reserved_resources_record + ) + + unchanged_base_path_update_task = monitor_task( + file_bindings.DistributionsFileApi.partial_update( + distribution.pulp_href, + {"name": str(uuid4()), "base_path": distribution.base_path}, + ).task + ) + assert distribution.prn in unchanged_base_path_update_task.reserved_resources_record + assert not any( + resource.endswith(":distributions") + for resource in unchanged_base_path_update_task.reserved_resources_record + ) + + base_path_update_task = monitor_task( + file_bindings.DistributionsFileApi.partial_update( + distribution.pulp_href, {"base_path": str(uuid4())} + ).task + ) + assert distribution.prn in base_path_update_task.reserved_resources_record + assert any( + resource.endswith(":distributions") + for resource in base_path_update_task.reserved_resources_record + ) + + delete_task = monitor_task( + file_bindings.DistributionsFileApi.delete(distribution.pulp_href).task + ) + assert distribution.prn in delete_task.reserved_resources_record + assert any( + resource.endswith(":distributions") for resource in delete_task.reserved_resources_record + ) + + @pytest.mark.parallel def test_distribution_filtering( file_bindings,