Skip to content

fix(governor-state): make scalar-state pause/rate-limit/cap-usage saves atomic against concurrent writers #7221

Description

@JSONbored

Context

packages/loopover-miner/lib/governor-state.js stores three conceptually independent pieces of mutable governor state — rate-limit buckets/backoff, cap usage, and the pause/resume flag (#4851) — in one shared row (governor_scalar_state, id=1). All three save paths (saveRateLimitState lines 220-231, saveCapUsage lines 236-247, savePauseState lines 261-277) follow the same pattern: const row = getScalarStatement.get(); followed by upsertScalarStatement.run(...), where the columns each call doesn't own are filled in from the just-read row snapshot (e.g. saveRateLimitState passes row ? row.paused : 0 for the pause columns it isn't touching). This SELECT and the subsequent INSERT ... ON CONFLICT(id) DO UPDATE SET (lines 166-178) are two separate SQL statements with no surrounding transactionopenLocalStoreDb only sets PRAGMA busy_timeout, which serializes individual statements but does nothing to make a SELECT-then-UPSERT pair atomic across two different processes' connections to the same file.

This is a real, intended concurrent-access pattern, not a misuse: governor-pause-cli.js's own header comment says the pause flag is "a real, persisted pause flag an operator... can toggle via this CLI, that loop-cli.js's iteration loop actually checks before each cycle" — the pause CLI is meant to be invoked while loop-cli.js runs as a separate long-lived process. loop-cli.js's in-flight attempts call evaluateGovernorChokepointGatePersisted (governor-chokepoint-persisted.js:41), which calls governorState.saveRateLimitState(...) on every gated write. packages/loopover-miner/docs/operations-runbook.md:43-48 explicitly documents "two short-lived writers on the same file" as supported, and states as a general guarantee that "INSERT … ON CONFLICT ... patterns avoid read-then-write races within one file."

The bug: saveRateLimitState/saveCapUsage/savePauseState don't actually follow that safe pattern — they read-then-write across columns they don't own. Two sibling functions in the same file do it correctly with a pure, per-column excluded.* upsert and no prior SELECT dependency for the columns they don't touch: upsertReputationStatement's use by saveReputationHistory (lines 182-189, 285-292) and recordOwnSubmission (a pure INSERT). run-state.js's setRunState (lines 101-129) is the cross-file example of the same safe idiom.

Concrete consequence: if a governor pause/resume CLI invocation's read+write interleaves with an in-flight loop/attempt process's saveRateLimitState/saveCapUsage call (or vice versa), one write's column-group is silently reverted to a stale snapshot. For the pause flag specifically, an operator's emergency pause — the intended manual override sitting right next to the kill-switch and run-halt in this same safety-critical subsystem — can be silently undone with no error, no log line, and no ledger event. The miner keeps writing.

Requirements

  • saveRateLimitState, saveCapUsage, and savePauseState in packages/loopover-miner/lib/governor-state.js must not lose a concurrent writer's update to a disjoint column-group of governor_scalar_state when their own read-then-write window overlaps with another process's read-then-write window on the same file.
  • The fix must preserve every currently-tested behavior: default-fallback values when no row exists yet, the trimmed/null reason handling in savePauseState, and the return shapes of all three methods.
  • A failure partway through must leave the row exactly as it was before the call (no partial writes).
  • Public signatures and call sites (governor-chokepoint-persisted.js, governor-pause-cli.js, loop-cli.js) must not need to change.

Deliverables

  • Atomic read+write for saveRateLimitState/saveCapUsage/savePauseState in governor-state.js — e.g. wrap each method's SELECT+UPSERT in an explicit BEGIN IMMEDIATE/COMMIT (with rollback on error), or restructure governor_scalar_state so each column-group has its own row/table and a pure excluded.*-only upsert (matching the already-proven-safe saveReputationHistory/run-state.js pattern) — implementer's choice, but the interleaving this issue describes must no longer lose data either way.
  • A regression test proving the interleaving (two independent handles/connections on the same on-disk file, one saving pause while the other saves rate-limit/cap-usage state with a stale read) no longer clobbers the other's write.

Test Coverage Requirements

packages/loopover-miner/lib/governor-state.js is covered by Codecov's patch gate (packages/loopover-miner/lib/** is not in codecov.yml's ignore list) — 99%+ patch coverage, branch-counted, applies in full to every line/branch touched by the fix, including the new transaction-commit success path and the rollback/error path.

  • Extend test/unit/miner-governor-state.test.ts (currently only exercises sequential single-handle calls — no test opens two handles on the same file and interleaves them) with a case that opens two openGovernorState(samePath) handles and demonstrates the fixed atomicity.
  • Cover both branches of whatever transaction-boundary code is added (commit-succeeds and rollback-on-error), not just the happy path.

Expected Outcome

After this ships, a loopover-miner governor pause/resume issued while the loop daemon (or an in-flight attempt) is mid-write to rate-limit/cap-usage state can no longer be silently reverted, and symmetrically a rate-limit/cap-usage save can no longer discard a concurrent pause/resume. governor_scalar_state's three save paths get the same per-column-group durability guarantee that saveReputationHistory, recordOwnSubmission, and run-state.js's setRunState already provide.

Links & Resources

  • packages/loopover-miner/lib/governor-state.jsupsertScalarStatement (166-178), saveRateLimitState (220-231), saveCapUsage (236-247), savePauseState (261-277); safe sibling pattern: upsertReputationStatement (182-189) + saveReputationHistory (285-292)
  • packages/loopover-miner/lib/run-state.jssetRunState (101-129), the cross-file safe atomic-upsert idiom
  • packages/loopover-miner/lib/governor-pause-cli.js — the CLI that races against the loop daemon
  • packages/loopover-miner/lib/governor-chokepoint-persisted.js:41 — the call site that saves rate-limit state on every gated write
  • packages/loopover-miner/lib/loop-cli.js:289-347 — the long-running daemon reading pause state each cycle
  • packages/loopover-miner/docs/operations-runbook.md:43-48 — the documented concurrency contract this violates
  • test/unit/miner-governor-state.test.ts — existing coverage (sequential-only, no concurrent-writer case)
  • Related (closed): Persist governor cross-attempt state (rate-limit, budget, convergence, reputation, self-plagiarism) #5134 (introduced this store), Build a governor pause/resume control surface #4851 (added the pause columns to the same row)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions