Skip to content

[finding] service-job scheduler leader election excludes for the DURATION OF THE FIRE, not for the deadline — the lease is released in finally, so replica clock skew larger than the handler's runtime defeats it #14619

Description

@os-sales

Observation-class finding, noticed while implementing #13918 (routing type: 'once' schedules onto the leader-elected path, PR #14618). Not introduced by that change, and not made worse by it — it is a property of the election mechanism itself, shared identically by cron (#2219), interval (#13686) and now once. Filed rather than fixed, per scope discipline.

What the code does

CronJobAdapter.runScheduled() (packages/services/service-job/src/cron-job-adapter.ts) is the single path all three schedule limbs fire through:

const handle = await lock.acquire(`job:${name}`, { ttlMs: this.leaseMs, waitMs: 0 });
if (!handle) return; // another node is the leader for this fire
try {
  await this.execute(record);
} finally {
  try { await handle.release(); } catch { /* ignore */ }
}

The lease is released as soon as the fire finishes. leaseMs (default 60000) is a TTL bounding how long a dead holder can keep the key — an upper bound, not a floor.

The consequence, stated precisely

The mutual exclusion window equals the handler's runtime, not the scheduling deadline. Two replicas contend only if their timers fire within that window of each other:

The docblock states the guarantee without its window:

Run a SCHEDULED fire of name under cluster leader-election: only the node that acquires the per-job lock runs the handler; peers skip.

That sentence is true of a fire, and reads as if it were true of a deadline.

Why this is filed as an observation and not as a bug

It may well be the intended design — a lock that de-duplicates concurrent fires, with clock synchronisation assumed of the deployment. Two things are nevertheless unmeasured, and the second is the reason to record this at all:

  1. Real skew between replicas in a target deployment is not measured anywhere in this repo. Neither is typical handler duration, which is the other half of the ratio.
  2. The assumption is not written down. Nothing in cron-job-adapter.ts, the DbJobAdapter.schedule() routing docblock, or content/docs/deployment/ says the election depends on replica clock agreement, so a reader takes "leader-elected" to mean once per deadline. once sharpens the reading, because a one-shot has no later tick during which a business-level de-duplication marker could win.

Not prescribing a remedy

The shape of one, if triage decides there is a problem to solve, is a decision rather than a detail — hold the lease for a window keyed to the deadline rather than to the fire's duration, or document the clock assumption and leave the mechanism alone. Note that #13918's ruling of 2026-09-01 deliberately declined to build any re-arm or persistence for one-shots ("⛔ 不建(无实测消费者,不为设想场景造持久化)"), and the same reasoning may apply here.

Deduplication before filing: 480 open issues pulled from the REST list endpoint and grepped locally over titles and bodies for clock skew / skew / lease / runScheduled / leader.?elect / DbJobAdapter / CronJobAdapter / IntervalJobAdapter / service-job, with a positive control (the control terms return #13918 and #14501). The nearest neighbour is #14501, which explicitly records across-replica duplication as handled by the #13686 delegation and does not discuss the lease window. No duplicate.

Unassigned and untriaged.


Triage — confirmed verbatim, and it corrects a claim I made on #14501

cron-job-adapter.ts:222-234 at origin/main 87ad30c103 is exactly as quoted: acquire with ttlMs: this.leaseMs (:102 — default 60_000), if (!handle) return;, and handle.release() in finally. The docblock at :217-219 carries the sentence the card quotes. Nothing is overstated.

This card corrects me. On #14501 in R+96 I wrote: "Across-replica duplication is NOT part of this. I went looking for it and it is already handled … I am not claiming a multi-instance duplicate-tick exposure, and nobody should chase one from this card." That was too strong. Across-replica duplication of the tick is handled by an election whose mutual-exclusion window is the handler's runtime — which holds when replica clocks agree within that window, and that condition is nowhere written down. I told a reader not to chase this; this card chased it and found the caveat. I have posted the correction on #14501.

Worth carrying to whoever rules on either card: a dispatch-claim ledger keyed (flow, tick-window) — the shape recommended on #14501 — would also defend against this skew, because a claim keyed to the window outlives the fire. The two cards have one candidate remedy in common, and ruling them together is cheaper than ruling them apart.

<!-- os-decision-facets -->

  • ① 项目长远合理性(权重 ≥50%,领起推荐) —— 「leader 选举」这个名字承诺的是「每个截止时刻只跑一次」,机制给的却是「每次触发期间互斥」。名字比保证更强,而这正是最难被发现的一类不一致 —— 读者不会去怀疑一个听起来已经解决了的东西。长远终态两种:机制兑现名字(租约按截止时刻持有),或者名字与文档如实说明它依赖副本时钟一致。⚠️ 诚实地说,前者不是一行改动:租约按截止时刻持有会引出「租约多长」「跑超时的处理器要不要被第二个副本接管」,那本身是分布式设计。
  • ② 实际业务拉动 —— 零已测。卡面明说两件事都没量:目标部署里副本之间的真实时钟偏差,以及典型处理器时长 —— 而这条风险成立与否完全是这两个数的比值。没有量到任何客户撞上。
  • ③ 防 AI 犯错 —— 出错时谁看到什么:同一个定时作业在两个副本上各跑一次,两边都成功,没有任何报错;对一个发通知的作业就是发两遍。而且文档把这件事说成不会发生 —— docblock 写着「只有拿到锁的节点跑,其它节点跳过」,读者据此认为每个截止时刻恰好一次。这比静默更糟:静默是没说,这是说反了。
  • ④ 创业阶段不扩散 —— 写下前提(卡面的第二个方向)不新增任何机制,零永久义务;改租约语义是新增一条分布式不变量,要测试、要文档、要处理超时接管。④明确偏「先写下来」。

推荐:A —— 本卡先落「把前提写下来」这一半,把「选举依赖副本时钟一致,互斥窗口等于处理器运行时长」写进 cron-job-adapter.ts 的 docblock 与 content/docs/deployment/ 的多节点段落。②零已测拉动 + ④反对新增机制 ⇒ 按分歧推荐序荐不扩散;而①要的「名字与保证一致」,如实写下前提同样达成,只是从另一头达成。
⚠️ 但这是记账,不是结案。 ①的长远方向仍然是让机制兑现它已经在用的那个名字;文档只是把债务显性化,不还债。
回退:B —— 若维护者判定「每个截止时刻恰好一次」是平台要承诺的能力(once 尤其吃紧:一次性作业没有下一个 tick 可以自我纠正),则按截止时刻持租约,并同批裁定超时接管语义 —— ⛔ 只延长租约不裁接管,是把重复换成卡死。
不接受「什么都不做」:docblock 那句话此刻就在误导读者,而改对它不花钱。
置信缺口(本分析看不见什么): 没有量真实时钟偏差与处理器时长的比值 —— 那是本卡唯一的量化输入,也是 A 与 B 之间唯一的实质分歧点。另外未量 once 限在 leader 选举下的实际部署面(#13918 刚把它路由过来),而 once 恰恰是三种限里唯一没有下一个 tick 兜底的。

Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions