Skip to content

fix(db): stagger the cleanup/model-sync 6h schedulers and index conversation_turn_nodes.last_seen_at (#13973) - #14567

Merged
diegosouzapw merged 4 commits into
release/v3.8.51from
fix/13973-scheduler-jitter-truncate
Sep 24, 2026
Merged

diegosouzapw merged 4 commits into
release/v3.8.51from
fix/13973-scheduler-jitter-truncate

Conversation

@diegosouzapw

@diegosouzapw diegosouzapw commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

Closes #13973

⚠️ base-red inherited: #14547

Root cause

PR #14005 already removed the periodic wal_checkpoint(TRUNCATE) that caused the original SIGBUS. Two items were explicitly left out of that fix and stayed the remaining scope of this issue:

  1. src/lib/db/cleanup.ts's CLEANUP_INTERVAL_MS and src/shared/services/modelSyncScheduler.ts's DEFAULT_INTERVAL_MS are both exactly 6h with zero relative offset, and both schedulers are started back-to-back in the same boot sequence (src/instrumentation-node.ts → startCleanupScheduler() alongside ensureCloudSyncInitialized() → startModelSyncScheduler()), so their periodic ticks land in the same wall-clock second every 6 hours for the life of the process.
  2. conversation_turn_nodes has no index on last_seen_at (migration 156 only indexes conversation_id/parent_id/content_hash), which cleanup.ts's own doc comment already flagged as making the retention DELETE a full table scan.

Fix

  • src/shared/services/modelSyncScheduler.ts: added MODEL_SYNC_STAGGER_OFFSET_MS (45 minutes) and apply it to the periodic setInterval delay (effectiveIntervalMs + MODEL_SYNC_STAGGER_OFFSET_MS), so the model-sync scheduler's tick is phase-shifted relative to cleanup.ts's own un-offset 6h interval. cleanup.ts itself is unchanged — it keeps its plain 6h cadence, which is now the reference phase.
  • src/lib/db/migrations/185_conversation_turn_nodes_last_seen_index.sql: additive CREATE INDEX IF NOT EXISTS idx_turn_nodes_last_seen ON conversation_turn_nodes(last_seen_at).
  • src/lib/db/cleanup.ts: updated the cleanupConversationTurnNodes doc comment to stop claiming last_seen_at has no index.

Remaining

A third scheduler with the same default 6h period exists (src/lib/db/core.ts's DB health-check timer, getDbHealthCheckIntervalMs() / startDbHealthCheckScheduler()), gated off entirely during automated test runs (isAutomatedTestProcess()), so no test currently proves or disproves a collision with it, and src/lib/db/core.ts is at 1799/1800 lines on the frozen file-size baseline (essentially no headroom to add a stagger there safely). Left out of this PR's surgical scope; flagging as a candidate follow-up rather than guessing at an untested change to a near-frozen file.

Regression test

tests/unit/scheduler-6h-stagger-13973.test.ts — spies on the global setInterval/setTimeout constructors around the real, unmocked startCleanupScheduler()/startModelSyncScheduler() calls and captures the delay each one registers.

  • RED (unfixed code): AssertionError [ERR_ASSERTION]: cleanup scheduler and model-sync scheduler must register a staggered 6h setInterval with a non-zero relative offset ... { actual: 21600000, expected: 21600000, operator: 'notStrictEqual' } — both schedulers registered the identical 21600000ms delay.
  • GREEN (fixed code): pass 1, fail 0 — cleanup keeps 21600000ms, model-sync registers 21600000 + 2700000 = 24300000ms (45-minute stagger), and the assertions on both the delay-length and the "cleanup unstaggered / model-sync staggered" relationship hold.

Existing tests

  • tests/unit/model-sync-scheduler.test.ts — aligned the one pre-existing assertion that hardcoded the periodic interval delay as exactly 6 * 60 * 60 * 1000 (that assertion encoded the old, now-fixed same-second-collision contract) to 6 * 60 * 60 * 1000 + scheduler.MODEL_SYNC_STAGGER_OFFSET_MS. No other assertion in that file touches the interval delay value. Ran green (EXIT:0).
  • tests/unit/db-cleanup-conversation-nodes-12453.test.ts — unrelated to the index/stagger change, ran to confirm no regression in cleanupConversationTurnNodes. Ran green (EXIT:0).

Gates run

  • npm run typecheck:core → exit 0
  • npx eslint --suppressions-location config/quality/eslint-suppressions.json <every changed file> → exit 0
  • node scripts/check/check-file-size.mjs → OK (154 frozen files, 4773 checked; neither touched file is on the frozen list)
  • node scripts/check/check-complexity-ratchets.mjs --base-ref origin/release/v3.8.51 → OK — new-code mode, 2 changed files in scope, 0 cyclomatic violations, cognitive complexity unchanged at its pre-existing base of 1
  • node scripts/check/check-changelog-integrity.mjs → OK
  • node scripts/check/check-mutation-test-coverage.mjs --strict → fails, but on 4 modules this PR does not touch (open-sse/services/accountFallback.ts, src/sse/services/auth.ts, open-sse/services/combo/comboStructure.ts, open-sse/services/combo/quotaScoring.ts) and 8 pre-existing test files not registered in stryker.conf.json's tap.testFiles; none of this PR's files or new test appear in that gate's output. Pre-existing gap unrelated to this diff.
  • node --import tsx/esm --test tests/unit/scheduler-6h-stagger-13973.test.ts → RED before fix, GREEN after (see above)
  • node --import tsx/esm --test tests/unit/model-sync-scheduler.test.ts → GREEN after aligning the superseded assertion
  • node --import tsx/esm --test tests/unit/db-cleanup-conversation-nodes-12453.test.ts → GREEN, unaffected

Plan-file: _tasks/pipeline/bugs/2-implementing/13973-fix-backend-three-6h-schedulers-fire-in-the-same-second-trunca.plan.md

Rework (merge-batch 2026-09-23)

Two defects found in review, fixed in this branch:

  1. Migration number collision (boot-breaking). 185_conversation_turn_nodes_last_seen_index.sql collided with 185_usage_history_cpa_auth_index.sql, which landed on the release tip in feat(providers): correlate X-CPA-TRACE-ID auth_index with usage history #14544. The migration runner throws Migration version collision detected, so boot fails. The migration is now 186_conversation_turn_nodes_last_seen_index.sql, and the comment in cleanup.ts was updated to match. The index itself is unchanged. Heads-up: open PRs fix(call-logs): record encrypted reasoning presence, duration and effort #14680, fix(dashboard): TPS over generation time (duration − TTFT) + reasoning-aware numerator (#13130) #13373 and feat(api): authenticate Claude Code to /v1/* with Entra ID SSO #14665 also add a 186_* migration, so whichever of them lands after this one will need to renumber.
  2. The stagger changed the period, not the phase. The PR added 45 min to the model-sync setInterval period (6h45m). With a different period the two schedulers drift apart and then collide again, and the operator's MODEL_SYNC_INTERVAL_HOURS was silently overridden. The fix arms the recurring interval from a one-shot MODEL_SYNC_STAGGER_OFFSET_MS (45 min, unref'd) phase timer. The period stays exactly effectiveIntervalMs, and stopModelSyncScheduler() also cancels a pending phase timer.

Tests:

  • tests/unit/scheduler-6h-stagger-13973.test.ts was rewritten. It asserts that cleanup arms 6h at boot, that model-sync arms no interval at boot and only a phase timer of MODEL_SYNC_STAGGER_OFFSET_MS, and that the interval it arms is exactly 6h. The capture timers are inert, so no real DB/HTTP work runs.
  • tests/unit/model-sync-scheduler.test.ts was aligned to the new behavior. A new test covers MODEL_SYNC_INTERVAL_HOURS=4: the period stays at exactly 4h, the first tick is delayed by the offset, and stop cancels the pending arm.
  • Red→green: against the original PR scheduler (e2a731af) the new or updated tests fail (3 tests). With the fix, model-sync-scheduler passes 16/16 (2 runs) and scheduler-6h-stagger-13973 passes 1/1.
  • The migration suites pass after merging origin/release/v3.8.51: migration-135-numbering-collision, db-migration-version-uniqueness, check-migration-numbering, db-migration-runner, db-core-migration and db-migration-missing-physical-schema. check-migration-numbering is OK.

Gates: typecheck:core shows only the inherited cliproxyAccountHealth.ts(157,5) error. check:open-sse-typecheck has 2 inherited auggie.ts regressions from the tip (#14215, tracked in #14547); this PR has no open-sse/ diff. eslint is clean on the changed files, and file-size is OK when measured after the commit.

…enumber migration to 186 (#13973)

The stagger added 45 min to the model-sync PERIOD (6h45m), so the two
schedulers only drifted and re-collided, and MODEL_SYNC_INTERVAL_HOURS was
silently overridden. Arm the recurring interval after a one-shot 45 min
phase timer instead, keeping effectiveIntervalMs unchanged, and cancel the
pending arm on stop.

Migration 185 collided with 185_usage_history_cpa_auth_index.sql on the
release tip (#14544), which aborts boot; renumber to 186.
…phase test

Without the flush, the fire-and-forget import armed a real loopback poll
after the stubs were restored, leaking a fetch into the concurrency-cap test.
@diegosouzapw
diegosouzapw merged commit fa3555f into release/v3.8.51 Sep 24, 2026
12 of 21 checks passed
diegosouzapw added a commit that referenced this pull request Sep 24, 2026
…w-up) (#14714)

Acompanhamento da leva /merge-batch 2026-09-23: contagem de migrations 182→183 depois da #14567 (migration 186). Só o número muda, aprovado pelo dono. check-docs-counts-sync sem drift STRICT.
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.

fix(backend): three 6h schedulers fire in the same second; TRUNCATE checkpoint SIGBUSes under live mmap

1 participant