fix: heartbeat extend drift + actual_source audit marker (v0.5.1) - #90
Conversation
extend_by_ms extends relative to the reservation's CURRENT expires_at_ms (spec ~1761; server extend.lua adds to current expiry), but every heartbeat extended by the full ttl_ms at ttl/2 cadence — drifting expiry outward +ttl/2 per beat. Consequences: a killed process left reserved budget locked until the drifted expiry (zombie window scaling with runtime, bounded only by max_extensions ~10), and long runs exhausted max_extensions twice as fast as necessary, silently losing heartbeat protection mid-flight. All four heartbeats (sync/async lifecycle, sync/async streaming) now use alternate-beat extension: extend on the first beat (only ttl/2 lifetime remains) and every second beat after a success; retry immediately after a failure. No client-vs-server clock comparison (skew-safe). Expiry lead stays within [ttl/2, 1.5*ttl]; extension consumption halved. Also: commits whose actual was structurally defaulted from the estimate (@cycles without an actual expression; streams without a recorded cost or with a raised cost_fn) now stamp metadata.actual_source="estimate" so audit evidence distinguishes measured from assumed spend. Defaults unchanged. Fleet-wide: TS/Java/Rust ship the same heartbeat fix; spec heartbeat guidance in cycles-protocol#148 (v0.1.25.16). 517 tests pass at 100% coverage; ruff and mypy --strict clean.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…nent stops Adversarial self-review of the alternate-beat fix found confirmed inward-drift liveness hazards: at steady state every attempt fired at exactly ttl/2 lead so one failed extend put the retry at lead 0; the 1s interval floor guaranteed lapse for spec-legal ttl in (1000,2000); and sleep-after-response re-arming slipped beats by one RTT per cycle. The heartbeat now estimates its remaining lead from the AUTHORITATIVE expires_at_ms the server returns, compared clock-skew-free (server-frame differences plus client-monotonic elapsed only), extending by ttl when lead < 1.5*ttl and skipping otherwise. Failures retry next beat with the SAME idempotency key (a lost-response extend cannot double-apply); permanent codes (RESERVATION_EXPIRED / RESERVATION_FINALIZED / MAX_EXTENSIONS_EXCEEDED / TENANT_CLOSED / NOT_FOUND, or status 410) stop the heartbeat for good; the 1s interval floor is removed. Spec-review round: tenant policy max_reservation_ttl_ms (default 1h) silently caps granted TTLs and the create response exposes no effective TTL — seeding from the requested ttl could schedule the first beat hours after expiry. The client now captures the HTTP Date header (CyclesResponse.server_date_ms) and seeds the heartbeat from effective_ttl = clamp(expires_at_ms - Date, 1000, requested), still skew-free (server-frame difference). All four heartbeats (sync/async lifecycle, sync/async streaming) share the design. Spec guidance updated in cycles-protocol#148 (02d1270). 531 tests pass at 100% coverage; ruff and mypy --strict clean.
|
Self-review round 2 fixes landed (lead-estimate redesign): the alternate-beat scheme had confirmed inward-drift hazards (single-failure lead-0 retry; guaranteed lapse for ttl<2s under the interval floor; RTT slippage). The heartbeat now runs on a clock-skew-free lead estimate from the authoritative |
…oted to hint Spec review round 3 (cycles-protocol#148): the HTTP Date header is not a safe same-clock anchor for effective TTL — per RFC 9110 it is a whole-second, best-effort origination timestamp that intermediaries may replace, and it need not come from the clock stamping expires_at_ms (in cycles-server that clock is Redis TIME while Date comes from the servlet container). Clamping the estimate upward to 1000ms fabricated lease. The heartbeat now maintains the only rigorous cross-clock-free quantity available: a conservative lead LOWER BOUND, lead_min = sum(measured grants) - monotonic elapsed, where grants are differences of successive returned expires_at_ms values (same server frame). lead_min starts at 0, so the first extension fires early (bounded by min(ttl/2, 30s, half the Date-derived hint when present)), establishing real measured margin and revealing the actual per-extend grant. Cadence derives from the measured grant (clamps self-correct); skip when lead_min >= 1.5*last_grant. The Date estimate informs only the first-beat delay and is never clamped upward. _effective_ttl_ms now returns None when underivable. Total protected runtime is invariant (initial TTL + sum of grants), so the early prime costs no coverage. All four heartbeats share the design. 533 tests pass at 100% coverage; ruff and mypy --strict clean.
|
v2.2 (round 3): the Date-header effective-TTL derivation was correctly rejected by spec review — RFC 9110 semantics plus a real two-clock split in cycles-server (Redis TIME stamps |
Round-4 review of the spec guidance (cycles-protocol#148) surfaced two P1s in the v2.2 design; this lands the fixes on the same PR, same version (0.5.1): - Immediate first extension. lead_min starts at 0 and no trustworthy effective-TTL signal exists on the wire, so ANY bounded first delay can outlive a tenant-policy-capped lease (24h request capped to a small lease → first beat after expiry). The primed beat costs one extension; total protected runtime is unchanged. The Date-derived effective-TTL hint (_effective_ttl_ms, streaming _est_ttl plumbing) is removed entirely — Date now plays no heartbeat role; CyclesResponse.server_date_ms stays as a general accessor. - Regime-aware cadence. Under maximum-lead clamping the grant mirrors elapsed time, not lease size: grant/2 cadence would collapse to the 500ms floor and burn max_extensions in seconds. The loop now detects the regime per success (grant <= 0, or grant < 0.9*ttl and grant <= 1.25*elapsed-since-last-success): clamped grants hold a bounded cadence min(ttl/2, 30s) and warn once that the extension budget will deplete; lease-tracking grants keep clamp(grant/2, 500ms, ttl/2). - Hot-loop backstop (found tracing this port): a transient failure on the primed delay-0 beat must not retry at 0ms; after the primed beat the baseline cadence is the held delay in all four loops. Tests reworked to v2.3 traces (immediate first beat, lead-clamp hold + single warning, no-hot-loop backoff); decorator tests get an optional extend mock since the primed beat now fires in-test. 532 tests, 100% coverage; ruff + mypy clean.
|
v2.3 (
Tests reworked to the v2.3 traces: immediate first beat (including the 24h-capped-lease case), lead-clamp regime holds cadence and warns exactly once, primed-beat failure backs off to 30s and reuses the same idempotency key. Decorator tests gained an optional |
Cross-SDK review (Rust port) caught a sticky misclassification in the v2.3 regime detector: after a lead_min skip, the next measured grant arrives across a doubled gap, so grant ~= elapsed exactly (cadence is grant/2) and the upper-bound-only test (grant <= 1.25*elapsed) claims lead-clamping. The hold then self-sustains — at held cadence 30s a 15s-grant lease banks +15s per 30s elapsed and decays to a lapse. The classifier now requires grant to sit INSIDE [0.75, 1.25]*elapsed (besides grant < 0.9*ttl; grant <= 0 still always clamps). Genuine maximum-lead clamps track any gap (ratio ~= 1) and stay held; a real post-skip small grant lands in the hold once, then at the held cadence its ratio falls to ~0.5, exits the band, and cadence re-tightens. Regression test pins the full 7-beat trace: 3 extends at grant/2, skip, one 30s hold, re-tighten to grant/2. 533 tests, 100% coverage.
|
v2.3 amendment ( The clamp arm now requires the grant to sit inside |
Spec review round 5 (cycles-protocol#148) proved the heuristic's regime detection formally undecidable — the band has a sticky window grant in [0.75*min(ttl/2,30s), 0.9*ttl) where a grant-clamped server stays misclassified while its lease erodes to a lapse — and immediate priming schedule-dependent under maximum-lead clamping. The adopted fix is a wire field: spec v0.1.25.16 adds remaining_ttl_ms (same clock snapshot as expires_at_ms) to create and extend responses; cycles-server v0.1.25.59 emits it (runcycles/cycles-server#260). When a response carries the field, the heartbeat schedules from it directly (all four loops): - lead_floor = max(0, remaining_ttl_ms - rtt), rtt measured monotonically around each extend call (max tracked per heartbeat) - next beat at lead_floor - min(lead_floor/2, max(1s, 2*rtt_max)) after response receipt; recomputed on every field-bearing response; never accumulates expiry differences in this mode - the create response's field drives the FIRST delay - no primed extension is spent, and a 24h request capped to a 1s lease gets its first beat at 500ms, inside the real lease - the heuristic lead_min skip is bypassed (scheduling is exact) - transient failures retry with the SAME idempotency key after clamp(current_lead/4, 1s, 30s), current_lead = last lead_floor decayed by monotonic elapsed Grants/lead bookkeeping keeps running in both modes, so the v2.3+band heuristic - now explicitly a best-effort fallback - resumes seamlessly when the field disappears; fieldless servers see unchanged behavior. ReservationCreateResponse gains the optional remaining_ttl_ms field. Tests: field-driven first beat + steady 59s cadence with the skip provably bypassed; capped-lease first beat; lead-clamp server WITH the field (no warn, no collapse); mid-flight field disappearance resuming the heuristic; bounded same-key retries on 503 and exceptions across sync/async lifecycle and both stream heartbeats. 543 tests, 100% coverage; ruff + mypy clean.
|
Round 5 ( When a create/extend response carries the field, the heartbeat schedules from it directly: The v2.3+band heuristic remains, now explicitly a best-effort fallback for fieldless servers — round 5 proved its regime detection undecidable (sticky window 543 tests, 100% coverage, ruff + mypy clean. |
Aligns the field mode with the settled HEARTBEAT GUIDANCE on cycles-protocol#148 (head dd60c27), which hardened the round-5 draft: - Scheduling: per-attempt monotonic rtt (max tracked); lead_floor = max(0, remaining_ttl_ms - rtt); attempt_budget = max(request_timeout_budget, 1s, 2*rtt_max) with the budget wired from the SDK's enforced httpx timeouts (connect + read + write = 12s by default); safety_margin = max(1s, 2*rtt_max); retry_reserve = 2*attempt_budget + safety_margin; next beat at lead_floor - retry_reserve after response receipt, recomputed on every field-bearing schema-valid HTTP 200. - Success predicate: only a schema-valid 200 counts; an ambiguous non-200 2xx is never applied and recovers with the SAME key. - Recovery: retry_window = lead_estimate - attempt_budget - safety_margin, unclamped; window < 0 stops and surfaces; recovery repeats with the same key while the freshly recomputed window shrinks (progress guard against zero-time loops; window 0 permits one immediate retry); 429 honors delta-seconds Retry-After only inside the window; other 4xx stop without key rotation. - Zero-delay guard: a success whose lease cannot hold the reserve permits ONE immediate fresh attempt, then the heartbeat stops and surfaces that the lease is shorter than the retry-safety budget. - Create rtt is now measured around the create call and feeds the first beat's lead_floor; the fallback-only backstop no longer clobbers meaningful authoritative zero delays. The v2.3+band heuristic is unchanged as the fieldless fallback; all its tests pass unedited. New coverage: scheduler edge cases, steady 35s cadence, shrinking recovery windows (6250 -> 4687.5 -> 1062.5 -> 0 -> stop), ambiguous-2xx same-key recovery, 429 within/exceeding window, 4xx stop, zero-delay guard, and a stop/recovery matrix across all four loops. 568 tests, 100% coverage, ruff + mypy clean.
|
Hardened-algorithm alignment ( |
1 participant
Heartbeat extend drift (P1 liveness)
extend_by_msis relative to the reservation's currentexpires_at_ms(spec;extend.luaadds to current expiry), but every heartbeat extended by the fullttl_msatttl/2cadence — drifting expiry outward+ttl/2per beat. Kill the process after a long run and the reserved budget stays locked until the drifted expiry (zombie window scaling with runtime, bounded only bymax_extensions); long runs also burnmax_extensionstwice as fast as needed and silently lose heartbeat protection mid-flight.All four heartbeats now use alternate-beat extension (first beat + every second beat after success; immediate retry after failure). Clock-skew-safe (no client-vs-server time comparison); expiry lead stays in
[ttl/2, 1.5×ttl]; extension budget consumption halved. Deterministic beat-pattern tests for all four sites.Fleet-wide: same fix landing in TS/Java/Rust; normative spec guidance in runcycles/cycles-protocol#148.
actual_source marker (audit honesty)
Commits whose actual was structurally defaulted from the estimate now stamp
metadata.actual_source: "estimate"(+ debug log) — audit evidence distinguishes measured from assumed spend without breaking the documented default. Flows into/v1/eventsrecovery bodies automatically.Verification
517 tests at 100% coverage; ruff + mypy strict clean. v0.5.0 → v0.5.1.