Narrow distribution task locks for unchanged base_path by pablomh · Pull Request #7896 · pulp/pulpcore

pablomh · 2026-07-23T13:17:44Z

Problem

Async distribution CUD operations currently reserve the domain-wide
pdrn:<domain>:distributions resource unconditionally. For ordinary
updates that leave base_path unchanged, this serializes unrelated
pulpcore.app.tasks.base.ageneral_update tasks behind a single lock.

This showed up as a bottleneck during capsule sync, where many
RefreshDistribution updates can run at once but end up waiting on the
same domain-wide reservation.

What this changes

This patch narrows distribution task reservations when the effective
base_path does not change:

create: keep the domain-wide distributions lock
delete: keep the domain-wide distributions lock
update with changed base_path: keep the broader lock
update with unchanged base_path: reserve only the distribution instance

The implementation also handles partial PATCH requests correctly by
falling back to instance.base_path when base_path is omitted from the
request body.

Test coverage

Adds a functional test covering the reservation behavior for:

create
partial update with no base_path in the payload
partial update with unchanged base_path
partial update with changed base_path
delete

Performance notes

Tested with Satellite 6.20 Stream,
~175 capsules, 15 Pulp workers).

For completed pulpcore.app.tasks.base.ageneral_update tasks, lock usage
shifted from all domain-wide reservations to a mix of domain and
instance-scoped reservations:

Lock type	Before patch	After patch
Domain-wide	110	26
Instance-only	0	42

Block wait times improved substantially:

Metric	Before (domain-locked)	After domain-locked	After instance-locked
P95	6.98s	75ms	28ms
P99	10.12s	105ms	29ms
Max	10.16s	121ms	29ms

These results are consistent with removing unnecessary serialization for
ordinary distribution updates while preserving the broader lock when the
base_path namespace can change.

gerrod3 · 2026-07-23T14:53:35Z

Can you add a changelog for this issue? #3322

pablomh · 2026-07-23T18:04:50Z

Done :)

gerrod3 · 2026-07-23T20:53:35Z

Not sure what is up with lint, maybe try rebasing with main. Also can you add fixes: #3322 to your commit message? Finally if you want this backported to older pulpcores mark the changelog as a bugfix.

pablomh · 2026-07-23T21:23:35Z

I'd like it to be backported to at least the release(s) that are part of Satellite 6.19.z.

…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>

mdellweg

I am a tiny bit concerned about Zero Downtime Upgrades here.
An old task (dispatched before this change) updating a distribution would only lock on the domain:distributions and a new one only on the single distribution.
Since distributions themselves are rather shallow, I cannot think about a situation where this collision leads to real world impacts. But my lack of imagination here is no guarantee for correctness.

mdellweg · 2026-07-24T08:53:34Z

+        """
+        Reserve the narrowest safe lock for async distribution operations.
+
+        Creates, deletes, and base_path changes still lock the domain-wide distributions resource


Maybe add a statement that "base_path overlap validation" is the main concern why this function even exists.

I'm wondering if we can safely assume that deleting a distribution will never violate base_path overlaps?

Yes, that is the main reason this logic exists, and I can make that clearer in the docstring.

I would still keep deletes on the broader lock. A delete does not create an overlap by itself, but it does release a base_path, so it is still part of the same domain-wide consistency concern as creates and moves.

mdellweg · 2026-07-24T09:01:08Z

+            return [instance, domain_distributions]
+
+        request_data = getattr(getattr(self, "request", None), "data", {})
+        requested_base_path = request_data.get("base_path", instance.base_path)


How come this is not None for a patch call that does not contain "base-path".

The test suggests, it actually works though (assuming it would actually pass once the linting works again).

For a PATCH that does not include base_path, we treat that as base_path staying unchanged, so it stays on the instance-only lock.

The test is meant to cover exactly that case. If it helps, I can rewrite that branch to make the “field omitted means unchanged” behavior more explicit.

Oh you feed the current value as default. Never mind. Ignore me on this.

pablomh · 2026-07-24T13:48:41Z

I am a tiny bit concerned about Zero Downtime Upgrades here. An old task (dispatched before this change) updating a distribution would only lock on the domain:distributions and a new one only on the single distribution. Since distributions themselves are rather shallow, I cannot think about a situation where this collision leads to real world impacts. But my lack of imagination here is no guarantee for correctness.

Thanks, I think this is the main caveat in the current approach.

From our look at the code, we see two ways to address the ZDU concern:

Add shared-resource support to the async mixins, so unchanged-base_path distribution updates can keep the narrower instance lock while still overlapping with older broad-lock tasks during rollout.
Keep the change local to distributions by overriding the update path there and adding the shared reservation only for that case.

Both approaches would keep the current concurrency improvement while restoring compatibility with older broad-lock tasks during mixed-version upgrades.

dralley · 2026-07-24T15:11:09Z

and a new one only on the single distribution.

But only if it doesn't actually change the base path, right? Tasks that do touch base_path would retain the domain:distributions lock. That seems OK.

github-actions Bot added no-changelog no-issue labels Jul 23, 2026

pablomh force-pushed the distribution-lock-narrowing-pr branch from c74e46f to 6faf576 Compare July 23, 2026 18:04

github-actions Bot removed the no-changelog label Jul 23, 2026

pablomh force-pushed the distribution-lock-narrowing-pr branch from 6faf576 to cb2a1be Compare July 23, 2026 21:22

pablomh force-pushed the distribution-lock-narrowing-pr branch 2 times, most recently from a15c525 to 951ea58 Compare July 23, 2026 21:32

pablomh force-pushed the distribution-lock-narrowing-pr branch from 951ea58 to b4d28e3 Compare July 23, 2026 21:33

dralley added backport-3.85 backport-3.105 backport-3.115 labels Jul 24, 2026

mdellweg reviewed Jul 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Narrow distribution task locks for unchanged base_path#7896

Narrow distribution task locks for unchanged base_path#7896
pablomh wants to merge 1 commit into
pulp:mainfrom
pablomh:distribution-lock-narrowing-pr

pablomh commented Jul 23, 2026 •

edited

Loading

Uh oh!

gerrod3 commented Jul 23, 2026

Uh oh!

pablomh commented Jul 23, 2026

Uh oh!

gerrod3 commented Jul 23, 2026

Uh oh!

pablomh commented Jul 23, 2026 •

edited

Loading

Uh oh!

mdellweg left a comment

Uh oh!

mdellweg Jul 24, 2026

Uh oh!

pablomh Jul 24, 2026

Uh oh!

mdellweg Jul 24, 2026 •

edited

Loading

Uh oh!

pablomh Jul 24, 2026

Uh oh!

mdellweg Jul 24, 2026

Uh oh!

pablomh commented Jul 24, 2026

Uh oh!

dralley commented Jul 24, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

pablomh commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

What this changes

Test coverage

Performance notes

Uh oh!

gerrod3 commented Jul 23, 2026

Uh oh!

pablomh commented Jul 23, 2026

Uh oh!

gerrod3 commented Jul 23, 2026

Uh oh!

pablomh commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdellweg left a comment

Choose a reason for hiding this comment

Uh oh!

mdellweg Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

pablomh Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mdellweg Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pablomh Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mdellweg Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

pablomh commented Jul 24, 2026

Uh oh!

dralley commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pablomh commented Jul 23, 2026 •

edited

Loading

pablomh commented Jul 23, 2026 •

edited

Loading

mdellweg Jul 24, 2026 •

edited

Loading

dralley commented Jul 24, 2026 •

edited

Loading