Skip to content

Commit 5f1f7ba

Browse files
committed
fix: heartbeat v2.3 — immediate first beat + lead-clamp regime
Spec review round 4 (two confirmed findings, both fixed): 1. Any bounded first-beat delay can outlive a small tenant-capped lease (a 30 s cap is still 28 s too late for a 2 s grant). The first extend now fires IMMEDIATELY: it costs one extension but provably beats an arbitrarily small lease, and its response primes the grant ledger with a real grant sample. All Date-derived effective-TTL/hint scheduling is removed from the heartbeat path (start_heartbeat drops the hint parameter; reserve() no longer threads date_ms). ApiResponse::date_ms + httpdate parsing remain as general response utilities with unit and e2e tests. 2. Under a server-side maximum-LEAD clamp (extend re-stamps expires_at ~ now + L), successive expires_at_ms differences measure elapsed time, not lease — grant-derived cadence is self-referential and collapses to the 500 ms floor, burning max_extensions in seconds. Each success is now classified (is_lead_clamp_grant): a grant that is non-positive, or < 0.9*requested while within [0.75, 1.25]x elapsed-since-last-success (a clock reading, not a lease), holds the cadence at min(requested/2, 30 s), never tightened, with a tracing::warn once per heartbeat. The lower band arm lets a real-but-small per-extend grant re-tighten after a skip doubles the gap. The same held cadence paces retries before any grant sample exists, so a failed immediate first beat can never hot-loop at zero delay. Everything else unchanged: skip rule (lead_min >= 1.5*last_grant), requested-amount wire extends, same-key transient retry, permanent stop set, 2xx-as-applied, intended-instant scheduling, cancellation. Also folds in the uncommitted v2.2 grant-ledger baseline this refines (rounds 2-3: Date rejected as a correctness input, grant ledger with same-frame arithmetic, per-beat delays, TENANT_CLOSED/NOT_FOUND stops). Tests: wiremock suite reworked — immediate-first-beat cadence (extend@0/1000/2000, skip@3000, extend@4000), capped-grant discovery via the immediate beat, 503-on-first-beat single held-cadence retry with same idempotency key, lead-clamp echo responder holding cadence (no floor collapse), zero-grant immediate prime holding cadence, per-extend grant clamp (+ttl/4) still tightening to 500 ms, small-ttl liveness, permanent stops, unknown-status 2xx as applied. Pure cadence/regime functions unit-tested (held-cadence pins incl. 30 s cap, lead-clamp band boundaries, skip threshold, trace). Obsolete effective-TTL/date- hint tests removed; Date parsing tests kept; date_ms asserted e2e in response_test. CHANGELOG and AUDIT amended in place (same 0.3.1 release, PR #75).
1 parent 2346011 commit 5f1f7ba

9 files changed

Lines changed: 556 additions & 336 deletions

File tree

AUDIT.md

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
---
1010

11-
## 2026-07-27 — heartbeat extend-drift fix, grant-ledger v2.2 (v0.3.1)
11+
## 2026-07-27 — heartbeat extend-drift fix, grant-ledger v2.3 (v0.3.1)
1212

1313
P1 liveness, fleet-wide (same bug in all four SDKs). The spec's `extend_by_ms`
1414
is relative to the reservation's *current* `expires_at_ms`, not request time,
@@ -29,32 +29,47 @@ correctness input — RFC 9110 `Date` is a whole-second best-effort
2929
*origination* timestamp replaceable by intermediaries, and in cycles-server
3030
`expires_at_ms` comes from Redis `TIME` while `Date` comes from the HTTP
3131
layer (not the same clock), so the difference is no lease measurement and the
32-
1000 ms upward clamp fabricated lease. Final **grant-ledger (v2.2)** design:
33-
correctness rests on `lead_min = grants_sum − elapsed` (signed, starts 0), a
34-
rigorous lower bound built only from same-frame arithmetic — each grant is
35-
the difference of successive server-frame `expires_at_ms` values (reserve's
36-
threaded from the guard, then each extend response's), elapsed is
37-
client-monotonic to the beat's intended instant. Skip iff a grant sample
38-
exists and `lead_min ≥ 1.5·last_grant`; else extend by the **requested**
39-
`ttl_ms`. Per-beat delays replace the fixed interval: first beat
40-
`min(requested/2, 30 s, date_hint/2)` (the `Date`-derived estimate survives
41-
only as this raw, unclamped cadence hint — `ApiResponse::date_ms`, `httpdate`
42-
direct dep), then `clamp(last_grant/2, 500 ms, requested/2)`, so the cadence
43-
tracks observed grants (clamped grants speed it up); transient failures retry
44-
at the current cadence with the same idempotency key (replay dedupe); beats
45-
schedule from intended instants and realign to now after stalls
46-
(`MissedTickBehavior::Skip` equivalent, hand-rolled for variable delays). Any
47-
2xx counts as applied (`expires_at_ms` authoritative, warn on odd status);
48-
permanent codes (`RESERVATION_EXPIRED`/`RESERVATION_FINALIZED`/
49-
`MAX_EXTENSIONS_EXCEEDED`/`TENANT_CLOSED`/`NOT_FOUND` or HTTP 410/404) stop
50-
the heartbeat. Cancellation unchanged. Ten wiremock tests with dynamic expiry
51-
responders (cadence extend@1..4/skip@5/extend@6; capped grant keeping
52-
`extend_by_ms` at the request while beating at hint/2 then grant/2; garbage
53-
`Date` → requested/2; key reuse; permanent stops; small-ttl liveness; clamped
54-
grants never skipping; unknown-status 200 as applied) + extracted pure
55-
delay/lead functions unit-tested (incl. the 30 s first-beat cap and raw
56-
unclamped hint) in `src/heartbeat.rs`; `Date`-parsing unit tests in
57-
`src/response.rs`. Coverage 95.73%; tests, clippy `-D warnings`, fmt green.
32+
1000 ms upward clamp fabricated lease. Spec review round 4 then removed
33+
lease estimation from scheduling entirely: any *bounded* first-beat delay
34+
can outlive a small capped lease, so the **first extend fires immediately**
35+
(costs one extension, provably beats an arbitrarily small grant, and primes
36+
the ledger with a real sample; `ApiResponse::date_ms`/`httpdate` stay as
37+
general response utilities the heartbeat no longer consumes). Round 4 also
38+
found that under a server-side **maximum-LEAD clamp** (extend re-stamps
39+
`expires_at ≈ now + L`) successive `expires_at_ms` differences measure
40+
*elapsed time, not lease* — grant-derived cadence is self-referential there
41+
and collapses to the 500 ms floor, burning `max_extensions` in seconds.
42+
Final **grant-ledger (v2.3)** design: correctness rests on
43+
`lead_min = grants_sum − elapsed` (signed, starts 0), a rigorous lower bound
44+
built only from same-frame arithmetic — each grant is the difference of
45+
successive server-frame `expires_at_ms` values (reserve's threaded from the
46+
guard, then each extend response's), elapsed is client-monotonic to the
47+
beat's intended instant. Skip iff a grant sample exists and
48+
`lead_min ≥ 1.5·last_grant`; else extend by the **requested** `ttl_ms`.
49+
Per-beat delays replace the fixed interval: first beat immediate, then
50+
`clamp(grant/2, 500 ms, requested/2)` (clamped grants speed it up) — unless
51+
`is_lead_clamp_grant` classifies the grant as a clock reading (non-positive,
52+
or `< 0.9·requested` while within `[0.75, 1.25]×` elapsed-since-last-
53+
success), which holds the cadence at `min(requested/2, 30 s)` and warns once
54+
per heartbeat; the same held cadence paces retries before any grant sample,
55+
so a failed immediate beat cannot hot-loop. Transient failures retry with
56+
the same idempotency key (replay dedupe); beats schedule from intended
57+
instants and realign to now after stalls (`MissedTickBehavior::Skip`
58+
equivalent, hand-rolled for variable delays). Any 2xx counts as applied
59+
(`expires_at_ms` authoritative, warn on odd status); permanent codes
60+
(`RESERVATION_EXPIRED`/`RESERVATION_FINALIZED`/`MAX_EXTENSIONS_EXCEEDED`/
61+
`TENANT_CLOSED`/`NOT_FOUND` or HTTP 410/404) stop the heartbeat.
62+
Cancellation unchanged. Eleven wiremock tests with dynamic expiry responders
63+
(immediate first beat + extend@0/1000/2000, skip@3000, extend@4000; capped
64+
grant keeping `extend_by_ms` at the request while the immediate beat
65+
discovers the cap; 503 on the immediate beat → single held-cadence retry
66+
with the same key; permanent stops; small-ttl liveness; per-extend grant
67+
clamp still tightening to grant/2; lead-clamp echo responder holding
68+
cadence instead of collapsing; zero-grant immediate prime holding cadence;
69+
unknown-status 200 as applied) + extracted pure cadence/regime functions
70+
unit-tested (incl. the 30 s held-cadence cap and the lead-clamp band
71+
boundaries) in `src/heartbeat.rs`; `Date`-parsing unit tests in
72+
`src/response.rs`. Coverage 95.12%; tests, clippy `-D warnings`, fmt green.
5873

5974
## 2026-07-27 — v0.3.0 self-review hardening
6075

CHANGELOG.md

Lines changed: 9 additions & 5 deletions
Large diffs are not rendered by default.

src/client.rs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,7 @@ impl CyclesClient {
169169
validation::validate_grace_period_ms(req.grace_period_ms)?;
170170
validation::validate_non_negative(req.estimate.amount, "estimate.amount")?;
171171

172-
// The metadata variant surfaces the HTTP `Date` header: paired with
173-
// the body's expires_at_ms it yields a rough TTL estimate used as
174-
// the heartbeat's *first-beat cadence hint* when a tenant policy
175-
// (max_reservation_ttl_ms) silently capped the grant. Hint only —
176-
// Date is not the same clock as expires_at_ms (RFC 9110 whole-second
177-
// best-effort origination timestamp vs. Redis TIME), so correctness
178-
// never depends on it; see src/heartbeat.rs module docs.
179-
let resp = self.create_reservation_with_metadata(&req).await?;
180-
let date_ms = resp.date_ms;
181-
let resp = resp.into_inner();
172+
let resp = self.create_reservation(&req).await?;
182173

183174
if resp.decision.is_denied() {
184175
return Err(Error::BudgetExceeded {
@@ -238,7 +229,6 @@ impl CyclesClient {
238229
resp.expires_at_ms,
239230
resp.affected_scopes.clone(),
240231
req.ttl_ms,
241-
crate::heartbeat::date_ttl_hint_ms(resp.expires_at_ms, date_ms),
242232
req.subject.clone(),
243233
req.action.clone(),
244234
))

src/guard.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,21 +83,20 @@ impl ReservationGuard {
8383
expires_at_ms: Option<u64>,
8484
affected_scopes: Vec<String>,
8585
requested_ttl_ms: u64,
86-
date_ttl_hint_ms: Option<u64>,
8786
subject: Subject,
8887
action: Action,
8988
) -> Self {
9089
let cancel = CancellationToken::new();
9190
// The reserve response's expires_at_ms (server frame) is the base of
92-
// the heartbeat's grant ledger; the Date-derived TTL estimate is a
93-
// first-beat cadence hint only (the Date header is not the same
94-
// clock as expires_at_ms); see src/heartbeat.rs module docs.
91+
// the heartbeat's grant ledger. The first extend fires immediately —
92+
// a tenant policy may have silently capped the lease far below the
93+
// requested TTL, and no bounded delay provably beats an arbitrarily
94+
// small grant; see src/heartbeat.rs module docs.
9595
let heartbeat = start_heartbeat(
9696
client.clone(),
9797
id.clone(),
9898
requested_ttl_ms,
9999
expires_at_ms,
100-
date_ttl_hint_ms,
101100
cancel.clone(),
102101
);
103102

0 commit comments

Comments
 (0)