Narrow distribution task locks for unchanged base_path

[pablomh]

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]

Can you add a changelog for this issue? #3322

[pablomh]

Done :)

[gerrod3]

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]

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

[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]

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?

[pablomh]

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.

[pablomh]

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:

1. 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.
2. 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]

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.

[pablomh]

If I’m understanding correctly, the mixed-version caveat would only apply to updates whose effective base_path does not change.

Creates, deletes, and updates that do change base_path would still retain the broader domain:distributions lock, so the overlap-validation-sensitive cases would continue to serialize as before.

So the remaining ZDU concern would be narrower: it would apply only to ordinary unchanged-base_path distribution updates during a mixed-version rollout window.

If that matches your reading as well, would you prefer to keep the current scoped change, or would you want the mixed-version case addressed in this PR too?

[mdellweg]

I am certain that destroying a distribution can only release a base_path and so cannot even conflict with a prefixed-overlapped base_path that is created at the same time around. (Either the create fails, or the delete succeeded first. In any case the result is consistent wrt the overlap constraint.)

As for the ZDU concerns, I think if we aquire the existing domain-scoped lock shared, and introduce a new (uhhh, regrets...) domain-scoped-base-path lock to use alongside the specific entity lock, we'd be completely safe.
But still my contention is when the base_path is really the only thing that ever desparately needs lock protection, we could go with the current state. (I would then question whether we needed to add the entity-scoped lock at all.)