Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/+content-ids-migration.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Filled missing repository version `content_ids` in SQL from content membership, committing per repository so large databases are less likely to hit statement timeouts.
68 changes: 49 additions & 19 deletions pulpcore/app/migrations/0152_alter_repositoryversion_content_ids.py
Original file line number Diff line number Diff line change
@@ -1,35 +1,65 @@
# Generated by Django 5.2.13 on 2026-06-08 21:54

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.


# Same membership as RepositoryVersion._content_relationships().
_POPULATE_SQL = """
UPDATE __RV__ AS rv
SET content_ids = COALESCE((
SELECT ARRAY_AGG(rc.content_id)
FROM __RC__ AS rc
INNER JOIN __RV__ AS va ON va.pulp_id = rc.version_added_id
LEFT JOIN __RV__ AS vr ON vr.pulp_id = rc.version_removed_id
WHERE rc.repository_id = rv.repository_id
AND va.number <= rv.number
AND (vr.pulp_id IS NULL OR vr.number > rv.number)
), '{}'::uuid[])
WHERE rv.repository_id = %s
AND rv.content_ids IS NULL
"""


def populate_content_ids(apps, schema_editor):
RepositoryVersion = apps.get_model('core', 'RepositoryVersion')
RepositoryContent = apps.get_model('core', 'RepositoryContent')
repo_versions = []
for rv in RepositoryVersion.objects.filter(content_ids=None).iterator(chunk_size=2000):
rv.content_ids = list(RepositoryContent.objects.filter(
repository_id=rv.repository_id, version_added__number__lte=rv.number
).exclude(version_removed__number__lte=rv.number).values_list("content_id", flat=True))
repo_versions.append(rv)
if len(repo_versions) >= 2000:
RepositoryVersion.objects.bulk_update(repo_versions, ['content_ids'])
repo_versions = []
if repo_versions:
RepositoryVersion.objects.bulk_update(repo_versions, ['content_ids'])
"""Fill missing content_ids from content membership, committing per repository."""
RepositoryVersion = apps.get_model("core", "RepositoryVersion")
RepositoryContent = apps.get_model("core", "RepositoryContent")
connection = schema_editor.connection
qn = schema_editor.quote_name
rv = qn(RepositoryVersion._meta.db_table)
rc = qn(RepositoryContent._meta.db_table)
sql = _POPULATE_SQL.replace("__RV__", rv).replace("__RC__", rc)

with connection.cursor() as cursor:
cursor.execute(f"SELECT DISTINCT repository_id FROM {rv} WHERE content_ids IS NULL")
repo_ids = [row[0] for row in cursor.fetchall()]

for repo_id in repo_ids:
with transaction.atomic(using=connection.alias):
with connection.cursor() as cursor:
cursor.execute(sql, [repo_id])


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.


dependencies = [
('core', '0151_upstreampulp_connect_timeout_and_more'),
("core", "0151_upstreampulp_connect_timeout_and_more"),
]

operations = [
migrations.RunPython(populate_content_ids, reverse_code=migrations.RunPython.noop, elidable=True),
migrations.RunPython(
populate_content_ids,
reverse_code=migrations.RunPython.noop,
elidable=True,
atomic=False,
),
migrations.AlterField(
model_name='repositoryversion',
name='content_ids',
field=django.contrib.postgres.fields.ArrayField(base_field=models.UUIDField(), default=list, size=None),
model_name="repositoryversion",
name="content_ids",
field=django.contrib.postgres.fields.ArrayField(
base_field=models.UUIDField(), default=list, size=None
),
),
]
77 changes: 77 additions & 0 deletions pulpcore/tests/unit/models/test_content_ids_migration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import importlib
from types import SimpleNamespace
from uuid import uuid4

import pytest
from django.apps import apps
from django.db import connection

from pulpcore.app.models import RepositoryVersion
from pulpcore.plugin.models import Content, Repository

populate_content_ids = importlib.import_module(
"pulpcore.app.migrations.0152_alter_repositoryversion_content_ids"
).populate_content_ids


def _run_populate():
schema_editor = SimpleNamespace(
connection=connection,
quote_name=connection.ops.quote_name,
)
populate_content_ids(apps, schema_editor)


def _membership_ids(version):
return set(version._content_relationships().values_list("content_id", flat=True))


@pytest.fixture
def repository(db):
repository = Repository.objects.create(name=uuid4())
repository.CONTENT_TYPES = [Content]
return repository


def test_populate_content_ids_from_content_relationships(repository):
contents = [Content(pulp_type="core.content") for _ in range(4)]
Content.objects.bulk_create(contents)
pks = [c.pk for c in contents]

version0 = repository.latest_version()
with repository.new_version() as version1:
version1.add_content(Content.objects.filter(pk__in=pks[:3]))
with repository.new_version() as version2:
version2.remove_content(Content.objects.filter(pk__in=pks[:1]))
with repository.new_version() as version3:
version3.add_content(Content.objects.filter(pk__in=pks[3:]))

# A second repository whose cache is already populated must be left alone.
other = Repository.objects.create(name=uuid4())
other.CONTENT_TYPES = [Content]
with other.new_version() as other_v1:
other_v1.add_content(Content.objects.filter(pk__in=pks[:2]))
other_v1.refresh_from_db()
other_ids_before = list(other_v1.content_ids)

rv_table = connection.ops.quote_name(RepositoryVersion._meta.db_table)
with connection.cursor() as cursor:
# Flush deferred triggers from the version inserts so ALTER TABLE can run
# inside the test transaction.
cursor.execute("SET CONSTRAINTS ALL IMMEDIATE")
cursor.execute(f"ALTER TABLE {rv_table} ALTER COLUMN content_ids DROP NOT NULL")
# Leave version3 populated (the post-3.83 case) and null the older versions.
cursor.execute(
f"UPDATE {rv_table} SET content_ids = NULL WHERE repository_id = %s AND number < %s",
[repository.pk, version3.number],
)

_run_populate()

for version in (version0, version1, version2, version3):
version.refresh_from_db()
assert version.content_ids is not None
assert set(version.content_ids) == _membership_ids(version)

other_v1.refresh_from_db()
assert list(other_v1.content_ids) == other_ids_before
Loading