initial commit for multidb by YasenT · Pull Request #7862 · pulp/pulpcore

YasenT · 2026-07-15T09:43:40Z

Initial PR for the multidb implementation. To test github actions. etc

YasenT · 2026-07-15T12:54:03Z

/retest

YasenT · 2026-07-21T07:35:06Z

@gerrod3 What do you think? Can I get a review?

gerrod3

Round 1 of reviews. This is honestly quite unreviewable in its current state. The AI comments are a nightmare and make references to docs and comments I don't have access to. The commits are not logically structured either. I would probably have ordered them something like:

The initial adding of the database-alias and db-router
Fixing management commands and other models (GenericReleation)
Adding the database domain migration command
CI work and tests

I would like to set expectations now that this will require major changes and many iterations before we are close to a state that might be mergeable.

gerrod3 · 2026-07-31T17:27:34Z

+CROSS_PLANE_RECONCILIATION_GRACE_MINUTES = 60
+
+# KI-11: how long, in days, a confirmed-orphaned cross-plane row is kept (logged/alerted on every
+# sweep) before the reconciliation sweep deletes it outright. 0 disables purging entirely --
+# orphans are only ever logged, never deleted, which is the safe default.
+CROSS_PLANE_RECONCILIATION_PURGE_AFTER_DAYS = 0


Why do we need these two different settings?

The first one makes sure we don't "flag" objects in "flight" as orphans. For example in the case of a migration of an active domain to a new DB, we need some grace period before we start flagging as orphans

And than how regularly we purge is a different setting, the 2x can be very different values. Multiple days vs multiple hours. Makes sense to be 2 phased in a way.

gerrod3 · 2026-07-31T18:47:46Z

+_DOMAIN_WALK_MAX_DEPTH = 2
+
+
+def _resolve_domain_id(value, _depth=0, _seen=None):


Do we really need this? Yeah it's probably the safest way to get the domain of the object, but I would expect that get_domain would always return the correct domain that the object is in. Maybe it doesn't matter since creating GenericRelationships typically never happen in a hot path

I see it more as a protection for the future, just playing safe.

dralley · 2026-08-05T12:55:42Z

Rebase this please

YasenT · 2026-08-10T10:54:11Z

Rebase this please

rebased. tests re-running, had to fix few things, seems okay for now

…ting Introduce the core routing infrastructure needed to split a Pulp instance's domains across multiple database aliases: a `database_alias` (and `moving`) field on `Domain`, a `PulpDomainRouter` that pins control-plane models to `default` and routes data-plane models to their owning domain's alias (via instance hints or the domain ContextVar), and a `CrossDBQuerySetMixin` to safely materialize subqueries that would otherwise span two databases. Also add the supporting ContextVar/util helpers (`with_migration_alias`, `domain_db`, `for_each_domain`, and a `get_domain_pk()` alias fix) used by later commits, and the app-startup guards needed for `migrate` and its existing post_migrate hooks to behave correctly once more than one database alias is configured. Co-authored-by: Cursor <cursoragent@cursor.com>

…ting Update existing management commands (datarepair, datarepair-2327, remove-plugin, repository-size, rotate-db-key, dump-publications-to-fs, handle-artifact-checksums, analyze-publication) to iterate per-domain and query the correct database alias instead of assuming a single database. Fix GenericForeignKey/GenericRelation handling across database boundaries: resolve the transitive domain for content_object targets that don't carry their own pulp_domain field (e.g. RepositoryVersion), add a content_object_domain column to CreatedResource/ExportedResource/ UserRole/GroupRole to record which alias the referenced object actually lives on, and clean up UserRole/GroupRole rows when their cross-plane target is deleted (Django's native GenericRelation cascade can't follow a relation across two different databases). Also fix ProfileArtifact's cross-plane artifact lookup in the task profiling viewset, and materialize a couple of querysets in role_util.py that would otherwise become invalid cross-database subqueries. Co-authored-by: Cursor <cursoragent@cursor.com>

Add pulpcore-manager migrate-all/migrate for orchestrating Django migrations across every configured database alias in the right order. Add move-domain/cleanup-moved-domain for relocating a domain's data-plane rows from one alias to another: begin with a read-only cutover, copy and verify the data, then complete the cutover and delete the stale copy left behind on the source alias. Add sync-domains/domain-size for keeping Domain metadata replicated to satellite aliases and reporting on a domain's footprint, and reconcile-cross-plane-references for sweeping up orphaned cross-plane references left behind by these operations. This is supported by MigrationStatus/DomainMove bookkeeping models, Domain row replication to satellites (domain_sync.py), advisory locks so only one orchestrator/mover runs at a time, per-alias connectivity and migration-completeness reporting on the /status/ endpoint, worker gating so tasks are deferred while a domain's data is mid-move or its alias is unreachable, and a degraded API response for the same cases. Co-authored-by: Cursor <cursoragent@cursor.com>

Add a "multi_db" CI matrix leg that spins up a second Postgres service container and runs the unit test suite against it with PulpDomainRouter registered. Add unit tests covering the router itself, Domain replication/reconciliation, domain move/cleanup, cross-plane reference reconciliation, and the DomainMiddleware/task changes from the preceding commits. Also fix flakiness in test_cancel_task_group by retrying task-group cancellation on a transient 409 instead of failing outright. Co-authored-by: Cursor <cursoragent@cursor.com>

Upstream added 0155-0158 for relative-path and default content guard work; shift the multi-db migration chain to 0159-0162 and update dependencies accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>

gerrod3

After more review I am not convinced that this is a good feature to add to Pulp. There are too many unknowns with how this could break and will potentially saddle us with a larger burden of maintenance then we are capable to provide. I would strongly suggest taking another look at running multiple Pulp instances to solve your problem.

If that is still not possible, here are some changes I would like to see.

gerrod3 · 2026-09-09T01:51:42Z

+
+        instance = hints.get("instance")
+        if instance is not None:
+            if "pulp_domain_id" in instance.__dict__:


We should probably add a comment for why we are looking at __dict__ instead of directly accessing it.

gerrod3 · 2026-09-10T17:54:45Z

+def is_multi_db_routing_active():
+    return any(isinstance(r, PulpDomainRouter) for r in django_router.routers)


I feel we should cache this calculation.

I think it's already a very cheap calculation, with djanngo_router.routers already being a cached property. An any/isinstance over something that should already be a very short list is a sub-microsecond as is.
Happy to add caching if you really feel it will help

gerrod3 · 2026-09-10T18:05:55Z

+        from pulpcore.app.db_router import is_multi_db_routing_active
+
+        if not is_multi_db_routing_active():
+            return super().filter(*args, **kwargs)


Instead of checking this every call, we should just swap these methods in the init if the router is selected.

Wouldn't we call it more often if we move it to the init?

gerrod3 · 2026-09-10T18:15:50Z

 def _ensure_default_domain(sender, **kwargs):
+    if kwargs.get("using", "default") != "default":
+        return
    table_names = connection.introspection.table_names()


This is wrong, with multi-db we need to ensure that every domain on a separate DB has that exact domain object in their DB. We might also need to ensure that each DB also has a copy of the default domain.

I believe that what you are looking for is handled already by _ensure_domains_replicated + reconcile_domains_to_alias()

gerrod3 · 2026-09-10T18:15:59Z

    return _current_domain.get() or get_default_domain()


 def get_domain_pk():


The handling here is incorrect. Let's take an example of a Repository:

The Repository model has a ForeignKey (pulp_domain) to Domain, with a default value set to this function. This ensures when a Repository is created it gets its pulp-domain set to the Domain that call was initiated from. With multi-db this would currently fail since the current_domain would live in the default database and wouldn't exist in that Domain's database, creating an invalid lookup.

Why wouldn't the domain exist in the domains database?
A domain has to be already migrated for us to start sending queries to the new db.
And domain_sync handles the sync part.

gerrod3 · 2026-09-10T18:38:35Z

-            with transaction.atomic():
-                content_guard, _created = ContentRedirectContentGuard.objects.get_or_create(
+            with transaction.atomic(using=alias):
+                content_guard, _created = ContentRedirectContentGuard.objects.using(


This would fail without a default domain in the other DB.

I think it's already covered.
_ensure_domains_replicated runs right before this hook in ready()
reconcile_domains_to_alias() always includes default in its desired set regardless of which alias it's reconciling
So by the time this runs, defaults row is already there?

gerrod3 · 2026-09-10T18:39:35Z

+        if is_multi_db_routing_active():
+            from pulpcore.app.role_util import on_any_model_post_delete
+
+            post_delete.connect(
+                on_any_model_post_delete, dispatch_uid="cleanup_cross_plane_roles_post_delete"


Is this needed? Are UserRoles even cleanedup currently?

Django`s deletion collector will treat a GenericRelation like any other reverse relation for cascade purposes and delete the UserRoles when you delete a Repository.
And the current django collector will bypass our domain route, therefore failing silently and leaving orphaned rows behind.

gerrod3 · 2026-09-10T19:02:05Z

I think we shouldn't allow cross DB generic relationships in this initial approach. Instead we should do:

Make Task's CreatedResource.save() a no-op when multi-db is enabled

Don't allow ObjectRolePermissionBackend or DefaultAccessPolicy to be set when multi-db is enabled so that UserRole & GroupRole objects are not used/created

CreatedResource really isn't needed anymore now that we have Task.output, so this isn't that big of a lost. For the RBAC stuff, if services need to use the role based per object we can think of removing User & Group from the control pane and just duplicating the needed users/groups across the DBs. They don't change as often as UserRole/GroupRoles do so I believe there would be less potential for errors this way. But I think that should be done in a future change if necessary.

Hm, Task.output? I am missing something I guess
Ripping all of this away, when we already have the code?

gerrod3 · 2026-09-10T19:09:51Z

I'm going to comment on the whole of this commit adding the migrate commands here:

I believe this whole piece should be moved to a separate PR in the future. We should test out the feature on brand new domains and see what works and what is broken before trying to migrate everything over. I've done some AI review on this PR and most of the bugs, the really bad ones, are from this part. The migration logic is hard, especially for all that you want it to do, so it needs a lot more design and testing that will delay this PR even further if it were to stay.

On the other hand, did you know you can do a pulp-export/import with domains? Have you thought of just using that for the migration?

Our issues, and the reason behind this PR, is the real need to move some big domains to seperate databases.
The rest of the PR without us being able to quickly move stuff, doesn't help us, because the users will keep seeing performance issues.
So I suggest that we keep moving with the review.

github-actions Bot added no-changelog no-issue multi-commit labels Jul 15, 2026

YasenT marked this pull request as ready for review July 24, 2026 10:30

gerrod3 reviewed Aug 3, 2026

View reviewed changes

YasenT force-pushed the multidb-implementation branch from ed36dcc to 2ad60ec Compare August 10, 2026 10:20

YasenT force-pushed the multidb-implementation branch from 35804e0 to 6c71cca Compare August 13, 2026 19:08

YasenT requested a review from gerrod3 August 19, 2026 09:49

YasenT force-pushed the multidb-implementation branch 2 times, most recently from dd289cb to 124b368 Compare September 1, 2026 17:27

YasenT and others added 5 commits September 4, 2026 09:27

Renumber multi-db migrations after rebase onto main.

1c14902

Upstream added 0155-0158 for relative-path and default content guard work; shift the multi-db migration chain to 0159-0162 and update dependencies accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>

YasenT force-pushed the multidb-implementation branch from 124b368 to 1c14902 Compare September 4, 2026 07:28

gerrod3 requested changes Sep 10, 2026

View reviewed changes

PR review - comment + remove multi db ci

e9c1d26

		_DOMAIN_WALK_MAX_DEPTH = 2


		def _resolve_domain_id(value, _depth=0, _seen=None):

		def is_multi_db_routing_active():
		return any(isinstance(r, PulpDomainRouter) for r in django_router.routers)

		return _current_domain.get() or get_default_domain()


		def get_domain_pk():

Uh oh!

Conversation

YasenT commented Jul 15, 2026

Uh oh!

YasenT commented Jul 15, 2026

Uh oh!

YasenT commented Jul 21, 2026

Uh oh!

gerrod3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dralley commented Aug 5, 2026

Uh oh!

YasenT commented Aug 10, 2026

Uh oh!

gerrod3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants