fix(service-queue,core): the idle job-queue poll backs off, on the one shared DispatchLoop - #18134
Conversation
…@objectstack/core The idle-backoff timer loop lived in service-messaging/src/dispatch-loop.ts, unexported. A third polling worker (service-queue's DbQueueAdapter) needs the same mechanism, and neither a second copy nor a queue -> messaging dependency is acceptable. Both services already depend on @objectstack/core, so the loop moves there and is exported from its index. Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude <noreply@anthropic.com>
…ing flat at 1s DbQueueAdapter.start() ran a flat setInterval, so a registered-but-idle queue issued one candidate read a second forever -- 3600 an hour per queue, every one an HTTP round trip on a remote driver. It now runs the shared DispatchLoop, and publish()/replay() wake it so local work keeps its base-interval latency. Measured on the engine boundary over one simulated idle hour, one queue: 3601 reads with the backoff disabled, 124 with it on. Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude <noreply@anthropic.com>
…ct and to core source The new idle-cost double is opened with assertEngineUpdateDispatch alongside assertEngineDeleteDispatch, and applies the caller's limit by presence so a `limit: 0` is not silently widened to the whole table. vitest now aliases @objectstack/core to its source, so these verdicts are about the loop in this checkout rather than about core's build state. Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 3 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin eff93004cf2f4bebfd801bc06ac55a8ec4e62f4f && git checkout eff93004cf2f4bebfd801bc06ac55a8ec4e62f4f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a90a9f26794e5a2c34c1eded83ba0e25087e4433 95070c921484eabf579456da80a0d6f7aef84446 && git checkout -B drift-repro a90a9f26794e5a2c34c1eded83ba0e25087e4433 && git merge --no-ff 95070c921484eabf579456da80a0d6f7aef84446
node scripts/docs-audit/affected-docs.mjs --json a90a9f26794e5a2c34c1eded83ba0e25087e4433
|
|
Served-tier: CONTRACT_REVIEW_TIER Contract reviewReviewed head: Implemented-by: ① Derived judgments (declaration
|
| symbol | .d.ts export list |
index.js |
index.cjs |
judgment |
|---|---|---|---|---|
DispatchLoop |
listed | reachable (function) |
reachable | correct — new published runtime symbol |
DEFAULT_MAX_IDLE_INTERVAL_MS |
listed | reachable (30000) |
reachable | correct — new published runtime symbol |
DispatchLoopOptions |
listed as type |
absent | absent | correct — type-only, by design |
ObjectKernel, LiteKernel (positive controls) |
listed | reachable | reachable | probe reads the published set |
DispatchLoopTimer, dispatchLoop (negative controls) |
absent | not reachable | not reachable | probe discriminates |
DbQueueAdapterOptions.maxIdleIntervalMs— a new key on an already-published payload:DbQueueAdapterOptionsisexport typefrompackages/services/service-queue/src/index.ts:8. Correctly named in the changeset. This alone is a clause-② floor hit onservice-queue, independent of core.DbQueueAdapter.wake()— a new PUBLIC method on the published class (index.ts:7).⚠️ The dev's re-derivation lists only the three core symbols and the option key;wake()is a real widening ofservice-queue's surface that the changeset prose does not name. Theminorgrade already covers it, andIQueueService(@objectstack/spec) is untouched, so this is an evidence gap in the write-up, not a wrong claim — recorded, not blocking.@objectstack/service-messaging: "nothing published moved" — verified true. Onorigin/mainitsexportsmap has only"."andsrc/index.tsnever re-exporteddispatch-loop.tsorDEFAULT_MAX_IDLE_INTERVAL_MS. Rebuilt at this head: the.d.tsexport list carries none ofDispatchLoop/DispatchLoopOptions/DEFAULT_MAX_IDLE_INTERVAL_MS; the runtime entry reportsfalsefor all three andtrueforNotificationDispatcher(control).dispatcher.ts:52still re-exports the constant from core, but that module is not a published entry.- Transitive:
packages/runtimeandplugin-hono-serverdoexport * from '@objectstack/core', so they gain the three names..changeset/config.jsonhasupdateInternalDependencies: "patch"and runtime's CHANGELOG shows the established pattern is the automaticUpdated dependenciesbump, not a hand-written entry — accepted as repo convention.
② Semver grading
| package | declared | what it did | verdict |
|---|---|---|---|
@objectstack/core |
minor | 3 new published exports (2 runtime, 1 type) | matches |
@objectstack/service-queue |
minor | new option key, new public wake(), new idle behaviour |
matches |
@objectstack/service-messaging |
patch | 2 import lines re-pointed (dispatcher.ts, http-dispatcher.ts), a test comment; published surface byte-identical in the export list; same loop code runs |
matches — genuinely only re-points imports |
③ Boundary flags
- Design choice (lift to core). Dependency claim verified in the manifests:
service-queue→{core, platform-objects, spec},service-messaging→{core, platform-objects, spec, types},core→{spec, types, zod}. Nopackage.jsonchanges in the diff, so zero new edges is literally true. No cycle or inversion: core imports nothing from either service (the one "service-messaging" hit inpackages/core/srcis a comment inindex.ts), anddispatch-loop.tshas no imports at all. The card's 「复用而非另写一套」 is honoured: git records a rename, not a copy, and messaging's own idle-backoff pins now run against core's SOURCE through its pre-existing alias (service-messaging/vitest.config.ts:32) — 460/460 green. Accepted. - Ledger.
scripts/engine-double-contract.pinned.jsondiffed row-by-row againstorigin/main: entries 800 → 802, pinned-sum 875 → 877; exactly two rows added (db-queue-idle-backoff.test.ts×delete/update, pinned 1 each), 0 dropped, 0 changed,$commentidentical.check:engine-double-contracton head reads the same 802. Accepted. - Three gates. Each run on this head with
--self-test:check:engine-double-contractexit 0,check:objectql-double-limitexit 0 (its own output: "baseline key set verified against a90a9f2: no files added" — the new double is JUDGED, not grandfathered),check:test-source-aliasexit 0. The three gate scripts and all their baselines (engine-double-contract.baseline.json,.seams.json,objectql-double-limit.baseline.json) are byte-identical to main. The alias is the anchored array form the gate prescribes. Genuinely fixed, not routed around, weakened or baselined. - Docs. None of the three flagged pages names
DispatchLoop,dispatch-loop, or a package location for the loop (nothing incontent/docsoutside releases does).jobs.mdx:87,95intervalMsis the job-schedule trigger field;lifecycle.mdx:696,706isPluginHealthMonitor;webhooks.mdx:381-398,624describeHttpDispatcher"in@objectstack/service-messaging" ticking fromintervalMsup tomaxIdleIntervalMs—HttpDispatcherstill lives there and still exposes those options. The drift rows are homonym hits. No page is inaccurate; no edit owed. (service-queue/README.md:62pollIntervalMs: 1000 // worker poll cadenceremains true as the base cadence.) - Behaviour change (first tick immediately). Verified in
DispatchLoop.start(). The one production site,QueueServicePlugin, constructs withautoStartdefaulting to true, so the immediate tick now runs inside the constructor before anysubscribe()—pollOnce()returns 0 without touching the engine when no handler is registered, so that tick issues no read. All 15 test-side constructions passautoStart: false(15/15 grep). Accepted. ⓘ Observation for the seat, not a contract error:subscribe()does notwake(), so a queue registered after the loop has backed off sees rows already persisted insys_job_queueup tomaxIdleIntervalMslate rather than ≤ 1 s; this is inside the documented latency class ("a row another node wrote") and every locally published or replayed row wakes the loop.
Last-defence checks
- No test skipped, disabled, quarantined or weakened. Diff grep for
.skip/.only/.todo/xit/ removedit(lines: none. Test files touched: one added, one comment-only line inhttp-dispatcher-idle-backoff.test.ts. - The control exists and discriminates.
db-queue-idle-backoff.test.tsruns the NEGATIVE CONTROL leg first (maxIdleIntervalMs: BASE, the documented "no backoff" setting) on the same counting engine and fake clock, pinned at 3601. I ablated three ways, each restored to the HEAD blob hash with a clean tree: (a) backoff disabled incore/src/dispatch-loop.ts→ the 124 leg fails withexpected 3601 to be 124(1 failed / 7 passed) — reproduces the before number and proves the alias reaches core source with no rebuild; (b) dead loop (schedule()never arms the timer) → BOTH legs fail, the control readingexpected 1 to be 3601— so 124 cannot be a stopped loop; (c)publish()wake seam removed → the wake leg and the vacuity-trap leg fail (2 failed / 6 passed). - Wake seam keeps the ceiling off the latency path.
publish()wakes after the insert only whenDate.parse(scheduledFor) <= now(db-queue-adapter.ts:377);replay()wakes unconditionally after its update (:454);DispatchLoop.wake()resets the exponent and ticks at once or coalesces into one follow-up;wake()is a no-op once stopped (pinned). The wake leg asserts pickup ≤BASE. - Suites on this head, local: service-queue 78/78, service-messaging 460/460, core 1364/1364 (the dev reported 1316 for core — a count delta I could not attribute; all green either way),
typecheckOK for all three,eslint --no-inline-configclean on the 7 changed source files. No model identifier in the diff or commit messages.node scripts/pm/check-clause2-carriers.mjs --pair 18134exit 0 before this record.
Verdict: PASS
Generated by Claude Code
|
Record of governing verdict: comment 5659878791 on this PR, judging head
⭐ Three things the review measured rather than accepted, worth reading:
Also verified independently: the ledger diffed row-by-row ( ⓘ Non-blocking observation carried forward for whoever next touches this area: Generated by Claude Code |
Fixes #17612
Scope item 3 (空闲轮询) — the last open item on this card. Items 1, 2 and 4 landed in #18105 and are re-verified untouched on
origin/mainhere.Clause-②: yes
What the card asked, and what was measured first
The card says item 3 should reuse #17610's backoff rather than write a second one, and the dispatching seat pre-ruled out the two cheap shapes: option D (a second backoff inside
service-queue) by the card's own text, option B (exporting the loop fromservice-messagingand depending on it fromservice-queue) as an inverted dependency direction. The remaining direction is A — lift the loop somewhere both can depend on.The escape hatch was to stop and report if A turned out disproportionate to the remaining benefit, since items 1/2 already removed the per-tick cost and only the statement count while idle was left. Both sides were measured before any code moved:
setInterval)DispatchLoop, 1 s base / 30 s ceiling)That is a 29x reduction, linear in the number of registered queues, and on a remote driver every one of those reads is an HTTP round trip. The cost of A turned out to be small:
service-queueandservice-messagingalready both depend on@objectstack/core, so the lift adds zero new dependency edges — it is one file move, one export, and two import re-points. A is not disproportionate to a 29x cut, so A was built rather than reported as a stop.What changed
packages/services/service-messaging/src/dispatch-loop.tsmoves topackages/core/src/dispatch-loop.tsand is exported from core's index. It is a timing primitive owned by neither the messaging domain nor the queue domain; core is the package all three consumers already depend on.service-messagingpublishes only itsindex, which never carried the loop, so nothing published moved — its two dispatchers just import from@objectstack/corenow.DbQueueAdapter.start()runs that loop instead of a baresetInterval. The loop also subsumes the oldrunningre-entrancy flag, which was doing tick coalescing by hand.DbQueueAdapterOptions.maxIdleIntervalMs(default 30 s). At or belowpollIntervalMsit disables the backoff and restores the flat poll exactly.publish()andreplay()wake the loop, so the ceiling is never on the latency path for work this process was told about. A deferredpublish()deliberately does not wake it — that tick would claim nothing and would throw the backoff away for free.Evidence
Tests:
service-queue78 passed (70 before + 8 new legs),service-messaging460,core1316, and the downstream consumersplugin-email468 andplugin-audit334 — all on the final head.typecheckgreen for all three edited packages, and it demonstrably reaches*.test.ts(it caught a real error there during this work).The flat-poll leg is a negative control, not decoration. An upper bound on statements is satisfied by a worker that stopped ticking altogether — the one failure a low number cannot distinguish. So the same clock, engine and counter are run with the backoff disabled and pinned at 3601; that is what makes 124 a reading about the backoff rather than about a dead loop.
Reverse verification — two legs, each mutated on disk (anchor count 1 before, injected marker 1 / removed anchor 0 after), run, restored with
git checkout HEAD --, and proved byte-identical by blob hash withgit diff HEADempty:core/src/dispatch-loop.tsexpected 3601 to be 124DbQueueAdapter.publish()The first ablation reads 3601 — independently reproducing the before number from a second direction. It also proves the new vitest alias really reaches core's source: no rebuild happened anywhere in that leg, and the verdict still moved.
Clause-② re-derivation, from the delivered diff. Reachability was taken as the two discriminating reads, not the word
exportand not a bundle grep:.d.tsexport list — core'sdist/index.d.ts(the fileexports['.'].typesnames) carriesDispatchLoop,type DispatchLoopOptionsandDEFAULT_MAX_IDLE_INTERVAL_MSinside its singleexport { ... }statement.in await import(entry)through the package entry (resolved topackages/core/dist/index.jsvia the exports map), with controls both ways:DispatchLoopDEFAULT_MAX_IDLE_INTERVAL_MSDispatchLoopOptions.d.tsexport listObjectKernel,LiteKernelDispatchLoopTimerdispatchLoopfilesis["dist", ...], so that entry ships. A new symbol is reachable from a shared package's published entry ⇒ Clause-② is yes, and the changeset grades@objectstack/coreminor accordingly (neverpatch).Gate denominator, reconciled against
scripts/pm/dispatch-gates.mjs --ranon the final head95070c921: 73 derived, 70 run, 3 NOT MEASURED, 0 UNRUN. The three arecheck:dual-build-cjs-loads,check:i18nandcheck:type-check-debt, each of which exited 3 — the code those gates use for "prerequisite not met, nothing was measured", all three wanting a whole-repopnpm build. They are declared NOT MEASURED rather than read as green; CI builds fresh and runs them.Three gates went red against my own diff during this work and were fixed, not routed around:
check:engine-double-contract(the new double'supdate()now opens withassertEngineUpdateDispatch),check:objectql-double-limit(the double applies the caller'slimitby presence, solimit: 0is not silently widened to the whole table), andcheck:test-source-alias(the package now aliases@objectstack/coreto source).scripts/engine-double-contract.pinned.jsongrew by 2 rows — onedelete, oneupdate, both for the new test file — from 800 entries / 875 pinned-sum onmainto 802 / 877.Acceptance notes
new DbQueueAdapterin the repo was enumerated, and every test site passesautoStart: falseand drivespollOnce()by hand, so the timer change reaches none of them. The one production construction isQueueServicePlugin, whosedestroy()already awaitsstop().start()now runs its first tick immediately, where the oldsetIntervalwaited one interval. That is strictly lower latency and no test depended on the delay.db-queue-adapter.test.tsandjob-queue-retention.test.tsapplylimitby truthiness (if (opts.limit)), the same shapecheck:objectql-double-limitrejected in the new file. They are inside that gate's existing baseline, so it already knows; a fixture that asked forlimit: 0would read the whole table.Generated by Claude Code