Skip to content

fix(selfhost): prevent maintenance queue self-pinning#2782

Merged
JSONbored merged 1 commit into
mainfrom
fix/selfhost-maintenance-backpressure-deadlock
Jul 3, 2026
Merged

fix(selfhost): prevent maintenance queue self-pinning#2782
JSONbored merged 1 commit into
mainfrom
fix/selfhost-maintenance-backpressure-deadlock

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • The self-host maintenance-admission policy (feat(selfhost): defer maintenance work under runtime pressure #2717) could self-pin: maintenance_pending_high is a lane-wide aggregate-count check with no feedback loop back to that count as jobs individually age out via the existing 4h trickle. Once the lane backed up past its threshold (e.g. the reported 68-pending incident: 30 rag-index-repo + 29 notify-evaluate + 9 other maintenance jobs, 0 live jobs pending, no rate-limit activity), every claim was denied maintenance_pending_high until each job individually reached the full MAINTENANCE_ADMISSION_MAX_DEFER_AGE_MS (4h) — deferred because the lane was over threshold, stuck over threshold because everything stayed deferred.
  • This PR adds a second, much shorter age escape (MAINTENANCE_ADMISSION_DRAIN_AGE_MS, default 10m, clamped to never exceed the 4h backstop) scoped only to the maintenance_pending_high reason, so the oldest jobs in a backed-up lane drain in a bounded trickle (further throttled by the queue's existing QUEUE_BACKGROUND_CONCURRENCY cap) well before the outer 4h ceiling. live_pending_high, live_job_age_high, and host_load_high are untouched hard blocks — live-review priority and host-load safety are preserved, and the drain escape itself is still blocked if host load is also high.
  • It also closes two real coalescing gaps that were inflating the maintenance lane: multiple merge-triggered rag-index-repo incrementals for the same repo now union into one pending row (bounded to 100 total paths) instead of piling up as separate rows, and a webhook delivery's notification events (review events + issue-watch matches) now batch into a single notify-evaluate job instead of one job per event/watcher.
  • Investigation showed the live-vs-maintenance queue-depth split, the _by_reason_total deferral counter, and the Grafana "Runtime Pressure & Maintenance" panels described in the bug report already exist (shipped in feat(selfhost): defer maintenance work under runtime pressure #2717) and are correctly wired — so this PR's observability work adds the complementary piece: a gittensory_jobs_maintenance_admission_granted_under_pressure_total{reason,job_type} counter (mirrors the existing denied-by-reason counter, but for admissions that happened despite pressure) plus a new dashboard panel and an updated alert runbook, so an operator can see the new drain escape actually firing rather than just inferring it from queue depth alone.

What changed

  • src/selfhost/maintenance-admission.ts — new maintenanceDrainAgeMs config field (env MAINTENANCE_ADMISSION_DRAIN_AGE_MS, clamped to maxDeferAgeMs), new maintenance_pending_high_drain reason, and isMaintenanceAdmissionGrantedUnderPressure() helper.
  • src/selfhost/sqlite-queue.ts / src/selfhost/pg-queue.ts — record the new granted-under-pressure metric; add the rag-index-repo incremental-merge coalescing branch to enqueue() (mirrored across both backends).
  • src/selfhost/queue-common.tsjobCoalesceMergeKeyPrefix() / jobCoalesceMergedPayload() (bounded to 100 paths); notify-evaluate's coalesce key now derives from a batch's full sorted dedup-key set.
  • src/types.ts / src/queue/processors.tsnotify-evaluate now carries events: DetectedNotificationEvent[]; the webhook handler batches one job per delivery instead of one per event, and the processor evaluates every event in the batch.
  • grafana/dashboards/gittensory.json / prometheus/rules/alerts.yml — new panel for the granted-under-pressure metric; updated GittensoryMaintenanceStarved runbook to reference both age escapes and the new metric.
  • .env.example / apps/gittensory-ui/src/lib/selfhost-env-reference.ts — documented + regenerated for the new env var.

Why

Directly reported operational incident: a self-host VPS showed 68 pending jobs, all maintenance, 0 live/due-now jobs, repeated selfhost_queue_maintenance_admission_deferred reason=maintenance_pending_high logs, and a dashboard that (per the report) made it look like review work was stuck — root-caused during investigation to the admission policy's aggregate-count check having no bounded-rate release valve short of the full 4h trickle, plus real (if secondary) coalescing gaps in the two job types that made up the bulk of the backlog.

Validation

  • git diff --check — clean
  • npm run actionlint — pass
  • npm run db:migrations:check — pass (105 migrations OK, next free 0103; no new migration needed, no schema change)
  • npm run db:schema-drift:check — pass
  • npm run selfhost:env-reference:check — pass (regenerated selfhost-env-reference.ts for the new env var)
  • npm run selfhost:validate-observability — pass (dashboards + alert rules valid)
  • npm run cf-typegen:check — pass (no wrangler.jsonc changes)
  • npm run typecheck — pass
  • npm run test:coverage (full, unsharded) — 7298 passed, 0 failed, 7 skipped across 375 test files; global 96.58%/95.66%/95.75%/97.14% (stmt/branch/func/line); every changed line and branch in this diff traced individually against lcov.info and confirmed covered (new admission-policy branches, both merge-coalescing branches in both queue backends, the notify-evaluate batching path, and the granted-under-pressure metric path all have dedicated tests)
  • npm run test:workers — pass
  • npm run build:mcp / npm run test:mcp-pack / npm run build:miner / npm run rees:test — pass
  • npm run ui:openapi:check / npm run ui:openapi:settings-parity / npm run ui:version-audit — pass
  • npm run ui:lint / npm run ui:typecheck / npm run ui:test / npm run ui:build — pass
  • npm audit --audit-level=moderate — 0 vulnerabilities

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused (self-host queue backpressure + its necessary observability/generated-artifact updates); no unrelated UI/MCP/docs/dependency changes.
  • Follows CONTRIBUTING.md; no site//CNAME/GitHub Pages changes.
  • No linked issue — this is a direct fix for a described production incident with its own root-cause investigation and regression tests; the summary above explains the "why".

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally — see Validation section above (full run, 100% of changed lines/branches individually confirmed covered against lcov.info)
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized — no public-facing text changed by this PR.
  • N/A — no auth, cookie, CORS, GitHub App, Cloudflare, or session changes.
  • N/A — no OpenAPI/MCP-visible behavior change (self-host queue internals + Grafana/alerts only).
  • N/A — no UI changes (backend/self-host + observability config only).
  • N/A — no visible UI change, so no UI Evidence section.
  • N/A — no public docs/changelog change needed.

Notes

  • The self-pinning root cause and the observability requirements in the original report were only partially new work: the live-vs-maintenance queue-depth split, the by-reason deferral counter, and the Grafana panels for both already existed and are correctly wired (shipped with feat(selfhost): defer maintenance work under runtime pressure #2717) — verified directly against the current dashboard JSON and server.ts before writing this PR, rather than assuming the report's framing. This PR's actual gap was the admission policy's missing bounded-rate release valve, plus the two coalescing gaps and the complementary granted-under-pressure metric.
  • pg-queue.ts is Codecov-ignored (validated by integration/smoke tests, not unit coverage per codecov.yml) — its changes are still mirrored line-for-line from sqlite-queue.ts and covered by dedicated mocked-Pool unit tests for regression insurance, matching the existing convention for that file.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 3, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 66413b7 Commit Preview URL

Branch Preview URL
Jul 03 2026, 09:52 PM

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.22%. Comparing base (4e48d2d) to head (66413b7).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/queue/processors.ts 88.23% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2782   +/-   ##
=======================================
  Coverage   96.22%   96.22%           
=======================================
  Files         255      255           
  Lines       27944    27998   +54     
  Branches    10156    10174   +18     
=======================================
+ Hits        26889    26941   +52     
  Misses        433      433           
- Partials      622      624    +2     
Files with missing lines Coverage Δ
src/selfhost/maintenance-admission.ts 100.00% <100.00%> (ø)
src/selfhost/queue-common.ts 94.57% <100.00%> (+0.31%) ⬆️
src/selfhost/sqlite-queue.ts 99.47% <100.00%> (+0.01%) ⬆️
src/queue/processors.ts 92.76% <88.23%> (-0.04%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 3, 2026
@loopover-orb

loopover-orb Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-03 21:07:01 UTC

16 files · 1 AI reviewer · 1 blocker · readiness 87/100 · CI failing · blocked

🛑 Suggested Action - Manual Review

  • AI reviewers agree on a likely critical defect: src/selfhost/pg-queue.ts:merge incremental RAG update and src/selfhost/sqlite-queue.ts:merge incremental RAG update reset `created_at` to `now`, so a pending incremental row that keeps absorbing new paths before `MAINTENANCE_ADMISSION_DRAIN_AGE_MS` never becomes old enough for either the drain escape or the 4h trickle
  • preserve the existing row's `created_at` in the UPDATE instead of assigning `$3`/`?`. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
The change correctly targets the maintenance-lane self-pinning path with a scoped drain escape and adds useful coalescing for RAG and notification jobs. The drain logic itself is mostly well-scoped, but the new RAG merge path resets `created_at` on every merge, which conflicts with the admission policy's own age-based guarantees and can reintroduce starvation for merged incremental work under sustained arrivals.

Blockers

  • src/selfhost/pg-queue.ts:merge incremental RAG update and src/selfhost/sqlite-queue.ts:merge incremental RAG update reset `created_at` to `now`, so a pending incremental row that keeps absorbing new paths before `MAINTENANCE_ADMISSION_DRAIN_AGE_MS` never becomes old enough for either the drain escape or the 4h trickle; preserve the existing row's `created_at` in the UPDATE instead of assigning `$3`/`?`.
Nits — 5 non-blocking
  • nit: src/selfhost/queue-common.ts:jobCoalesceMergedPayload should either assert same-repo inputs or document that callers must enforce the repo-prefix invariant, because the exported helper itself will merge paths across repos if called directly.
  • nit: src/queue/processors.ts:mapWithConcurrency is generic but only used for notification evaluation here; consider keeping it local-named for that use or moving to an existing shared concurrency helper if one exists.
  • In both queue implementations, change the incremental merge UPDATE to leave `created_at` untouched so the oldest absorbed work keeps its original admission clock.
  • Add a regression test that enqueues an old incremental RAG job, repeatedly merges a fresh incremental into it, then verifies the row remains drain/trickle-eligible based on the original `created_at`.
  • Add a queue-common unit test proving cross-repo payloads are not merged at the helper or caller boundary, depending on where you want that invariant to live.

Why this is blocked

  • src/selfhost/pg-queue.ts:merge incremental RAG update and src/selfhost/sqlite-queue.ts:merge incremental RAG update reset `created_at` to `now`, so a pending incremental row that keeps absorbing new paths before `MAINTENANCE_ADMISSION_DRAIN_AGE_MS` never becomes old enough for either the drain escape or the 4h trickle; preserve the existing row's `created_at` in the UPDATE instead of assigning `$3`/`?`.

CI checks failing

  • validate
  • validate-code
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ⚠️ 12/25 Preflight needs author follow-up before maintainer review.
Contributor workload ✅ 10/10 Author activity: 60 registered-repo PR(s), 51 merged, 454 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 60 PR(s), 454 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 60 PR(s), 454 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Address findings or add validation evidence.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@JSONbored
JSONbored force-pushed the fix/selfhost-maintenance-backpressure-deadlock branch from 00089fd to 7e7cbb9 Compare July 3, 2026 21:03
The maintenance-admission policy denied every claim with
maintenance_pending_high once the lane backed up past its threshold, with
no way for the backlog to shrink short of each job individually reaching
the full 4h trickle ceiling -- deferred because the lane was over
threshold, stuck over threshold because everything stayed deferred. Add a
second, shorter age escape (MAINTENANCE_ADMISSION_DRAIN_AGE_MS) scoped
only to that reason so the oldest jobs drain in a bounded trickle well
before the outer backstop, while live-priority, host-load, and GitHub
rate-limit admission are untouched. Also coalesce merge-triggered
rag-index-repo incrementals for the same repo (bounded to 100 paths) and
batch a webhook delivery's notification events into one notify-evaluate
job instead of one job per event, both of which were inflating the
maintenance lane unnecessarily.
@JSONbored
JSONbored force-pushed the fix/selfhost-maintenance-backpressure-deadlock branch from 7e7cbb9 to 66413b7 Compare July 3, 2026 21:50
@JSONbored JSONbored self-assigned this Jul 3, 2026
@JSONbored
JSONbored merged commit 287ae03 into main Jul 3, 2026
10 of 11 checks passed
@JSONbored
JSONbored deleted the fix/selfhost-maintenance-backpressure-deadlock branch July 3, 2026 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant