Skip to content

Make content_ids migration incremental and restartable - #8021

Merged
gerrod3 merged 1 commit into
pulp:mainfrom
gerrod3:fix/0152-content-ids-migration
Aug 31, 2026
Merged

Make content_ids migration incremental and restartable#8021
gerrod3 merged 1 commit into
pulp:mainfrom
gerrod3:fix/0152-content-ids-migration

Conversation

@gerrod3

@gerrod3 gerrod3 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Rewrite the 0152 content_ids data fill to compute each repository version in Postgres from the previous version (plus added, minus removed) instead of an N+1 ORM loop that ships UUID arrays through Python.
  • Commit per repository (atomic = False) so a DBaaS statement/session timeout does not roll back already-filled repos. Re-running migrate continues from remaining NULLs, then AlterField makes the column required.
  • Alternative to fix(0152_alter_repositoryversion_content_ids): add a batch size to avoid timeout on large update #8017: shrinking bulk_update batches does not fix the per-version recompute or the single wrapping transaction.

Test plan

  • Unit test covering add/remove across versions, already-filled post-3.83 versions as the incremental base, and a second repo left untouched
  • CI unit tests on this PR
  • Confirm an upgrade that previously died on 0152 can re-run migrate without losing progress

Made with Cursor

@gerrod3
gerrod3 marked this pull request as draft August 27, 2026 14:46

class Migration(migrations.Migration):
# Per-repository commits inside RunPython; AlterField runs afterwards.
atomic = False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This opens up a window, right?
But as long as nothing runs at the same time, or (in the case of ZDU) the currently running code is sufficiantly new there is nothing to introduce new repository versions with NULL. I think this is actually fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, if the user is doing a big jump and trying to do ZDU they might run into a problem if an old worker gets a task to create a new repo-vesion. Theoretically if that were to happen and then the ALTER TABLE call fails, they could just rerun the migration again and we would only need to update that one new repo-version that slipped through.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"big jump and ZDU" is not supported. Let's please not use vague language around that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, then there should be no window if they upgrade properly.


import django.contrib.postgres.fields
from django.db import migrations, models
from django.db import migrations, models, transaction

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about something along the lines of
UPDATE core.repositoryversion AS rv SET content_ids=(SELECT ARRAY_AGG(content_id) FROM core.repositoryversion_content WHERE version_added <= rv.version and version_removed > rv.version) WHERE rv.content_ids is NULL;

Maybe needs a few more joins and some care for version_removed = NULL.

Am oversimplifying the challenge here?

Maybe one can even batch that by adding a LIMIT 1000 clause.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the query to be the exact SQL version of _content_relationships. Calculating the memberships can take a little while so not sure if batching them would be good if we want to avoid timeouts.

Perform the cache populating entirely in SQL and add per repository commits.

Assisted By: Cursor Grok 4.6

Co-authored-by: Cursor <cursoragent@cursor.com>
@gerrod3
gerrod3 force-pushed the fix/0152-content-ids-migration branch from 87e316f to ceba57e Compare August 31, 2026 15:54
@gerrod3
gerrod3 marked this pull request as ready for review August 31, 2026 17:14
@gerrod3
gerrod3 merged commit edef435 into pulp:main Aug 31, 2026
13 of 14 checks passed
@patchback

patchback Bot commented Aug 31, 2026

Copy link
Copy Markdown

Backport to 3.116: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.116/edef435849726ca02cda937da16bdd7a0abc1ea4/pr-8021

Backported as #8031

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants