mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-12 16:02:56 +00:00
fix: settle fork polls by the fork's existence, adopt in-flight creations
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
492a342e47
commit
fd032a16c5
@@ -45,6 +45,13 @@ async fn test_fork_created_in_background(db: Pool<Postgres>) -> anyhow::Result<(
|
||||
assert_eq!(resp.status(), 200, "{}", resp.text().await?);
|
||||
let status = wait_for_fork(&client, &base_url, "wm-fork-bg-ok").await;
|
||||
assert_eq!(status, json!({ "status": "completed" }));
|
||||
// A member who neither started the fork nor administers the workspace cannot read its outcome.
|
||||
let resp = client
|
||||
.get(format!("{base_url}/fork_creation_status/wm-fork-bg-ok"))
|
||||
.header("Authorization", "Bearer SECRET_TOKEN_2")
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 404);
|
||||
let exists: bool =
|
||||
sqlx::query_scalar("SELECT EXISTS(SELECT 1 FROM workspace WHERE id = 'wm-fork-bg-ok')")
|
||||
.fetch_one(&db)
|
||||
|
||||
@@ -9050,7 +9050,7 @@ async fn create_workspace_fork(
|
||||
let response_fork_id = fork_id.clone();
|
||||
sqlx::query(
|
||||
"DELETE FROM workspace_fork_creation
|
||||
WHERE finished_at < now() - interval '7 days'",
|
||||
WHERE COALESCE(finished_at, heartbeat_at) < now() - interval '7 days'",
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
|
||||
@@ -23,14 +23,17 @@ import {
|
||||
} from "../../core/conf.ts";
|
||||
|
||||
/**
|
||||
* Wait for a fork requested with `background: true`. A server without background forks ignores
|
||||
* the flag and has already created the fork when the request returns; it has no status route.
|
||||
* Wait for a fork requested with `background: true`. A poll that fails says nothing about the
|
||||
* fork: it can be lost to a proxy, or reach a server without background forks (an older version,
|
||||
* possibly one replica during a rolling deploy), which ignored the flag and created the fork
|
||||
* before answering.
|
||||
*/
|
||||
async function waitForForkCreation(parentWorkspace: string, forkId: string) {
|
||||
let failedPolls = 0;
|
||||
while (true) {
|
||||
await new Promise((resolve) => setTimeout(resolve, 1500));
|
||||
let result: Awaited<ReturnType<typeof wmill.getForkCreationStatus>>;
|
||||
let result:
|
||||
| Awaited<ReturnType<typeof wmill.getForkCreationStatus>>
|
||||
| undefined;
|
||||
try {
|
||||
result = await wmill.getForkCreationStatus({
|
||||
workspace: parentWorkspace,
|
||||
@@ -38,15 +41,17 @@ async function waitForForkCreation(parentWorkspace: string, forkId: string) {
|
||||
});
|
||||
failedPolls = 0;
|
||||
} catch (e) {
|
||||
if ((e as { status?: number }).status === 404) return;
|
||||
// A poll lost to a proxy says nothing about the fork, which is still being created.
|
||||
if (++failedPolls >= 5) throw e;
|
||||
continue;
|
||||
const exists = await wmill
|
||||
.existsWorkspace({ requestBody: { id: forkId } })
|
||||
.catch(() => false);
|
||||
if (exists) return;
|
||||
if (++failedPolls >= 10) throw e;
|
||||
}
|
||||
if (result.status === "completed") return;
|
||||
if (result.status === "failed") {
|
||||
if (result?.status === "completed") return;
|
||||
if (result?.status === "failed") {
|
||||
throw new Error(result.error ?? "Unknown error");
|
||||
}
|
||||
await new Promise((resolve) => setTimeout(resolve, 1500));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -404,16 +409,21 @@ async function createWorkspaceFork(
|
||||
|
||||
// --- Create the fork workspace ---
|
||||
try {
|
||||
await wmill.createWorkspaceFork({
|
||||
workspace: workspace.workspaceId,
|
||||
background: true,
|
||||
requestBody: {
|
||||
id: trueWorkspaceId,
|
||||
name: opts.createWorkspaceName ?? workspaceName ?? trueWorkspaceId,
|
||||
color: forkColor,
|
||||
forked_datatables: forkedDatatables,
|
||||
},
|
||||
});
|
||||
await wmill
|
||||
.createWorkspaceFork({
|
||||
workspace: workspace.workspaceId,
|
||||
background: true,
|
||||
requestBody: {
|
||||
id: trueWorkspaceId,
|
||||
name: opts.createWorkspaceName ?? workspaceName ?? trueWorkspaceId,
|
||||
color: forkColor,
|
||||
forked_datatables: forkedDatatables,
|
||||
},
|
||||
})
|
||||
.catch((e) => {
|
||||
// Started by an earlier run: wait for that creation instead.
|
||||
if (!String((e as { body?: unknown })?.body ?? "").includes("is already being created")) throw e;
|
||||
});
|
||||
await waitForForkCreation(workspace.workspaceId, trueWorkspaceId);
|
||||
|
||||
log.info(colors.green(`✅ Created forked workspace ${trueWorkspaceId}`));
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
vi.mock('$lib/gen', () => ({
|
||||
WorkspaceService: {
|
||||
createWorkspaceFork: vi.fn(),
|
||||
getForkCreationStatus: vi.fn(),
|
||||
existsWorkspace: vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
import { WorkspaceService } from '$lib/gen'
|
||||
import { createWorkspaceForkAndWait } from './forkCreation'
|
||||
|
||||
const svc = vi.mocked(WorkspaceService)
|
||||
const fork = { id: 'wm-fork-x', name: 'x' }
|
||||
|
||||
async function run() {
|
||||
const p = createWorkspaceForkAndWait('ws', fork)
|
||||
const settled = p.then(
|
||||
() => 'completed',
|
||||
(e) => `failed: ${e.message}`
|
||||
)
|
||||
await vi.runAllTimersAsync()
|
||||
return settled
|
||||
}
|
||||
|
||||
describe('createWorkspaceForkAndWait', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
vi.resetAllMocks()
|
||||
svc.createWorkspaceFork.mockResolvedValue('started')
|
||||
svc.existsWorkspace.mockResolvedValue(false)
|
||||
})
|
||||
afterEach(() => vi.useRealTimers())
|
||||
|
||||
it('waits while the fork is running and reports how it ended', async () => {
|
||||
svc.getForkCreationStatus
|
||||
.mockResolvedValueOnce({ status: 'running' })
|
||||
.mockResolvedValueOnce({ status: 'failed', error: 'boom' })
|
||||
expect(await run()).toBe('failed: boom')
|
||||
expect(svc.getForkCreationStatus).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('takes a failed poll for success once the fork exists', async () => {
|
||||
// A server without background forks created it synchronously and has no status route.
|
||||
svc.getForkCreationStatus.mockRejectedValue(
|
||||
Object.assign(new Error('Not Found'), { status: 404 })
|
||||
)
|
||||
svc.existsWorkspace.mockResolvedValue(true)
|
||||
expect(await run()).toBe('completed')
|
||||
})
|
||||
|
||||
it('gives up after repeated failed polls for a fork that never appears', async () => {
|
||||
svc.getForkCreationStatus.mockRejectedValue(new Error('upstream request timeout'))
|
||||
expect(await run()).toBe('failed: upstream request timeout')
|
||||
expect(svc.getForkCreationStatus).toHaveBeenCalledTimes(10)
|
||||
})
|
||||
|
||||
it('waits for a creation an earlier request already started', async () => {
|
||||
svc.createWorkspaceFork.mockRejectedValue({
|
||||
body: "Bad request: workspace 'wm-fork-x' is already being created"
|
||||
})
|
||||
svc.getForkCreationStatus.mockResolvedValue({ status: 'completed' })
|
||||
expect(await run()).toBe('completed')
|
||||
})
|
||||
})
|
||||
@@ -1,7 +1,7 @@
|
||||
import { WorkspaceService, type CreateWorkspaceFork } from '$lib/gen'
|
||||
|
||||
const POLL_INTERVAL_MS = 1500
|
||||
const MAX_FAILED_POLLS = 5
|
||||
const MAX_FAILED_POLLS = 10
|
||||
|
||||
/**
|
||||
* Create a fork and resolve once it exists, rejecting with the reason it could not be created.
|
||||
@@ -13,16 +13,22 @@ export async function createWorkspaceForkAndWait(
|
||||
parentWorkspace: string,
|
||||
fork: CreateWorkspaceFork
|
||||
): Promise<void> {
|
||||
await WorkspaceService.createWorkspaceFork({
|
||||
workspace: parentWorkspace,
|
||||
background: true,
|
||||
requestBody: fork
|
||||
})
|
||||
// A poll lost to the same proxy says nothing about the fork, which is still being created.
|
||||
try {
|
||||
await WorkspaceService.createWorkspaceFork({
|
||||
workspace: parentWorkspace,
|
||||
background: true,
|
||||
requestBody: fork
|
||||
})
|
||||
} catch (e) {
|
||||
// A retry of a request whose response was lost: wait for the creation it started.
|
||||
if (!String(e?.body ?? '').includes('is already being created')) throw e
|
||||
}
|
||||
// A poll that fails says nothing about the fork: it can be lost to the same proxy, or reach a
|
||||
// server without background forks (one of an older version during a rolling deploy), which
|
||||
// ignored the flag and created the fork before answering.
|
||||
let failedPolls = 0
|
||||
while (true) {
|
||||
await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL_MS))
|
||||
let result: Awaited<ReturnType<typeof WorkspaceService.getForkCreationStatus>>
|
||||
let result: Awaited<ReturnType<typeof WorkspaceService.getForkCreationStatus>> | undefined
|
||||
try {
|
||||
result = await WorkspaceService.getForkCreationStatus({
|
||||
workspace: parentWorkspace,
|
||||
@@ -30,10 +36,19 @@ export async function createWorkspaceForkAndWait(
|
||||
})
|
||||
failedPolls = 0
|
||||
} catch (e) {
|
||||
if (await forkExists(fork.id)) return
|
||||
if (++failedPolls >= MAX_FAILED_POLLS) throw e
|
||||
continue
|
||||
}
|
||||
if (result.status === 'completed') return
|
||||
if (result.status === 'failed') throw new Error(result.error ?? 'Unknown error')
|
||||
if (result?.status === 'completed') return
|
||||
if (result?.status === 'failed') throw new Error(result.error ?? 'Unknown error')
|
||||
await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL_MS))
|
||||
}
|
||||
}
|
||||
|
||||
async function forkExists(id: string): Promise<boolean> {
|
||||
try {
|
||||
return await WorkspaceService.existsWorkspace({ requestBody: { id } })
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user