Skip to content
Open
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
13 changes: 13 additions & 0 deletions .github/workflows/pythontest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ jobs:
- 6379:6379
steps:
- uses: actions/checkout@v6
with:
fetch-depth: 0
- name: Set up minio
run: |
docker run -d -p 9000:9000 --name minio \
Expand All @@ -79,6 +81,17 @@ jobs:
run: |
# Use uv to install dependencies directly from requirements files
uv pip sync requirements.txt requirements-dev.txt
- name: Lint new migrations for unsafe operations
if: github.event_name == 'pull_request'
env:
BASE_REF: ${{ github.base_ref }}
DJANGO_SETTINGS_MODULE: contentcuration.not_production_settings
run: |
set -euo pipefail
git fetch --no-tags origin "$BASE_REF"
base="$(git merge-base "origin/$BASE_REF" HEAD)"
test -n "$base"
python contentcuration/manage.py lintmigrations --git-commit-id "$base" --no-cache --warnings-as-errors
- name: Test pytest
run: |
sh -c './contentcuration/manage.py makemigrations --check'
Expand Down
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ migrate:
# 4) Remove the management command from this `deploy-migrate` recipe
# 5) Repeat!
deploy-migrate:
echo "Nothing to do here!"
# studio#5974: remove at cutover.
python contentcuration/manage.py backfill_column --model contentcuration.File --source-field file_size --target-field file_size_bigint

contentnodegc:
python contentcuration/manage.py garbage_collect
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from django_prometheus.db.backends.postgresql.base import (
DatabaseWrapper as PrometheusDatabaseWrapper,
)
from django_zero_downtime_migrations.backends.postgres.schema import (
DatabaseSchemaEditor,
)


class DatabaseWrapper(PrometheusDatabaseWrapper):
"""Prometheus query metrics + zero-downtime safe-DDL schema editor."""

SchemaEditorClass = DatabaseSchemaEditor
31 changes: 31 additions & 0 deletions contentcuration/contentcuration/db/dual_write.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import hashlib

import pgtrigger


def mirror_field(source, target):
"""Mirror Django field `source` into `target` via a BEFORE INSERT/UPDATE
trigger (expand/contract dual-write)."""

def decorator(model):
source_col = model._meta.get_field(source).column
target_col = model._meta.get_field(target).column
name = "mirror_{}_to_{}".format(source_col, target_col)
if len(name) > 43: # stay safely under pgtrigger's trigger-name limit
digest = hashlib.sha1(
"{}_{}".format(source_col, target_col).encode()
).hexdigest()[:8]
name = "mirror_{}".format(digest)
# Change-guard (IS DISTINCT FROM): keeps a read cutover from clobbering
# writes to the repointed column with the stale source value.
trigger = pgtrigger.Trigger(
name=name,
when=pgtrigger.Before,
operation=pgtrigger.Insert | pgtrigger.Update,
func="IF NEW.{s} IS DISTINCT FROM OLD.{s} THEN NEW.{t} = NEW.{s}; END IF; RETURN NEW;".format(
s=source_col, t=target_col
),
)
return pgtrigger.register(trigger)(model)

return decorator
100 changes: 100 additions & 0 deletions contentcuration/contentcuration/management/commands/backfill_column.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
from django.apps import apps
from django.core.exceptions import FieldDoesNotExist
from django.core.management.base import BaseCommand
from django.core.management.base import CommandError
from django.db import transaction
from django.db.models import F


class Command(BaseCommand):
help = (
"Idempotent, resumable online backfill of one column into another, in batches."
)

def add_arguments(self, parser):
parser.add_argument("--model", required=True, help="app_label.ModelName")
parser.add_argument("--source-field", required=True)
parser.add_argument("--target-field", required=True)
parser.add_argument("--batch-size", type=int, default=10000)
parser.add_argument("--start-id", default=None, help="resume from this pk")
parser.add_argument(
"--progress-check",
action="store_true",
help="report unbackfilled rows, exit nonzero if any",
)

def _resolve_model_fields(self, model_label, source, target):
try:
model = apps.get_model(model_label)
except (LookupError, ValueError) as e:
raise CommandError("Bad --model {!r}: {}".format(model_label, e))
try:
model._meta.get_field(source)
model._meta.get_field(target)
except FieldDoesNotExist as e:
raise CommandError(str(e))
return model

def _batch_end_pk(self, queryset, pk_name, start_pk, batch_size):
"""Last pk of the batch of `batch_size` rows starting at `start_pk`.

Returns None when fewer than `batch_size` rows remain at/after
`start_pk` — the final, short batch. Keyset paging by pk, so it works
for any pk type (int or UUID).
"""
return (
queryset.filter(pk__gte=start_pk)
.order_by(pk_name)
.values_list("pk", flat=True)[batch_size - 1 : batch_size]
.first()
)

def handle(self, *args, **options):
if options["batch_size"] < 1:
raise CommandError("--batch-size must be >= 1")
source = options["source_field"]
target = options["target_field"]
model = self._resolve_model_fields(options["model"], source, target)

pk_name = model._meta.pk.name
batch_size = options["batch_size"]
only_unfilled = {target + "__isnull": True, source + "__isnull": False}
unfilled = model.objects.filter(**only_unfilled)
unfilled_pks = unfilled.order_by(pk_name).values_list("pk", flat=True)

if options["progress_check"]:
# exists(), not count() — the target table can have millions of rows.
if unfilled.exists():
raise CommandError("backfill incomplete: rows still pending")
self.stdout.write("Backfill complete: no rows pending.")
return

# Start at the first unfilled pk (>= --start-id if given); re-runs and
# resumes skip straight past an already-filled prefix.
batch_start = unfilled_pks
if options["start_id"] is not None:
batch_start = batch_start.filter(pk__gte=options["start_id"])
batch_start = batch_start.first()

total = 0
while batch_start is not None:
batch_end = self._batch_end_pk(
model.objects, pk_name, batch_start, batch_size
)
if batch_end is None:
window = {"pk__gte": batch_start}
else:
window = {"pk__gte": batch_start, "pk__lte": batch_end}
with transaction.atomic():
total += model.objects.filter(**window, **only_unfilled).update(
**{target: F(source)}
)
self.stdout.write(
"backfilled through pk={} (updated {} so far)".format(
batch_start if batch_end is None else batch_end, total
)
)
if batch_end is None:
break
batch_start = unfilled_pks.filter(pk__gt=batch_end).first()
self.stdout.write("Done. {} rows updated.".format(total))
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Generated by Django 3.2.24 on 2026-06-23 05:56
import pgtrigger.compiler
import pgtrigger.migrations
from django.db import migrations
from django.db import models
from django.db.models import Q


class Migration(migrations.Migration):

dependencies = [
("contentcuration", "0166_add_usersubscription"),
]

operations = [
migrations.AddField(
model_name="file",
name="file_size_bigint",
field=models.BigIntegerField(blank=True, null=True),
),
migrations.AddIndex(
model_name="file",
index=models.Index(
fields=["checksum", "file_size_bigint"],
name="file_checksum_fsizebig_idx",
condition=Q(file_size_bigint__isnull=False),
),
),
pgtrigger.migrations.AddTrigger(
model_name="file",
trigger=pgtrigger.compiler.Trigger(
name="mirror_file_size_to_file_size_bigint",
sql=pgtrigger.compiler.UpsertTriggerSql(
func="IF NEW.file_size IS DISTINCT FROM OLD.file_size THEN NEW.file_size_bigint = NEW.file_size; END IF; RETURN NEW;",
hash="051e321c4cdf91ea81f96b9f9a29e3b5015def67",
operation="INSERT OR UPDATE",
pgid="pgtrigger_mirror_file_size_to_file_size_bigint_54326",
table="contentcuration_file",
when="BEFORE",
),
),
),
]
19 changes: 19 additions & 0 deletions contentcuration/contentcuration/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
from contentcuration.constants import feedback
from contentcuration.constants import user_history
from contentcuration.constants.contentnode import kind_activity_map
from contentcuration.db.dual_write import mirror_field
from contentcuration.db.models.expressions import Array
from contentcuration.db.models.functions import ArrayRemove
from contentcuration.db.models.functions import Unnest
Expand Down Expand Up @@ -3255,6 +3256,8 @@ class StagedFile(models.Model):


FILE_DISTINCT_INDEX_NAME = "file_checksum_file_size_idx"
# studio#5974: bigint shadow of FILE_DISTINCT_INDEX_NAME, for the file_size widening.
FILE_DISTINCT_BIGINT_INDEX_NAME = "file_checksum_fsizebig_idx"
FILE_MODIFIED_DESC_INDEX_NAME = "file_modified_desc_idx"
FILE_DURATION_CONSTRAINT = "file_media_duration_int"
MEDIA_PRESETS = [
Expand All @@ -3266,6 +3269,14 @@ class StagedFile(models.Model):
]


# studio#5974 swap (next release, after backfill completes). One migration:
# - drop the @mirror_field decorator and the file_size_bigint field below
# - file_size = models.BigIntegerField(blank=True, null=True)
# - DB ops: drop the trigger + int file_size column, then RENAME file_size_bigint -> file_size
# - wrap in SeparateDatabaseAndState so the int->bigint AlterField is state-only (no rewrite)
# Transparent to old pods: they keep writing file_size (now bigint); only a brief metadata lock.
# Do NOT add db_column to reach file_size_bigint first — that generation breaks at the rename.
@mirror_field("file_size", "file_size_bigint") # studio#5974: dual-write int->bigint
class File(models.Model):
"""
The bottom layer of the contentDB schema, defines the basic building brick for content.
Expand All @@ -3275,6 +3286,9 @@ class File(models.Model):
id = UUIDField(primary_key=True, default=uuid.uuid4)
checksum = models.CharField(max_length=400, blank=True, db_index=True)
file_size = models.IntegerField(blank=True, null=True)
file_size_bigint = models.BigIntegerField(
blank=True, null=True
) # studio#5974 shadow
file_on_disk = models.FileField(
upload_to=object_storage_name,
storage=default_storage,
Expand Down Expand Up @@ -3485,6 +3499,11 @@ class Meta:
models.Index(
fields=["checksum", "file_size"], name=FILE_DISTINCT_INDEX_NAME
),
models.Index(
fields=["checksum", "file_size_bigint"],
name=FILE_DISTINCT_BIGINT_INDEX_NAME,
condition=Q(file_size_bigint__isnull=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.

A constraint on file_size_bigint being not null may speed up this creation of this new index.

models.Index(fields=["-modified"], name=FILE_MODIFIED_DESC_INDEX_NAME),
]
constraints = [
Expand Down
9 changes: 9 additions & 0 deletions contentcuration/contentcuration/not_production_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,14 @@

AWS_AUTO_CREATE_BUCKET = True

INSTALLED_APPS += ("django_migration_linter",) # noqa F405

MIGRATION_LINTER_OPTIONS = {
"exclude_apps": [
"kolibri_content"
], # SQLite content-export app; not on the safe-DDL Postgres backend
"sql_analyser": "postgresql",
}

# Use local instance for curriculum automation for development
CURRICULUM_AUTOMATION_API_URL = "http://localhost:8000"
2 changes: 1 addition & 1 deletion contentcuration/contentcuration/production_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
if SITE_READ_ONLY:
CACHES["default"]["BACKEND"] = "django_prometheus.cache.backends.locmem.LocMemCache"

DATABASES["default"]["ENGINE"] = "django_prometheus.db.backends.postgresql"
DATABASES["default"]["ENGINE"] = "contentcuration.db.backends.zero_downtime_prometheus"


REST_FRAMEWORK["DEFAULT_RENDERER_CLASSES"] = [
Expand Down
10 changes: 9 additions & 1 deletion contentcuration/contentcuration/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
"django_celery_results",
"kolibri_public",
"automation",
"pgtrigger",
)

SESSION_ENGINE = "django.contrib.sessions.backends.cached_db"
Expand Down Expand Up @@ -193,7 +194,7 @@

DATABASES = {
"default": {
"ENGINE": "django.db.backends.postgresql_psycopg2",
"ENGINE": "django_zero_downtime_migrations.backends.postgres",
"NAME": os.getenv("DATA_DB_NAME") or "kolibri-studio",
# For dev purposes only
"USER": os.getenv("DATA_DB_USER") or "learningequality",
Expand All @@ -204,6 +205,13 @@
},
}

ZERO_DOWNTIME_MIGRATIONS_LOCK_TIMEOUT = "30s"
ZERO_DOWNTIME_MIGRATIONS_STATEMENT_TIMEOUT = "60s"
# don't kill long-but-safe ops (e.g. CREATE INDEX CONCURRENTLY)
ZERO_DOWNTIME_MIGRATIONS_FLEXIBLE_STATEMENT_TIMEOUT = True
# surface unsafe DDL at runtime
ZERO_DOWNTIME_MIGRATIONS_RAISE_FOR_UNSAFE = True

IS_CONTENTNODE_TABLE_PARTITIONED = (
os.getenv("IS_CONTENTNODE_TABLE_PARTITIONED") or False
)
Expand Down
3 changes: 3 additions & 0 deletions contentcuration/contentcuration/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

DEBUG = True

# Test DB build replays legacy pre-safe-DDL migrations; runtime and the CI linter still enforce.
ZERO_DOWNTIME_MIGRATIONS_RAISE_FOR_UNSAFE = False

WEBPACK_LOADER["DEFAULT"][ # noqa
"LOADER_CLASS"
] = "contentcuration.tests.webpack_loader.TestWebpackLoader"
Expand Down
Loading
Loading