Skip to content

Complete the File.file_size bigint cutover - #6162

Merged
rtibbles merged 3 commits into
learningequality:hotfixesfrom
rtibbles:file_size_bigint_swap
Sep 29, 2026
Merged

rtibbles merged 3 commits into
learningequality:hotfixesfrom
rtibbles:file_size_bigint_swap

Conversation

@rtibbles

@rtibbles rtibbles commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Finalizes the bigint migration by dropping the int file size field and replacing it with the bigint shadow.
  • Reuses the existing bigint file size index (which was partial over not null) and instead updates all the file calculation queries to filter by not null to retain index usage.
  • Updates documentation for zero downtime migrations as it was too File.file_size specific.

References

Fixes #5974. Expand stage: #5986.

Reviewer guidance

  1. On a copy of a database already at 0167 with data in it, run manage.py migrate contentcuration. file_size comes out bigint, row count and SUM/MAX match the pre-swap file_size_bigint, and the one remaining index is file_checksum_file_size_idx ... WHERE file_size IS NOT NULL.
  2. manage.py backfill_column --model contentcuration.File --source-field file_size --target-field file_size_bigint --progress-check against the target database. The swap drops the int column, so this has to report complete before the release goes out. It does for both master and hotfixes.
  3. manage.py lintmigrations --project-root-path . --git-commit-id upstream/hotfixes --no-cache --warnings-as-errors. 0169 reports IGNORE; remove its ignore_name entry and the same run reports the drop and rename as errors.

AI usage

Used Claude Code to write the swap migration, the query changes and the runbook rewrite. Verified with the Python test suite, lintmigrations, and applying the migration to a populated database that had already been backfilled.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UQvLTghKxeZYiS9xBpyS8r

@rtibbles
rtibbles marked this pull request as ready for review September 22, 2026 21:38
@rtibbles
rtibbles requested a review from a team September 22, 2026 21:38
@bjester bjester self-assigned this Sep 23, 2026

@bjester bjester left a comment

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 couple of requests regarding the isnull filters

Comment thread contentcuration/contentcuration/utils/nodes.py
Comment thread contentcuration/contentcuration/utils/publish.py
Comment thread contentcuration/contentcuration/models.py Outdated
rtibbles and others added 3 commits September 23, 2026 13:45
Swap stage of the zero-downtime int->bigint widening, on top of the
expand stage in 0167:
- Drop the mirror trigger and the int column, rename file_size_bigint
  onto file_size, and rename its index onto the vacated name. All
  metadata-only, in one transaction.
- Keep the shadow index's partial predicate, and filter file_size
  IS NOT NULL in the dedupe-then-Sum queries so they stay eligible for
  it. Sum already ignores NULLs, so the totals are unchanged.
- Read file_size directly when publishing; Kolibri's LocalFile keeps its
  own 32-bit column and bigint shadow.
- Retire the backfill deploy-migrate step.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015PZU9jWMFWezWayyhuSkyL
The linter runs git diff --relative from --project-root-path and only
treats a changed file as a migration when its path contains
/migrations/. Defaulting to the settings directory truncated every path
past that segment, so the step has never linted anything.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015PZU9jWMFWezWayyhuSkyL
Describe the procedure in terms of any field and its shadow, with the
file_size widening as the worked example rather than the subject. Add
the shadow-index rule the swap depends on, and correct the linter
escape hatch: IgnoreMigration imports a dev-only dependency from a
migration production loads.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015PZU9jWMFWezWayyhuSkyL
@bjester

bjester commented Sep 28, 2026

Copy link
Copy Markdown
Member

I'm unsure whether this is an issue with this PR specifically or hotfixes, but it seems that when I upload files to my local studio instance, the nodes end up incomplete. It seems the file objects for the uploads are not having contentnode_id set on them, which I presume is connected.

@bjester
bjester self-requested a review September 28, 2026 19:26
@bjester
bjester dismissed their stale review September 28, 2026 19:26

Resolved

@bjester bjester left a comment

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 can confirm that I do not experience the incomplete-node issue on a fresh branch off hotfixes, so it appears related to the changes in this PR. I used an old database with existing data, per the instructions, so please let me know if you can't recreate the issue

@bjester bjester left a comment

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.

LGTM!

@rtibbles
rtibbles merged commit 3e143e0 into learningequality:hotfixes Sep 29, 2026
22 checks passed
@rtibbles
rtibbles deleted the file_size_bigint_swap branch September 29, 2026 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants