mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
perf(heartbeat): retry a preparation a restart interrupted at once (#20255)
## Thinking Path - Restart recovery should resume interrupted work quickly. #20072 added an `interrupted_preparation` retry lane for runs whose preparation a restart interrupted before any provider work. - Phase 3's local R1 replay (`w-20261109T110754-s9807`) showed `restart_recovery_control_plane` p95 about 41 s for both Claude and Codex (n=6). - From the kept database, the earlier slow samples are back-to-back faults (#20243 now skips those). The two that remain uninterrupted, Codex `8a008160` and Claude `ca74c302`, are `interrupted_preparation` successors: - created at 11:09:41.7–41.8, as f03 recovered; - `scheduled_retry_at` 26.3 s and 34.2 s later; - claimed 18.5 s and 26.5 s after f04 was ready. - The lane reused the transient-failure backoff (30 s, then 60 s, jittered). A restart isn't a transient fault, so the successor shouldn't wait. ## Linked Issues or Issue Description Refs #20072 **What happened?** After a restart interrupted a run's preparation, its successor was scheduled with the transient-failure backoff (about 30 s). Recovery took about 40–50 s instead of a few seconds. **Steps to reproduce** 1. Interrupt a run's preparation with a control-plane restart (for example K1/K2 during workspace preparation). 2. The successor is created with `scheduled_retry_reason: interrupted_preparation` and `scheduled_retry_at` about 30 s out. **Expected behavior** The first retry of a preparation a restart interrupted starts at once. Backoff applies only if it keeps being interrupted. ## What Changed - `server/src/services/heartbeat/retries.ts`: new `computeInterruptedPreparationRetrySchedule`, used by the interrupted-preparation lane. - Attempt 1 is due immediately (delay 0). - Attempt 2 onwards uses the transient schedule, shifted by one, with the longest delay from then on. - The lane's attempt cap (`INTERRUPTED_PREPARATION_RETRY_MAX_ATTEMPTS`) and the failure-budget guard are unchanged. - `retries.test.ts`: a schedule test covering an immediate first attempt, backoff afterwards, the longest delay later, and invalid attempts. It's red on master. ## Verification - `retries.test.ts`: 24 passed. - `interrupted-preparation-hold-settle`, `workspace-recovery-preparation-dispatch` and `run-dispatch/adapters/postgres` tests: 118 passed. - `heartbeat-process-recovery.test.ts` filtered to interrupt, prepare and retry cases: 59 passed and 1 failed. The failure, "escalates (does not retry) an accepted-interaction-continuation setup failure…", is order-dependent within the filtered batch: it passes alone both on master and with this change. The unfiltered file exceeds the local timeout; CI runs it. ## Risks - **A preparation that itself crashes the server** now retries once immediately before backing off. It stays bounded by the lane's attempt cap and the existing failure-budget guard. - **No change** to transient-failure retries or to other lanes. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
8eb11fa8aa
commit
7adb78d811
2 files changed
+39
-6
No files matched your search
@@ -475,3 +475,19 @@ describePostgres("heartbeat retry module database wiring", () => {
|
||||
expect(await retries.retryScheduledRetryNow({ issueId: selected.issue.id, now })).toMatchObject({ outcome: "already_promoted" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("interrupted preparation retry schedule", () => {
|
||||
it("retries a preparation a restart interrupted at once, and backs off only if it keeps being interrupted", () => {
|
||||
const now = new Date("2026-11-09T11:09:41.846Z");
|
||||
const first = extracted.computeInterruptedPreparationRetrySchedule(1, now, () => 0.5);
|
||||
expect(first).toMatchObject({ attempt: 1, delayMs: 0, dueAt: now });
|
||||
const second = extracted.computeInterruptedPreparationRetrySchedule(2, now, () => 0.5);
|
||||
expect(second?.delayMs).toBeGreaterThan(0);
|
||||
expect(second).toMatchObject({ attempt: 2, baseDelayMs: extracted.BOUNDED_TRANSIENT_HEARTBEAT_RETRY_DELAYS_MS[0] });
|
||||
// Later attempts keep the longest transient delay.
|
||||
expect(extracted.computeInterruptedPreparationRetrySchedule(5, now, () => 0.5))
|
||||
.toMatchObject({ attempt: 5, baseDelayMs: extracted.BOUNDED_TRANSIENT_HEARTBEAT_RETRY_DELAYS_MS.at(-1) });
|
||||
expect(extracted.computeInterruptedPreparationRetrySchedule(0, now)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -405,6 +405,28 @@ export function computeBoundedTransientHeartbeatRetrySchedule(
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The interrupted-preparation lane: a preparation that a restart interrupted
|
||||
* spent no provider work and hit no transient fault, so its first retry starts
|
||||
* at once. If it is interrupted again it backs off like a transient failure
|
||||
* (the longest delay from then on), so a preparation that itself keeps taking
|
||||
* the server down is slowed and still capped by the lane's attempt limit.
|
||||
*/
|
||||
export function computeInterruptedPreparationRetrySchedule(
|
||||
attempt: number,
|
||||
now = new Date(),
|
||||
random: () => number = Math.random,
|
||||
) {
|
||||
if (!Number.isInteger(attempt) || attempt <= 0) return null;
|
||||
if (attempt === 1) {
|
||||
return { attempt, baseDelayMs: 0, delayMs: 0, dueAt: new Date(now.getTime()),
|
||||
maxAttempts: BOUNDED_TRANSIENT_HEARTBEAT_RETRY_MAX_ATTEMPTS };
|
||||
}
|
||||
const schedule = computeBoundedTransientHeartbeatRetrySchedule(
|
||||
Math.min(attempt - 1, BOUNDED_TRANSIENT_HEARTBEAT_RETRY_MAX_ATTEMPTS), now, random);
|
||||
return schedule ? { ...schedule, attempt } : null;
|
||||
}
|
||||
|
||||
export { normalizeAgentNameKey };
|
||||
|
||||
function readNonEmptyString(value: unknown): string | null {
|
||||
@@ -515,12 +537,7 @@ export function createHeartbeatRetries(db: Db, dependencies: HeartbeatRetryDepen
|
||||
: null
|
||||
: nextAttempt <= maxAttempts
|
||||
? interruptedPreparationLane
|
||||
? (() => {
|
||||
// Later lane attempts keep the longest transient delay.
|
||||
const schedule = computeBoundedTransientHeartbeatRetrySchedule(
|
||||
Math.min(nextAttempt, BOUNDED_TRANSIENT_HEARTBEAT_RETRY_MAX_ATTEMPTS), now, opts?.random);
|
||||
return schedule ? { ...schedule, attempt: nextAttempt } : null;
|
||||
})()
|
||||
? computeInterruptedPreparationRetrySchedule(nextAttempt, now, opts?.random)
|
||||
: computeBoundedTransientHeartbeatRetrySchedule(
|
||||
nextAttempt,
|
||||
now,
|
||||
|
||||
Reference in new issue
Block a user