Skip to content

fix(service-messaging): reap once per dispatcher tick, back off while idle, wake on emit - #17622

Merged
hotlong merged 5 commits into
mainfrom
claude/issue-17610-dispatcher-idle-cost
Sep 11, 2026
Merged

fix(service-messaging): reap once per dispatcher tick, back off while idle, wake on emit#17622
hotlong merged 5 commits into
mainfrom
claude/issue-17610-dispatcher-idle-cost

Conversation

@hotlong

@hotlong hotlong commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Closes #17610

What changed

NotificationDispatcher stops paying for an empty outbox on every tick.

  1. The reap runs once per tick, decoupled from claiming. INotificationOutbox gains an optional reap(opts), and ClaimOptions an optional skipReap. The dispatcher runs the reap once, at the start of the tick and outside any partition lock, then claims every partition with skipReap: true. Direct claim() / claimDigest() callers are unchanged: without skipReap they reap exactly as before.
  2. Idle backoff. Each loop tick that claims nothing doubles the delay to the next one, from intervalMs (500 ms) up to maxIdleIntervalMs (new, default 30 s, plugin option dispatchMaxIdleIntervalMs). A tick that claims anything snaps back to intervalMs, and a failed tick counts as idle, so a broken store is not hammered. The loop is now a self-rescheduling unref'd setTimeout, measured from tick start, with the same never-two-ticks-at-once guard.
  3. Wake on enqueue. MessagingService.setOutbox(outbox, { onEnqueued }) fires once per emit() that enqueued at least one delivery, which is the one fan-out site that writes delivery rows (enqueueDeliveries). MessagingServicePlugin points it at the new NotificationDispatcher.wake(). A wake ticks immediately; a wake during a running tick queues exactly one follow-up tick for when it settles, and any number of wakes in that window collapse into one.

Premise check against origin/main (3ef96b4)

The card's arithmetic is right. I measured it with a throwaway probe on a real ObjectQL + SqlDriver (better-sqlite3), counting both the IDataEngine calls and the driver's own knex query events, over 5 idle ticks with 8 partitions:

per idle tick before (3ef96b4) after (this PR)
reap UPDATE (in_flight rows whose claimed_at is older than the TTL) 16 1
claim SELECT id … digest_key IS NULL AND partition_key = ? 8 8
digest SELECT id … digest_key IS NOT NULL AND partition_key = ? 8 8
total statements (engine calls = knex queries) 32 17

Loop cadence while idle, with default options and ticks treated as instant: 1,201 ticks / 38,432 statements over 10 minutes before, 24 ticks / 408 statements after. Real ticks take time on remote Turso (the card measured about 1.1 s at 32 statements), which only lowers both numbers. In the steady idle state this is 17 statements every 30 s, about 0.57 statements/s, against the measured ~28/s.

No objection to the card's premise. The reap has no correctness reason to run per partition. Its predicate names no partition, and the partition lock never covered it: a reap under partition p's lock was already rewriting rows in every other partition.

