-
-
Notifications
You must be signed in to change notification settings - Fork 2
Finish scheduled maintenance safety checks #1880
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import * as v from "valibot"; | ||
|
|
||
| export const DenoAppIdentitySchema = v.object({ | ||
| id: v.string(), | ||
| slug: v.string(), | ||
| }); | ||
|
|
||
| export const DenoAppEnvVarsSchema = v.object({ | ||
| env_vars: v.array(v.object({ key: v.string() })), | ||
| }); | ||
|
|
||
| export const DenoRevisionStatusSchema = v.picklist([ | ||
| "skipped", | ||
| "queued", | ||
| "building", | ||
| "succeeded", | ||
| "failed", | ||
| ]); | ||
|
|
||
| export type DenoRevisionStatus = v.InferOutput<typeof DenoRevisionStatusSchema>; | ||
|
|
||
| export const DenoRevisionSchema = v.object({ | ||
| failure_reason: v.optional( | ||
| v.nullable(v.picklist(["error", "cancelled", "timed_out", "skipped"])), | ||
| null, | ||
| ), | ||
|
Comment on lines
+23
to
+26
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.
For Deno revision responses, Useful? React with 👍 / 👎. |
||
| id: v.string(), | ||
| status: DenoRevisionStatusSchema, | ||
| }); | ||
|
|
||
| export type DenoRevision = v.InferOutput<typeof DenoRevisionSchema>; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,19 +20,43 @@ export type MaintenanceTaskContext = { | |
| setCheckpoint: (checkpoint: string | null) => void; | ||
| }; | ||
|
|
||
| export interface MaintenanceTaskCheck { | ||
| enabled: () => boolean | Promise<boolean>; | ||
| maxDatabaseCalls: number; | ||
| maxExternalCalls: number; | ||
| settingsKeys: readonly string[]; | ||
| } | ||
|
|
||
| export interface MaintenanceTaskDeclaration { | ||
| check: MaintenanceTaskCheck; | ||
| deadlineMs: number; | ||
| enabled: () => boolean | Promise<boolean>; | ||
| failureRetryIntervalMs: number; | ||
| intervalMs: number; | ||
| maxDatabaseCalls: number; | ||
| maxExternalCalls: number; | ||
| name: string; | ||
| run: (context: MaintenanceTaskContext) => void | Promise<void>; | ||
| settingsKeys: readonly string[]; | ||
| wakePolicy: MaintenanceWakePolicy; | ||
| } | ||
|
|
||
| export const maintenanceStartupCalls = ( | ||
| tasks: readonly MaintenanceTaskDeclaration[], | ||
| ): { database: number; external: number; total: number } => { | ||
| if (tasks.length === 0) return { database: 0, external: 0, total: 0 }; | ||
| const settingsRead = tasks.some((task) => task.check.settingsKeys.length > 0) | ||
| ? 1 | ||
| : 0; | ||
| const database = | ||
| 1 + | ||
| settingsRead + | ||
| tasks.reduce((sum, task) => sum + task.check.maxDatabaseCalls, 0); | ||
|
Comment on lines
+49
to
+52
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.
For a maintenance task whose activation check has no settings keys, this counts only one database startup call, but Useful? React with 👍 / 👎. |
||
| const external = tasks.reduce( | ||
| (sum, task) => sum + task.check.maxExternalCalls, | ||
| 0, | ||
| ); | ||
| return { database, external, total: database + external }; | ||
| }; | ||
|
|
||
| const assertNonNegativeInteger = ( | ||
| task: MaintenanceTaskDeclaration, | ||
| value: number, | ||
|
|
@@ -72,6 +96,16 @@ const validateTask = (task: MaintenanceTaskDeclaration): void => { | |
| } | ||
| assertNonNegativeInteger(task, task.maxDatabaseCalls, "database"); | ||
| assertNonNegativeInteger(task, task.maxExternalCalls, "external"); | ||
| assertNonNegativeInteger( | ||
| task, | ||
| task.check.maxDatabaseCalls, | ||
| "enabled-check database", | ||
| ); | ||
| assertNonNegativeInteger( | ||
| task, | ||
| task.check.maxExternalCalls, | ||
| "enabled-check external", | ||
| ); | ||
| const calls = task.maxDatabaseCalls + task.maxExternalCalls; | ||
| if (calls > MAINTENANCE_TASK_CALL_LIMIT) { | ||
| throw new Error( | ||
|
|
@@ -93,6 +127,12 @@ export const defineMaintenanceTasks = < | |
| names.add(task.name); | ||
| validateTask(task); | ||
| } | ||
| const startup = maintenanceStartupCalls(tasks); | ||
| if (startup.total > MAINTENANCE_REQUEST_CALL_LIMIT) { | ||
| throw new Error( | ||
| `Maintenance checks declare ${startup.total} startup calls; maximum is ${MAINTENANCE_REQUEST_CALL_LIMIT}`, | ||
| ); | ||
| } | ||
| return tasks; | ||
| }; | ||
|
|
||
|
|
||
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 Deno accepts a revision but leaves it queued or building for more than 20 seconds, this hard timeout makes
publishSitereturn{ ok: false }even though the revision can still finish and route afterwards. In the builder flow the site has already been retained beforepublishSiteruns, so operators can see a failed build with a recorded site that may later go live; either avoid treating an accepted pending revision as a synchronous failure or move the wait to a background status path.Useful? React with 👍 / 👎.