improvement(search): run Search retirement as a throttled background migration - #8521
waleedlatif1 wants to merge 1 commit into
Conversation
…migration The deploy advances the retirement for one minute and defers; a scheduled workflow advances it between deploys in resumable slices paced on commit latency, WAL rate and replication lag, starts index maintenance only in an off-peak window, and journals 0029 once everything is finished.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
|
| return 'deferred' | ||
| } | ||
| try { | ||
| if (!(await prepareTargets(sql))) return 'complete' |
There was a problem hiding this comment.
Initial snapshot exceeds deploy budget
If the first deploy has many Search knowledge bases, prepareTargets scans and records all of them before the deadline is checked. The migration can remain on the deploy path well beyond its promised one-minute slice without starting a retirement page. The initial snapshot needs a budget or a resumable checkpoint.
Knowledge Base Used: Database schema and migrations
| const pause = (ms: number) => sleep(Math.max(0, Math.min(ms, deadline - performance.now()))) | ||
|
|
||
| for (;;) { | ||
| if (performance.now() >= deadline) { |
There was a problem hiding this comment.
Completed retirement repeats lengthy validation
When retirement is saved as done but maintenance is still pending, each deploy repeats the full target-marker validation. The deadline is checked only before that call, which has a 30-minute statement timeout. A deploy intended to defer after one minute can therefore wait for the recheck on every release until maintenance journals 0029.
Knowledge Base Used: Database schema and migrations
| const dutyPauseMs = pageMs * throttle.dutyRatio | ||
| /** Long enough that the WAL written during the page, spread over page and pause, fits the budget. */ | ||
| const walPauseMs = walBytes / walBytesPerMs - pageMs | ||
| const strainPauseMs = state === 'slow_commit' ? backoffMs() : 0 | ||
| const pauseMs = Math.max( | ||
| 0, | ||
| Math.min(throttle.maxPauseMs, Math.max(dutyPauseMs, walPauseMs, strainPauseMs)) |
There was a problem hiding this comment.
WAL pressure can outlast pause
If a page and other database writers produce enough WAL to require more than five minutes of pacing, this code caps the pause anyway. It also does not count WAL written during the pause before starting the next page. Retirement can thus add more WAL while the database remains above the configured rate budget.
| if (page.transition) { | ||
| /** A phase change may include a full recheck, which says nothing about page cost. */ | ||
| state = 'phase_change' | ||
| logger.info('Search retirement phase changed', { | ||
| transition: page.transition, | ||
| batches, | ||
| mutated, | ||
| }) | ||
| } else if (page.commitMs > throttle.slowCommitMs) { |
There was a problem hiding this comment.
Phase changes skip commit backoff
A phase-change page whose commit takes longer than slowCommitMs enters this branch before the slow-commit check. It neither reduces the next page size nor applies the commit back-off. That leaves a gap in the pressure response precisely when a slow commit is observed; phase changes can be exempted from page-duration sizing without ignoring commit latency.
|
Superseded: the retirement moves out of the deploy entirely and runs as an operator-run, throttled maintenance command. A new PR replaces this one. |
Summary
Takes
0029_retire_all_search_embeddingsoff the deploy's critical path. The deploy slice advances the retirement for at most one minute, never starts index maintenance, and throwsScriptMigrationDeferred. The release then switches over, and0029stays unrecorded until the cleanup is actually finished.Adds
.github/workflows/search-retirement.ymlas the background runner. It runs an hourly 50-minute retirement slice against staging and production, using the same secrets and migration role asmigrations.yml. A daily off-peak slice (03:07 UTC) is the only scheduled run allowed to startREINDEX CONCURRENTLYandVACUUM. The first slice that finishes both journals0029and its superseded names. Every slice resumes the existing cursor and maintenance checkpoints, with no reset.Throttles on database health, not just page time. After each page it pauses for the longest of:
pg_current_wal_lsn()and counting every writer, under 10% ofmax_wal_sizepercheckpoint_timeout, so checkpoints stay time-triggered;It also pauses before a page when
pg_stat_replicationlag exceeds 10 s.Session advisory lock: only one runner retires pages at a time. A deploy that finds the background runner active defers at once.
Maintenance is budgeted. No rebuild or vacuum starts after the slice's budget, and one already started finishes, since a cancelled concurrent rebuild starts over. Vacuums run with autovacuum's 2 ms
vacuum_cost_delayinstead of the unthrottled manual default.One structured
Search retirement batchlog per page:duty/wal/backoff/slow_commit/slow_page/phase_change/replica_lag);Rewrites the runbook (
search-embedding-retirement.md): execution model, pacing, observing, and pausing (gh workflow disable search-retirement.ymlplus cancelling the in-flight run, which rolls back with its cursor).Why
A bulk delete run inside the deploy pushed WAL far past
max_wal_size. Checkpoints ran back to back, and each one re-logs a full page image on the first touch. fsync and synchronous-replication commit waits stretched, and unrelated app writes hit lock and statement timeouts. Page-time adaptation alone did not stop this: each page stayed under its own timeout while the cluster degraded. The app does not depend on this cleanup finishing (documents are already fenced), so it belongs outside the deploy, paced on the database's own pressure.Research
ensure_batched_background_migration_is_finished. The migration pauses on database health indicators: WAL rate, the WAL archive queue, autovacuum on the touched tables, and Patroni apdex. https://docs.gitlab.com/development/database/batched_background_migrations/--max-lag-millis) and load, and supports manual throttling or pausing. https://github.com/github/gh-ost/blob/master/doc/throttle.md--max-lagand--max-loadchecked after every chunk, and--chunk-timesizing. https://docs.percona.com/percona-toolkit/pt-online-schema-change.htmlcheckpoint_timeoutor whenmax_wal_sizeis about to be exceeded, and shorter intervals raise WAL volume through full-page writes: https://www.postgresql.org/docs/17/wal-configuration.htmlVACUUMhas no cost delay by default: https://www.postgresql.org/docs/17/runtime-config-resource.htmlpg_stat_replicationlag columns: https://www.postgresql.org/docs/17/monitoring-stats.htmlpg_current_wal_lsn/pg_wal_lsn_diff: https://www.postgresql.org/docs/17/functions-admin.htmlChosen pattern: GitLab's (deploy enqueues, background worker executes, health throttling, finalize when done), using the infrastructure Sim already has:
ScriptMigrationDeferredplus thescript_migrationsjournal for "unfinished, retry later";The cron routes and Trigger.dev tasks run as the app role. That role cannot rebuild or vacuum these tables and cannot read replication lag, so using them would have meant new credentials.
Type of Change
Testing
0027_retire_search_embeddings.integration.ts(real Postgres + pgvector). Each new test went red with its guard reverted:--maintenanceslice rebuilt, vacuumed and journaled.bun run test:integration(packages/db: 131 passed; apps/sim: 1274 passed), packages/db unit tests,bun run type-check,bun run lint,bun run check:audits,docs-manifest:check.Checklist
test-auditauthoring gate)