Why reaping once per tick does not stretch claim-TTL recovery

  • The reap runs before the tick's claims. So every claim in the tick sees every row that had expired when the tick began, which is what the per-claim reap gave. The only difference: a claim that expires during the tick is returned by the next tick's reap. So a crashed node's in_flight rows are still recovered within one tick of claimTtlMs passing.
  • The TTL keeps its meaning. It is the floor below which a claim is never re-taken; this PR changes nothing there, and a test pins it (see A1 below).
  • No lock is needed. The reap moves only rows already past their timeout, claim() only takes pending rows, and an ack whose claim was reaped is refused by its compare-and-set (INotificationOutbox.ack() carries no nodeId, so its compare-and-set can prove a claim exists but not whose #11859).
  • While idle, "one tick" is one backed-off interval. Recovery is then bounded by claimTtlMs + maxIdleIntervalMs, which with defaults is 5 s + 30 s. The fixed loop gave claimTtlMs + 500 ms. That is the stated trade (below).

Latency bound

  • Notifications emitted in the process running the dispatcher: no added latency. emit() wakes the dispatcher and the new rows go out on the tick that wake starts.
  • Work nobody announces, while idle: noticed within one backed-off interval, at most maxIdleIntervalMs (30 s by default). This covers a deferred delivery coming due (the retry schedule's later steps, quiet hours, a digest window), a row enqueued by a process that does not run this dispatcher, and a crashed node's expired claim. The first retry (~1 s) stays on time: the failed attempt claimed a row and reset the backoff, and the next ticks come 0.5 s and then 1 s later.
  • dispatchMaxIdleIntervalMs: dispatchIntervalMs restores the fixed interval.

Compatibility

reap() is optional on INotificationOutbox, following the capability-probe pattern DispatchLockHandle.renew? / isHeld? already use in this file. A custom outbox written before it keeps working: the dispatcher probes for reap, and when it is absent, claims are not told to skip, so each claim reaps as before. That is correct, just at the old per-claim cost. A test pins this fallback (see A4 below). Both built-in stores implement reap(). The changeset is minor (additive API: wake(), maxIdleIntervalMs, dispatchMaxIdleIntervalMs, reap?, ReapOptions, setOutbox's second argument), and nothing is declared breaking.

I first made reap a required member. check-adr-0087-registration rightly refused that shape: a changeset carrying a FROM/TO prescription cannot claim no-migration-prescription, and no ledger category fits a runtime TS interface member. Making the member optional removes the break instead of arguing with the gate. Its green reading is below.

Acceptance, item by item

  1. Reap decoupled from claiming. src/dispatcher-idle-cost.integration.test.ts, real ObjectQL + SqlDriver, 8 partitions, 10 idle ticks: exactly 10 reap UPDATEs, and total engine calls at most 10 × (1 + 2 × 8). The legs recovers a crashed node's expired claim and delivers it within ONE tick and leaves a claim that has NOT expired alone pin recovery and the TTL floor. The leg an outbox without reap() keeps working pins the compatibility fallback. delivery-claim-tenant-audit.integration.test.ts gains a reap() leg: it reaps across organizations with no tenant-audit finding, with the file's positive control.
  2. Idle backoff and wake. src/dispatcher-idle-backoff.test.ts runs on fake timers. It checks the exact gap sequence (1 s, 2 s, 4 s, 8 s, 16 s, 30 s, then 30 s), the 30 s default, that a ceiling at or below intervalMs disables the backoff, the one-interval bound for an unannounced row plus the snap-back to 500 ms, that wake() ticks at the same fake instant, that wakes during a running tick collapse to one follow-up, that stop() cancels the timer, and that MessagingService.emit() wakes a wired dispatcher while an emit that enqueues nothing does not. src/plugin-enqueue-wakes-dispatcher.test.ts boots the composed MessagingServicePlugin on a real kernel and engine with dispatchIntervalMs = 60 s, so no timer tick can happen during the test. A row written straight into the table (no emit, no wake) stays pending after a real 500 ms wait. That is the negative control. Then emit() delivers both rows.
  3. Count test against regression, plus "enqueued after idle is claimed next tick". The first leg of dispatcher-idle-cost.integration.test.ts is the count test. Its leg rows enqueued after an idle stretch go out on the very next tick uses 16 rows hashed across several of the 8 partitions plus a two-row digest window: all delivered in one tick, and the digest sent as one message.
  4. Semantics unchanged. The full @objectstack/service-messaging suite is green, including the INotificationOutbox has no cancellation, and ack() on an unclaimed pending row silently succeeds in both implementations #11453 / INotificationOutbox.ack() carries no nodeId, so its compare-and-set can prove a claim exists but not whose #11859 ack and claim-ownership suites, digest, and the flaky: a vitest worker teardown race (EnvironmentTeardownError: Closing rpc while onUserConsoleLog was pending) fails app-showcase with 334/334 tests passing #9371 shutdown test. service-automation's three notify integration files, which drive the real NotificationDispatcher through the rebuilt dist, are green too.

Reverse verification (ablations, each restored from HEAD and proven by blob hash plus an empty git diff HEAD)

The ablation run was on HEAD f368fd0aa. Every mutation was confirmed on disk by occurrence counts before its run.

ablation expected red observed
A1: dispatcher never takes the once-per-tick reap, so claims reap per claim (the pre-fix cost) count leg 1 failed / 4 passed, only the count leg
A4: claims told skipReap: true even when the outbox has no reap() legacy-outbox fallback leg 1 failed / 4 passed, only that leg
A2: emit() no longer calls onEnqueued both wake-on-emit legs (fake-timer + composed plugin) 2 failed / 8 passed, exactly those two
A3: backoff disabled (nextIntervalMs always intervalMs) backoff legs 2 failed / 7 passed (gap sequence, 30 s default)
restoration leg on HEAD all green 15 / 15 passed

A3 leaves a row nobody announced waits at most one backed-off interval green, as expected: that leg pins an upper bound, and a fixed 500 ms loop meets it trivially.

Commands and results

  • pnpm --filter @objectstack/service-messaging exec vitest run --maxWorkers=2: 37 files, 389 tests passed (HEAD f368fd0aa)
  • pnpm --filter @objectstack/service-messaging typecheck: exit 0. The package tsconfig includes all of src, so the new tests are type-checked.
  • pnpm exec turbo run build --filter="@objectstack/service-automation^..." --concurrency=2, then vitest run on service-automation/src/builtin/notify-{delivery-outcome,organization-stamp,zero-delivery-visibility}.integration.test.ts: 3 files, 18 tests passed
  • eslint (--no-inline-config --format json) over the 11 touched .ts files: 11 files, 0 errors, 0 warnings. This narrowing is sound: eslint.config.mjs never enables type-aware linting (no parserOptions.project), so this diff cannot move the verdict on any untouched file.
  • node scripts/check-adr-0087-registration.mjs --base origin/main: ✓ no declared-breaking changeset. check-changeset-no-major ✓. check-empty-changeset ✓. All three --self-tests ✓.
  • Derived gate families (node scripts/pm/dispatch-gates.mjs --commands): all 94 run with exit codes recorded; --ran reconcile reads 94 derived, 91 run, 3 NOT-MEASURED, 0 UNRUN.
    • check-tenant-audit-census went red on this diff: the reap consolidation removed one write call site (223 → 222). ff0ba83a2 re-measures the census page and its prose counts; the gate and its --self-test both exit 0 at ff0ba83a2.
    • NOT MEASURED, prerequisite refusals: check:i18n, check:type-check-debt and check:dual-build-cjs-loads (exit 3; they need the CLI closure or a full-repo build, which was out of bounds for this seat). check:skill-examples exits 1 but with its own refusal text, packages/client-react/dist holds no .d.ts; this diff touches no marked example.
    • check:merge-driver exits 1, failing the same two check-regen-pending --self-test cases on a clean origin/main 0918c4411 checkout on this host. It is not caused by this diff, which touches no merge-driver or regen surface.
    • The first batch ran at f368fd0aa; the 31 families the census files added ran at ff0ba83a2.

Declared: the heavy commands above ran through scripts/pm/os-verify-lock.sh, which reported UNLOCKED (declared). This host has no usable flock, so nothing was serialized.

Acceptance notes (out of scope, not changed here)


Generated by Claude Code

hotlong and others added 4 commits September 11, 2026 11:58
…it and once-per-tick reap recovery

Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569
Co-authored-by: Claude <noreply@anthropic.com>
… dispatcher probes for

A store without reap() keeps working: its claims keep reaping as before.

Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-messaging, touching 28 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-messaging/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/webhooks.mdx (via claimTtlMs (symbol, a field of interface ReapOptions), in_flight (literal, a string literal in claim; a string literal in claimDigest; a string literal in reapExpired))

3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v14.mdx (via MessagingServicePlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-2.mdx (via MessagingService (symbol, a top-level class))
  • content/docs/releases/v17/17-3.mdx (via INotificationOutbox (symbol, a top-level interface))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-messaging/src/index.ts) — pages documenting those are invisible to this run
  • 10 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4packageMentionDocs.

Which tree this was computed on

This run read content/docs from ce6d45da6b6bcd01cdef14bd0317831f6b8b693b — the merge of head ff0ba83a2555402db3214ca7082b0824acffbde1 into base 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ce6d45da6b6bcd01cdef14bd0317831f6b8b693b && git checkout ce6d45da6b6bcd01cdef14bd0317831f6b8b693b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4 ff0ba83a2555402db3214ca7082b0824acffbde1 && git checkout -B drift-repro 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4 && git merge --no-ff ff0ba83a2555402db3214ca7082b0824acffbde1

node scripts/docs-audit/affected-docs.mjs --json 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… (223 -> 222)

SqlNotificationOutbox's two inline reap UPDATEs became one reapExpired()
helper, so the census counts one fewer write call site.

Claude-Session: https://claude.ai/code/session_c5c0ce54-bb9c-478c-9e5b-cf44b80d4569
Co-authored-by: Claude <noreply@anthropic.com>
@hotlong

hotlong commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

PM 复核(maintainer direct dispatch,Session c5c0ce54…

逐项核对的是 PR head ff0ba83a2实际代码

读数 结论
改动面 14 个文件;packages/spec/src/** 0;受治理面(AGENTS.md / CLAUDE.md / .claude/** / docs/adr/**0 Clause-②: no 成立
两份审计普查文档 content/docs/permissions/tenant-audit-census.mdxdocs/audits/…counts.md:reap 合并使写入调用点 223 → 222,由 check-tenant-audit-census 要求重测 合理,非受治理面
changeset @objectstack/service-messaging: minorINotificationOutbox.reap()ClaimOptions.skipReap 均为可选 非破坏性
reap 移到分区锁外 runTick 开头每跳一次;reap 是条件更新 status='in_flight' AND claimed_at < now - claimTtlMs,谓词不含分区 安全——锁内跑时本来就在改写其他分区的行,ack 对已被回收的认领匹配不到(#11859
旧 outbox 兼容 if (outbox.reap) 才置 reapedForTick;未实现 reap() 的存储 skipReap=false,每次领取照旧自 reap 兼容
退避计时器可被停掉 setTimeout 链;stop()running=false、清 tickRequestedclearTimeout、等待进行中的一跳 满足——这正是 cloud 侧「归档驱逐 kernel」修复所依赖的行为
唤醒 emit()setOutbox(outbox, { onEnqueued })wake(),重置空闲计数并立即排一跳 满足验收 2

开放问题的裁定:空闲退避上限默认 30 秒(选项 A)

  • 本进程内的 emit 会立即唤醒,所以上限只推迟无人通知的工作:延迟重试、静默时段、摘要、其他进程写入的行、崩溃节点的认领回收
  • 这几类对 30 秒不敏感,而空闲往返从每秒约 28 条降到约 0.57 条(约 1/50)
  • 运维可按插件用 dispatchMaxIdleIntervalMs 调低
  • 崩溃节点认领的回收上界相应变为 claimTtlMs + 30s,已在 PR 正文写明——接受

席位带回的范围外发现

下一步:CI 全部结束且无失败 → 转 ready → 入合并队列。有红按日志回派同一席位。

@hotlong

hotlong commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

@hotlong
hotlong marked this pull request as ready for review September 11, 2026 04:47
@hotlong
hotlong enabled auto-merge September 11, 2026 04:47
@hotlong
hotlong added this pull request to the merge queue Sep 11, 2026
Merged via the queue into main with commit 690f083 Sep 11, 2026
42 checks passed
@hotlong
hotlong deleted the claude/issue-17610-dispatcher-idle-cost branch September 11, 2026 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

1 participant