Skip to content

fix(omnigraph): make the storage-format migration Job rebuild into omnigraph 0.13 - #6350

Merged
blarghmatey merged 2 commits into
mainfrom
omnigraph-013-migration-job
Oct 13, 2026
Merged

blarghmatey merged 2 commits into
mainfrom
omnigraph-013-migration-job

Conversation

@blarghmatey

Copy link
Copy Markdown
Member

What are the relevant tickets?

witan task tk-patch-the-storage-format-migration-job-for-fmt9--bbde00, part of the omnigraph 0.13 / storage format 14 upgrade (D8 of the spec in mitodl/agent-kit#461).

Description (What does it do?)

The migration Job was written for the 0.10 to 0.11 rebuild. Against a 0.13 binary it stops at its first write: cluster import no longer exists, and from 0.12 every direct write on a cluster root (cluster apply, load, optimize) keeps the root's exclusive lock after it exits, so the next write is refused with state_lock_held.

  • Drops cluster import. A direct cluster apply on an empty root creates the ledger.
  • Releases each write's lock by the id that command printed (Admission lock: <id> from apply, lock_id=<id> from load and optimize), after the command has exited. A write that reports no single id is not unlocked. A failed write keeps its lock, because 0.13 reports its effects as unknown. The rebuild fails if the new root is still locked at the end, since the server's first boot there would be refused.
  • Bounds load batches by bytes as well as rows. 0.13 refuses a keyed write over 32 MiB of parsed rows per table (resource limit exceeded for keyed parsed entity bytes), and the 8,192-row cap is still there. Batches close at 16 MiB of JSONL. The cap counts parsed values and I only measured how strings are counted, hence half.
  • Removes the 0.10 export rewrite (id relocation, keyed-edge duplicate collapse). A 0.11 export loads into 0.13 unchanged. The verdict loses collapsed_duplicates, and bin/omnigraph-cutover-pr drops that column.
  • The pre-flight also waits for the omnigraph-server Deployment to declare 0 replicas with no pod behind its selector outside phase Failed/Succeeded. depends_on only orders the Job after the scale-down update, and the old pod keeps committing until it exits. The armed-only Role in the omnigraph namespace gains get on that Deployment and list on pods.
  • The runbook's manual step 4 and its lock troubleshooting entry are rewritten for 0.13, and it now says a failed run leaves fmt<N> half-built and locked, to be deleted before a re-run.

The writer list still names the optimize and cleanup CronJobs. Replacing them with the single stop-window CronJob belongs to the stack change for 0.13.

This is not enough to cut over by itself. At cutover the stack still recreates the cluster-apply Job, and on 0.13 even a no-op direct apply leaves a lock on the root this Job left unlocked. The stack change for 0.13 (task tk-ol-infrastructure-omnigraph-stack-for-0-12-clust-2dee94) removes that Job on format 14 and has to merge before any environment's cutover.

How can this be tested?

No stack has a migration armed, so nothing changes on pulumi up in any environment. I did not run an armed pulumi preview.

  • I ran the script's export, rebuild and verify functions with the real binaries: 0.11.0 reading s3://ol-data-witan-ci/fmt9 read-only, 0.13.0 rebuilding into a local file:// root. Four graphs (council, code-bridge, and the ol-infrastructure and ol-concourse code graphs), 178,595 rows. Every table matched, format 9 -> 14 on all four, 28 locks taken and released, root unlocked at the end. omnigraph-server 0.13.0 then booted on the rebuilt root and /readyz reported serving with 4 of 4 graphs ready. The ol-infrastructure graph (largest table 44,492 rows, 95 MiB export) went in as 20 batches.
  • Not covered by that run: S3 as the new root, the pre-flight against a real API server, the other 14 CI graphs, and the image's baked schema directory. Those need the Job armed in CI with a 0.13 image, which does not exist until the agent-kit pin bump.
  • The 32 MiB and 8,192-row refusals, the lock a failed load leaves, and the runbook's write shell helper were each reproduced on 0.13.0 against a local root.
  • pytest tests/ol_infrastructure/applications/omnigraph/ tests/test_omnigraph_cutover_pr.py: 237 passed.

https://claude.ai/code/session_01SfdKkJ4YSVXssjVYmBPTWh

…nigraph 0.13

The Job was written for a 0.10 to 0.11 rebuild. Against a 0.13 binary it
stops at its first write: `cluster import` no longer exists, and from 0.12
every direct write on a cluster root (`cluster apply`, `load`, `optimize`)
keeps the root's exclusive lock after it exits, so the second write is
refused with `state_lock_held`.

- Drop `cluster import`. A direct apply on an empty root creates the ledger.
- Release each write's lock by the id the command printed, once it has
  exited, and fail if the new root is still locked when the rebuild ends. A
  lock left there refuses the server's first boot. A failed write keeps its
  lock.
- Bound load batches by bytes as well as rows. 0.13 refuses a keyed write
  over 32 MiB of parsed rows per table.
- Remove the 0.10 export rewrite (id relocation, keyed-edge collapse). A
  0.11 export loads into 0.13 unchanged, so the verdict and the cutover PR
  table no longer carry collapsed duplicates.
- The pre-flight also waits for the server Deployment to declare 0 replicas
  with no live pod. Its Role gains get on that Deployment and list on pods.

Claude-Session: https://claude.ai/code/session_01SfdKkJ4YSVXssjVYmBPTWh
Copilot AI balanced review requested due to automatic review settings October 13, 2026 01:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The production-critical S3 and Kubernetes migration path remains untested with an armed Job and 0.13 image.

0 open findings

What changed in this PR

Updates the Omnigraph storage migration workflow for the 0.11-to-0.13 format upgrade.

Changes:

  • Handles persistent write locks and removes obsolete export rewriting.
  • Adds byte-bounded batching and server shutdown preflight checks.
  • Updates cutover tooling, runbook guidance, and tests.
File Description
src/​.../​storage_migration.py Adds server preflight RBAC and configuration.
src/​.../​scripts/​migrate_storage_format.py Implements lock release, batching, and 0.13 rebuild behavior.
bin/​omnigraph-cutover-pr Updates verdict rendering.
docs/​omnigraph-storage-format-upgrade-runbook.md Documents the 0.13 procedure.
tests/​.../​test_storage_migration.py Tests migration logic and preflight behavior.
tests/​.../​test_storage_migration_job.py Tests Job RBAC and environment configuration.
tests/​test_omnigraph_cutover_pr.py Updates cutover verdict tests.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@blarghmatey
blarghmatey merged commit a85abb1 into main Oct 13, 2026
13 checks passed
@blarghmatey
blarghmatey deleted the omnigraph-013-migration-job branch October 13, 2026 13:58
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