Skip to content

fix(service-messaging): HttpDispatcher reaps once per tick, backs off while idle, wakes on enqueueHttp - #17632

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-17623-http-dispatcher-idle-cost
Sep 11, 2026
Merged

fix(service-messaging): HttpDispatcher reaps once per tick, backs off while idle, wakes on enqueueHttp#17632
hotlong merged 2 commits into
mainfrom
claude/issue-17623-http-dispatcher-idle-cost

Conversation

@hotlong

@hotlong hotlong commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Closes #17623

HttpDispatcher / SqlHttpOutbox get the pattern PR #17622 landed for NotificationDispatcher — no new design: an optional reap() on the outbox contract with skipReap on the claim, an idle exponential backoff on a setTimeout chain that stop() clears, and wake-on-enqueue (setHttpOutbox(outbox, { onEnqueued })wake()). The loop itself moves out of NotificationDispatcher into dispatch-loop.ts, shared by both dispatchers (reasoning under item 5).

Measurement — base 690f083f8 vs this PR

Real ObjectQL + SqlDriver (better-sqlite3 :memory:), 8 partitions, empty sys_http_delivery, 10 manually driven ticks.

reading base 690f083f8 this PR
engine calls on sys_http_delivery, 10 ticks update 80 · find 80 · other 0 update 10 · find 80 · other 0
SQL statements on the knex query event, 10 ticks 160 (80 UPDATE · 80 SELECT) 90 (10 UPDATE · 80 SELECT)
statements per idle tick 16 9 = 1 + partitionCount
ticks in 10 idle minutes (fake timers, 500 ms base) 1,201 24

The card's 16-per-tick figure reproduces exactly. The SQL-statement row is a one-off measurement (a throwaway test hooking driver.knex.on('query'), deleted before commit). The committed test counts at the engine boundary, as #17622's does; on this harness one engine call is one statement.

Acceptance, item by item

1. Bounded statement count on an empty outbox, red on base first. src/http-dispatcher-idle-cost.integration.test.ts pins { update: TICKS, find: TICKS × PARTITIONS, other: 0 } exactly and the total at ≤ TICKS × (1 + PARTITIONS). Run on base (the file committed alone as 2235f6973, every non-test file byte-identical to 690f083f8):

AssertionError: expected { update: 80, find: 80, other: +0 } to deeply equal { update: 10, find: 80, other: +0 }
Tests  1 failed | 4 passed (5)

On this PR: 5/5. The bound cannot pass vacuously — the same harness proves rows enqueued after an idle stretch go out on the very next tick in every partition, a crashed node's expired claim is recovered and delivered within ONE tick, and a claim still inside its TTL is left alone.

2. Idle backoff and wake-on-enqueue, each tested; latency bound stated. src/http-dispatcher-idle-backoff.test.ts (fake timers, 11 legs): doubling 1 s → 30 s cap (24 ticks in 10 idle minutes); 30 s default; ceiling at or below intervalMs disables it; an unannounced row waits at most one ceiling and its tick resets the backoff; a retry coming due is attempted less than min(delay + intervalMs, maxIdleIntervalMs) late; wake() ticks immediately; wakes during a running tick collapse into one follow-up; stop() cancels the pending tick and a later wake() is a no-op; enqueueHttp() wakes; an enqueueHttp() that PARKS an undeliverable row (dead on arrival) does not; redeliverHttp() wakes. On base 7 of 11 are red, the doubling leg with expected [ 500, 500, 500, 500, 500, 500 ] to deeply equal [ 1000, 2000, 4000, 8000, 16000, 30000 ].

The composed-plugin leg added to src/plugin-enqueue-wakes-dispatcher.test.ts boots the real plugin with dispatchIntervalMs 60 s and a stubbed globalThis.fetch, proves a row written straight into the table stays pending (negative control on the same boot), then shows enqueueHttp() POSTs it and the unannounced row at once.

The wake sits exactly where enqueueHttp writes the delivery row — after outbox.enqueue() resolves — and also after redeliverHttp(), the other service write that leaves a row pending; without it an operator's redeliver would wait up to the ceiling.

Latency bound. A delivery enqueued or redelivered in the dispatcher's own process goes out on the tick wake() starts — sooner than the old fixed 500 ms. Work nobody announces waits at most one backed-off interval: a row written by another process, at most maxIdleIntervalMs (30 s); a retry, less than min(its delay + intervalMs, maxIdleIntervalMs) late, because the backoff restarts from intervalMs at the attempt that scheduled it (a ~1 s retry is under ~1.7 s late); a crashed node's claim, recovered within claimTtlMs + maxIdleIntervalMs (~35 s at defaults, was ~5.5 s).

3. A legacy outbox is unchanged; partition lock, at-least-once and claim-TTL semantics are unchanged. The dispatcher probes outbox.reap; when it is absent no claim is told to skip, so each claim reaps itself as before (leg "an outbox without reap() keeps working": 8 claims seen, none with skipReap, the expired claim still delivered in the same tick). Direct claim() callers without skipReap reap exactly as before. Lock keys, batch processing and ack() are untouched. A new reap() leg in delivery-claim-tenant-audit.integration.test.ts recovers both organizations' rows with no [tenant-audit] finding, and its positive control fires. Full package suite green (below).

4. Default and naming. The ceiling default is the notification side's own DEFAULT_MAX_IDLE_INTERVAL_MS (30 s) — the same constant. HttpDispatcherOptions.maxIdleIntervalMs has the same name as on NotificationDispatcherOptions. No new plugin option: dispatchMaxIdleIntervalMs now sets the ceiling for both dispatchers, the way dispatchIntervalMs and partitionCount already govern both. It is unreleased (.changeset/17610-notification-dispatcher-idle-cost.md is still pending; npm is at 17.4.0), so widening it costs no consumer anything, and a per-dispatcher split could still be added later without breaking anyone.

5. Shared helper — extracted. The loop is about 90 lines of order-sensitive concurrency: delays measured from tick start, wakes collapsing into one follow-up, stop() clearing a pending request, a rejected tick counted as idle. Two copies would have to be kept in step by hand, and the next fix would land in one of them. The extraction is cheap to prove safe because #17622 pinned that behaviour precisely: dispatcher-idle-backoff.test.ts and dispatcher-idle-cost.integration.test.ts pass unmodified on the extracted loop. NotificationDispatcher's public surface (start / stop / wake / tick, and DEFAULT_MAX_IDLE_INTERVAL_MS still exported from ./dispatcher.js) is unchanged, and DispatchLoop is not exported from the package index. The cost is touching dispatcher.ts days after it landed; no open PR touches it.

Ablations — one-off, trap-restored

Restores are proven by HEAD blob hash plus an empty git diff HEAD. The tests import the source by relative path, so no dist rebuild is in the resolution path.

  • A — the plugin no longer wires the HTTP wake (service.setHttpOutbox(httpOutbox); on disk: removed text 0, injected text 1). plugin-enqueue-wakes-dispatcher.test.ts: the HTTP leg fails Error: condition not met within 5000 ms, the notification leg stays green (1 failed, 1 passed).
  • BskipReap forced even when the outbox has no reap() (let reapedForTick = true). http-dispatcher-idle-cost.integration.test.ts: the legacy-outbox leg fails (expected false to be true), the other 4 stay green.

Contract

No packages/spec/src/** in the diff. The contract additions are on service-messaging's own IHttpOutbox and all optional (reap?, skipReap?), so no implementer or caller must change; the changeset is minor and not declared breaking.

Verification — head 3c90f49ad

  • pnpm --filter @objectstack/service-messaging test → 39 files, 407 tests passed
  • pnpm --filter @objectstack/service-messaging typecheck → exit 0; tsc --listFiles includes all four touched test files and dispatch-loop.ts
  • pnpm --filter @objectstack/service-messaging build → exit 0, check-dts-emitted: 2/2
  • dependency closure turbo run build --filter="@objectstack/service-messaging^..." --concurrency=2 → 15/15 (13 cached)
  • ⚠️ scripts/pm/os-verify-lock.sh ran in declared UNLOCKED mode on this host (no usable flock), so nothing was serialized.
  • derived gate families: node scripts/pm/dispatch-gates.mjs --commands → 92 commands, every one run on this head; node scripts/pm/dispatch-gates.mjs --ran92 derived famil(ies) accounted for — 90 run, 2 NOT-MEASURED, 0 unrun. All 90 measured exit 0, including check-tenant-audit-census: OK -- 222 write call sites certified (the count fix(service-messaging): reap once per dispatcher tick, back off while idle, wake on emit #17622 left, so no census rewrite was due), check-adr-0087-registration: this PR adds no declared-breaking changeset, check-changeset-no-major, check-empty-changeset, check:nul-bytes and check:cross-package-test-inputs.
    • check:i18n and check:skill-examples first refused on unbuilt prerequisites (the workspace CLI; packages/client-react/dist with no .d.ts). After the targeted closure the gates name (turbo run build over the CLI, its 9 extract packages and @objectstack/client-react...: 58 tasks, 18 cached) both re-ran green: check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys) and ✅ 258 prose examples type-check across 3 surface(s).
    • NOT MEASURED: check:dual-build-cjs-loads — exit 3, PREREQUISITE NOT MET: it reads the dist/ of every package (hono, account, setup, studio, client, cloud-connection, …), a workspace-wide build this seat does not run. CI measures it.
    • NOT MEASURED: check:type-check-debt — exit 3, PREREQUISITE NOT MET: its prescribed prerequisite is turbo run build --filter='./packages/*' --filter='./packages/*/*', the whole workspace. CI measures it.
  • CI on this PR was still pending when this body was written; not waited on.

Docs

content/docs/automation/webhooks.mdx: §4.4 describes the once-per-tick reap, the idle backoff and the wake; the §12 crash-recovery row states the claimTtlMs + maxIdleIntervalMs bound instead of implying ~5 s.

Acceptance notes


Generated by Claude Code

@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation tests tooling labels Sep 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-messaging, touching 37 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.

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

  • content/docs/api/environment-routing.mdx (via HttpDispatcher (symbol, a top-level class))
  • content/docs/automation/jobs.mdx (via intervalMs (symbol, a field of class DispatchLoop; a field of interface DispatchLoopOptions))
  • content/docs/automation/webhooks.mdx (via HttpDispatcher (symbol, a top-level class), HttpDispatcherOptions (symbol, a top-level interface), MessagingServicePlugin (symbol, a top-level class), claimTtlMs (symbol, a field of interface HttpReapOptions), intervalMs (symbol, a field of class DispatchLoop; a field of interface DispatchLoopOptions), maxIdleIntervalMs (symbol, a field of class DispatchLoop; a field of interface DispatchLoopOptions; a field of interface HttpDispatcherOptions), in_flight (literal, a string literal in claim; a string literal in reapExpired), maxIdleIntervalMs (literal, a string literal in HttpDispatcher; a string literal in NotificationDispatcher), onAttempt (literal, a string literal in HttpDispatcher; a string literal in NotificationDispatcher))
  • content/docs/kernel/cluster.mdx (via HttpDispatcher (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via HttpDispatcher (symbol, a top-level class))

2 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 IHttpOutbox (symbol, a top-level interface), MessagingService (symbol, a top-level class), SqlHttpOutbox (symbol, a top-level class))

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
  • 11 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 690f083f83d4b864a949c0707f79490116bbae23packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9512003d64003ec68d55ede30a1970a8da77a730 — the merge of head 3c90f49adfc83499ee922cee9ff2ef26e2fac63c into base 690f083f83d4b864a949c0707f79490116bbae23, 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 9512003d64003ec68d55ede30a1970a8da77a730 && git checkout 9512003d64003ec68d55ede30a1970a8da77a730
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 690f083f83d4b864a949c0707f79490116bbae23 3c90f49adfc83499ee922cee9ff2ef26e2fac63c && git checkout -B drift-repro 690f083f83d4b864a949c0707f79490116bbae23 && git merge --no-ff 3c90f49adfc83499ee922cee9ff2ef26e2fac63c

node scripts/docs-audit/affected-docs.mjs --json 690f083f83d4b864a949c0707f79490116bbae23

⚠️ 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 690f083f83d4b864a949c0707f79490116bbae23 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@hotlong

hotlong commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

PM 复核(maintainer direct dispatch,Session c5c0ce54…

对照 PR head 3c90f49ad 的实际代码逐项核对:

读数 结论
改动面 15 个文件;packages/spec/src/** 0;受治理面 0 Clause-②: no 成立
changeset @objectstack/service-messaging: minorIHttpOutbox.reap()HttpClaimOptions.skipReap 均为可选 非破坏性
文档 content/docs/automation/webhooks.mdx 写明延迟上界 非受治理面,合理
抽出的 DispatchLoop stop()running=false、清 tickRequestedclearTimeout、等待进行中的一跳;进行中的多次唤醒经 tickRequested 合并为一次后续跳 安全,停得干净
NotificationDispatcher 公开面 start / stop / wake / tick 仍在;DEFAULT_MAX_IDLE_INTERVAL_MS 仍从 ./dispatcher.js 再导出;DispatchLoop 从包入口导出 不破坏既有消费者;#17622 的两套测试不改一行照样通过
HTTP reap reapExpired 条件 claimed_at: { $lt: now - claimTtlMs } 条件更新,与通知侧同构
基线红 / 修复绿 仅提交测试时 update 80 / find 80(每跳 16);修复后 update 10 / find 80(每跳 9 = 1 + 分区数) 满足验收 1

开放问题裁定

  1. 插件选项:两个调度器共用 dispatchMaxIdleIntervalMs(选项 A)——dispatchIntervalMspartitionCount 本就共用;该选项尚未发布(17610 的 changeset 仍 pending),放宽不影响任何消费者;日后真有部署需要两个上限不同,再加独立选项也不破坏兼容
  2. redeliverHttp() 同样唤醒(选项 A)——它是另一条让行回到 pending 的服务写入;不唤醒的话运维手动重投要从 500ms 变成最多等 30s

崩溃节点认领的回收上界随之变为 claimTtlMs + 30s(默认约 35s,原约 5.5s),与 #17622 已接受的取舍一致。

范围外发现的去向

下一步:CI 全部结束且无失败 → 转 ready → 入合并队列(以 timeline added_to_merge_queue 为准)。有红按日志回派。

@hotlong
hotlong marked this pull request as ready for review September 11, 2026 06:10
@hotlong
hotlong enabled auto-merge September 11, 2026 06:10
@hotlong
hotlong added this pull request to the merge queue Sep 11, 2026
Merged via the queue into main with commit a9096af Sep 11, 2026
42 checks passed
@hotlong
hotlong deleted the claude/issue-17623-http-dispatcher-idle-cost branch September 11, 2026 06:38
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