-
Notifications
You must be signed in to change notification settings - Fork 8
fix(temporal): retry Glitter startup metric restoration #2006
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
shepherdjerred
wants to merge
1
commit into
main
Choose a base branch
from
glitter-pagerduty-alert
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
35 changes: 35 additions & 0 deletions
35
packages/docs/plans/2026-08-07_glitter-corpus-startup-metric-retry.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| --- | ||
| id: plan-2026-08-07-glitter-corpus-startup-metric-retry | ||
| type: plan | ||
| status: in-progress | ||
| board: false | ||
| --- | ||
|
|
||
| # Self-healing Glitter startup metric restoration | ||
|
|
||
| ## Summary | ||
|
|
||
| Add an indefinite, exponential-backoff retry supervisor for Glitter's startup | ||
| snapshot-metric restoration. This prevents transient SeaweedFS startup races | ||
| from producing a false stale alert while preserving the existing alert for | ||
| genuinely missing, invalid, or inaccessible snapshots. | ||
|
|
||
| ## Implementation | ||
|
|
||
| - Add an injectable startup retry helper with equal-jitter exponential backoff: | ||
| 10-second initial ceiling, doubling per attempt, capped at five minutes. | ||
| - Retry only transient SeaweedFS connection and 408/429/5xx failures. Do not | ||
| retry missing pointers, authorization failures, malformed data, checksum | ||
| failures, or schema violations. | ||
| - Stop retrying when worker shutdown begins, log every retry, and emit one | ||
| Sentry warning after ten consecutive transient failures. | ||
| - Keep the existing Prometheus alert expression, schedule timing, and | ||
| PagerDuty routing unchanged. | ||
|
|
||
| ## Verification | ||
|
|
||
| - Cover retry success, jitter bounds, cap, shutdown cancellation, persistent | ||
| transient failures, and storage-error classification with `bun:test`. | ||
| - Run the Temporal package tests, typecheck, and lint. | ||
| - After deployment, verify startup restoration, the snapshot timestamp metric, | ||
| alert resolution, and natural PagerDuty resolution. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,26 @@ const S3ErrorShapeSchema = z.object({ | |
| $metadata: z.object({ httpStatusCode: z.number().optional() }).optional(), | ||
| }); | ||
|
|
||
| const TRANSIENT_STORAGE_ERROR_PATTERN = | ||
| /\b(?:ECONNREFUSED|ECONNRESET|ETIMEDOUT|EAI_AGAIN|ENOTFOUND)\b/i; | ||
|
|
||
| export function isTransientCorpusStorageError(error: unknown): boolean { | ||
| const parsed = S3ErrorShapeSchema.safeParse(error); | ||
| const statusCode = parsed.success | ||
| ? parsed.data.$metadata?.httpStatusCode | ||
| : undefined; | ||
| if ( | ||
| statusCode !== undefined && | ||
| (statusCode === 408 || statusCode === 429 || statusCode >= 500) | ||
| ) { | ||
| return true; | ||
| } | ||
| return ( | ||
| error instanceof Error && | ||
| TRANSIENT_STORAGE_ERROR_PATTERN.test(`${error.name} ${error.message}`) | ||
| ); | ||
|
Comment on lines
+37
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Cause chain not retried isTransientCorpusStorageError only checks the top-level error’s $metadata.httpStatusCode and a regex
against ${error.name} ${error.message}, so a transient connection failure that is wrapped in an
Error.cause chain (where the outer error message doesn’t include the connection code) will be
treated as non-transient and will stop the startup retry loop.
Agent Prompt
|
||
| } | ||
|
|
||
| export function isNotFoundError(error: unknown): boolean { | ||
| const parsed = S3ErrorShapeSchema.safeParse(error); | ||
| if (!parsed.success) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { | ||
| equalJitterRetryDelayMs, | ||
| retryUntilReady, | ||
| STARTUP_RETRY_MAXIMUM_DELAY_MS, | ||
| } from "./startup-retry.ts"; | ||
|
|
||
| describe("equal-jitter startup retry delays", () => { | ||
| test("doubles the ceiling and keeps delays within the equal-jitter range", () => { | ||
| expect(equalJitterRetryDelayMs(1, 0)).toBe(5000); | ||
| expect(equalJitterRetryDelayMs(1, 1)).toBe(10_000); | ||
| expect(equalJitterRetryDelayMs(2, 0.5)).toBe(15_000); | ||
| }); | ||
|
|
||
| test("caps the exponential ceiling at five minutes", () => { | ||
| expect(equalJitterRetryDelayMs(20, 0)).toBe(150_000); | ||
| expect(equalJitterRetryDelayMs(20, 1)).toBe(STARTUP_RETRY_MAXIMUM_DELAY_MS); | ||
| }); | ||
| }); | ||
|
|
||
| describe("retryUntilReady", () => { | ||
| test("retries a transient operation until it succeeds", async () => { | ||
| let calls = 0; | ||
| const delays: number[] = []; | ||
|
|
||
| const result = await retryUntilReady({ | ||
| operation: async () => { | ||
| calls += 1; | ||
| if (calls < 3) { | ||
| throw new Error("ECONNREFUSED"); | ||
| } | ||
| }, | ||
| shouldRetry: () => true, | ||
| isClosed: () => false, | ||
| random: () => 0, | ||
| sleep: async (delayMs) => { | ||
| delays.push(delayMs); | ||
| }, | ||
| }); | ||
|
|
||
| expect(result).toBe("succeeded"); | ||
| expect(calls).toBe(3); | ||
| expect(delays).toEqual([5000, 10_000]); | ||
| }); | ||
|
|
||
| test("does not retry a non-transient failure", async () => { | ||
| let calls = 0; | ||
| let sleeps = 0; | ||
|
|
||
| await expect( | ||
| retryUntilReady({ | ||
| operation: async () => { | ||
| calls += 1; | ||
| throw new Error("access denied"); | ||
| }, | ||
| shouldRetry: () => false, | ||
| isClosed: () => false, | ||
| sleep: async () => { | ||
| sleeps += 1; | ||
| }, | ||
| }), | ||
| ).rejects.toThrow("access denied"); | ||
|
|
||
| expect(calls).toBe(1); | ||
| expect(sleeps).toBe(0); | ||
| }); | ||
|
|
||
| test("stops after shutdown during a retry delay", async () => { | ||
| let calls = 0; | ||
| let closed = false; | ||
|
|
||
| const result = await retryUntilReady({ | ||
| operation: async () => { | ||
| calls += 1; | ||
| throw new Error("ETIMEDOUT"); | ||
| }, | ||
| shouldRetry: () => true, | ||
| isClosed: () => closed, | ||
| random: () => 0, | ||
| sleep: async () => { | ||
| closed = true; | ||
| }, | ||
| }); | ||
|
|
||
| expect(result).toBe("closed"); | ||
| expect(calls).toBe(1); | ||
| }); | ||
|
|
||
| test("continues transient retries and escalates once", async () => { | ||
| let calls = 0; | ||
| let closed = false; | ||
| const retries: number[] = []; | ||
| const escalations: number[] = []; | ||
|
|
||
| const result = await retryUntilReady({ | ||
| operation: async () => { | ||
| calls += 1; | ||
| throw new Error("HTTP 503"); | ||
| }, | ||
| shouldRetry: () => true, | ||
| isClosed: () => closed, | ||
| random: () => 0, | ||
| onRetry: ({ attempt }) => { | ||
| retries.push(attempt); | ||
| }, | ||
| onEscalate: ({ attempt }) => { | ||
| escalations.push(attempt); | ||
| }, | ||
| sleep: async (_delayMs, isClosed) => { | ||
| if (retries.length === 11) { | ||
| closed = true; | ||
| } | ||
| expect(isClosed()).toBe(closed); | ||
| }, | ||
| }); | ||
|
|
||
| expect(result).toBe("closed"); | ||
| expect(calls).toBe(11); | ||
| expect(retries).toEqual( | ||
| Array.from({ length: 11 }, (_, index) => index + 1), | ||
| ); | ||
| expect(escalations).toEqual([10]); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| export const STARTUP_RETRY_INITIAL_DELAY_MS = 10_000; | ||
| export const STARTUP_RETRY_MAXIMUM_DELAY_MS = 300_000; | ||
| export const STARTUP_RETRY_ESCALATION_ATTEMPT = 10; | ||
|
|
||
| export type RetrySleep = ( | ||
| delayMs: number, | ||
| isClosed: () => boolean, | ||
| ) => Promise<void>; | ||
|
|
||
| export type StartupRetryFailure = { | ||
| readonly attempt: number; | ||
| readonly delayMs: number; | ||
| readonly error: unknown; | ||
| }; | ||
|
|
||
| export type StartupRetryInput = { | ||
| readonly operation: () => Promise<void>; | ||
| readonly shouldRetry: (error: unknown) => boolean; | ||
| readonly isClosed: () => boolean; | ||
| readonly sleep?: RetrySleep; | ||
| readonly random?: () => number; | ||
| readonly initialDelayMs?: number; | ||
| readonly maximumDelayMs?: number; | ||
| readonly onRetry?: (failure: StartupRetryFailure) => void; | ||
| readonly onEscalate?: (failure: StartupRetryFailure) => void; | ||
| }; | ||
|
|
||
| export type StartupRetryResult = "succeeded" | "closed"; | ||
|
|
||
| export function equalJitterRetryDelayMs( | ||
| attempt: number, | ||
| randomValue: number, | ||
| initialDelayMs = STARTUP_RETRY_INITIAL_DELAY_MS, | ||
| maximumDelayMs = STARTUP_RETRY_MAXIMUM_DELAY_MS, | ||
| ): number { | ||
| if (!Number.isSafeInteger(attempt) || attempt < 1) { | ||
| throw new Error( | ||
| `retry attempt must be a positive integer, got ${String(attempt)}`, | ||
| ); | ||
| } | ||
| if (randomValue < 0 || randomValue > 1 || !Number.isFinite(randomValue)) { | ||
| throw new Error( | ||
| `retry random value must be between 0 and 1, got ${String(randomValue)}`, | ||
| ); | ||
| } | ||
| if (initialDelayMs <= 0 || maximumDelayMs < initialDelayMs) { | ||
| throw new Error("retry delay bounds are invalid"); | ||
| } | ||
|
|
||
| const exponentialDelay = Math.min( | ||
| maximumDelayMs, | ||
| initialDelayMs * 2 ** (attempt - 1), | ||
| ); | ||
| return Math.round( | ||
| exponentialDelay / 2 + randomValue * (exponentialDelay / 2), | ||
| ); | ||
| } | ||
|
|
||
| export async function sleepUnlessClosed( | ||
| delayMs: number, | ||
| isClosed: () => boolean, | ||
| sleep: (delayMs: number) => Promise<void> = Bun.sleep, | ||
| ): Promise<void> { | ||
| const deadline = Date.now() + delayMs; | ||
| while (!isClosed() && Date.now() < deadline) { | ||
| const remainingMs = deadline - Date.now(); | ||
| await sleep(Math.min(remainingMs, 1000)); | ||
| } | ||
| } | ||
|
|
||
| export async function retryUntilReady( | ||
| input: StartupRetryInput, | ||
| ): Promise<StartupRetryResult> { | ||
| const sleep: RetrySleep = | ||
| input.sleep ?? | ||
| ((delayMs, isClosed) => sleepUnlessClosed(delayMs, isClosed)); | ||
| const random = input.random ?? Math.random; | ||
| let attempt = 0; | ||
|
|
||
| while (!input.isClosed()) { | ||
| try { | ||
| await input.operation(); | ||
| return "succeeded"; | ||
| } catch (error: unknown) { | ||
| if (input.isClosed()) { | ||
| return "closed"; | ||
| } | ||
| if (!input.shouldRetry(error)) { | ||
| throw error; | ||
| } | ||
|
|
||
| attempt += 1; | ||
| const failure: StartupRetryFailure = { | ||
| attempt, | ||
| delayMs: equalJitterRetryDelayMs( | ||
| attempt, | ||
| random(), | ||
| input.initialDelayMs, | ||
| input.maximumDelayMs, | ||
| ), | ||
| error, | ||
| }; | ||
| input.onRetry?.(failure); | ||
| if (attempt === STARTUP_RETRY_ESCALATION_ATTEMPT) { | ||
| input.onEscalate?.(failure); | ||
| } | ||
| await sleep(failure.delayMs, input.isClosed); | ||
| } | ||
| } | ||
|
|
||
| return "closed"; | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When SeaweedFS resets a connection, the Node/Bun HTTP stack can surface an error such as
Object.assign(new Error("socket hang up"), { code: "ECONNRESET" }); the code is not guaranteed to appear innameormessage. This predicate therefore returns false for a transient failure it explicitly intends to handle, causing the startup supervisor to stop after the first failed restoration and leaving the snapshot metric absent until the worker restarts. Parse and inspect the structuredcodefield (and, where applicable, the transport cause) rather than testing only rendered error text.Useful? React with 👍 / 👎.