Repository navigation
perf(desktop,host-service): enqueue workspace creation off pooled sockets and batch the host-service client #6304
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
aac6167
perf(desktop): batch the host-service tRPC client with httpBatchStrea…
Kitenite a0a9d78
feat(desktop,host-service): enqueue workspaces.create and settle over…
Kitenite a6f1506
test(host-service): assert createEnqueued returns before anything set…
Kitenite 12e7c0c
test(host-service): cover canonical-id/alreadyExists settling for enq…
Kitenite 55b8017
fix(desktop): harden enqueued-create recovery paths (review follow-ups)
Kitenite b197d2d
fix(desktop): tri-state row probe + spaced retries for create recovery
Kitenite 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
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 |
|---|---|---|
| @@ -1,9 +1,18 @@ | ||
| import { | ||
| getEventBus, | ||
| type WorkspaceCreateSettledPayload, | ||
| } from "@superset/workspace-client"; | ||
| import { TRPCClientError } from "@trpc/client"; | ||
| import { useCallback } from "react"; | ||
| import { resolveHostUrl } from "renderer/hooks/host-service/useHostTargetUrl"; | ||
| import { useRelayUrl } from "renderer/hooks/useRelayUrl"; | ||
| import { authClient } from "renderer/lib/auth-client"; | ||
| import { electronTrpc } from "renderer/lib/electron-trpc"; | ||
| import { getHostServiceClientByUrl } from "renderer/lib/host-service-client"; | ||
| import { getHostServiceWsToken } from "renderer/lib/host-service-auth"; | ||
| import { | ||
| getHostServiceClientByUrl, | ||
| type HostServiceClient, | ||
| } from "renderer/lib/host-service-client"; | ||
| import { getHostServiceUnavailableMessage } from "renderer/lib/host-service-unavailable"; | ||
| import { electronTrpcClient } from "renderer/lib/trpc-client"; | ||
| import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider"; | ||
|
|
@@ -38,6 +47,179 @@ export interface UseWorkspaceCreatesApi { | |
| submit: (args: SubmitArgs) => SubmitHandle; | ||
| } | ||
|
|
||
| /** Sessions create a folder + git init — quick. A wedged transport must not | ||
| * hold the pending-create UI forever. */ | ||
| const SESSION_CREATE_TIMEOUT_MS = 30_000; | ||
| /** The enqueue call validates and returns — it does none of the git work. */ | ||
| const ENQUEUE_TIMEOUT_MS = 15_000; | ||
| /** Backstop for a lost `workspace:create-settled` event (host restart or WS | ||
| * drop mid-create). Sized for worktree adds on monorepo-scale repos. */ | ||
| const CREATE_SETTLE_TIMEOUT_MS = 10 * 60_000; | ||
|
|
||
| /** What the completed-create pipeline needs, from either transport. */ | ||
| interface CreateOutcome { | ||
| workspace: { id: string; projectId: string | null }; | ||
| terminals: Array<{ terminalId: string; label?: string }>; | ||
| agents: Array< | ||
| | { ok: true; kind: "terminal" | "chat"; sessionId: string; label: string } | ||
| | { ok: false; error: string } | ||
| >; | ||
| } | ||
|
|
||
| /** Older hosts don't have `workspaces.createEnqueued` yet. */ | ||
| function isMissingProcedureError(error: unknown): boolean { | ||
| return ( | ||
| error instanceof TRPCClientError && | ||
| /no procedure found on path/i.test(error.message) | ||
| ); | ||
| } | ||
|
|
||
| /** An `AbortSignal.timeout` firing — host reachable but slow, as opposed to | ||
| * unreachable (network errors reject differently). */ | ||
| function isTimeoutAbort(error: unknown): boolean { | ||
| const cause = error instanceof TRPCClientError ? error.cause : error; | ||
| return ( | ||
| cause instanceof DOMException && | ||
| (cause.name === "TimeoutError" || cause.name === "AbortError") | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Bounded row-existence probe: did the host actually create this workspace? | ||
| * "unknown" means the probe itself failed (transport/auth) — callers must not | ||
| * treat that as absence. | ||
| */ | ||
| async function probeWorkspaceRow( | ||
| client: HostServiceClient, | ||
| workspaceId: string, | ||
| ): Promise<"exists" | "absent" | "unknown"> { | ||
| try { | ||
| const existing = await client.workspace.get.query( | ||
| { id: workspaceId }, | ||
| { signal: AbortSignal.timeout(15_000) }, | ||
| ); | ||
| return existing != null ? "exists" : "absent"; | ||
| } catch (error) { | ||
| if (error instanceof TRPCClientError && error.data?.code === "NOT_FOUND") { | ||
| return "absent"; | ||
| } | ||
| return "unknown"; | ||
| } | ||
| } | ||
|
|
||
| /** A few spaced probes: a client-side timeout can race a create that hasn't | ||
| * inserted its row yet. Resolves "exists" as soon as any probe sees the row. */ | ||
| async function probeWorkspaceRowWithRetries( | ||
| client: HostServiceClient, | ||
| workspaceId: string, | ||
| attempts = 3, | ||
| delayMs = 4_000, | ||
| ): Promise<"exists" | "absent" | "unknown"> { | ||
| let last: "absent" | "unknown" = "absent"; | ||
| for (let attempt = 0; attempt < attempts; attempt++) { | ||
| if (attempt > 0) { | ||
| await new Promise((resolve) => setTimeout(resolve, delayMs)); | ||
| } | ||
| const result = await probeWorkspaceRow(client, workspaceId); | ||
| if (result === "exists") return result; | ||
| last = result; | ||
| } | ||
| return last; | ||
| } | ||
|
|
||
| /** | ||
| * Enqueue the create and resolve from the `workspace:create-settled` event. | ||
| * The synchronous `workspaces.create` mutation holds one of Chromium's six | ||
| * pooled sockets for the whole create (minutes on big repos) and exceeds the | ||
| * relay's 30s exchange cap outright; the enqueue variant returns immediately | ||
| * and the result arrives over the eventBus WebSocket instead. | ||
| */ | ||
| async function createViaEnqueue( | ||
| client: HostServiceClient, | ||
| hostUrl: string, | ||
| workspaceId: string, | ||
| payload: WorkspacesCreateInput, | ||
| ): Promise<CreateOutcome> { | ||
| const bus = getEventBus(hostUrl, () => getHostServiceWsToken(hostUrl)); | ||
| const releaseBus = bus.retain(); | ||
| let unsubscribe: () => void = () => {}; | ||
| try { | ||
| // Subscribe before enqueueing so the settled event can't slip past. | ||
| const settled = new Promise<WorkspaceCreateSettledPayload>((resolve) => { | ||
| unsubscribe = bus.on( | ||
| "workspace:create-settled", | ||
| workspaceId, | ||
| (_workspaceId, eventPayload) => resolve(eventPayload), | ||
| ); | ||
| }); | ||
|
|
||
| try { | ||
| await client.workspaces.createEnqueued.mutate(payload, { | ||
| signal: AbortSignal.timeout(ENQUEUE_TIMEOUT_MS), | ||
| }); | ||
| } catch (error) { | ||
| if (isMissingProcedureError(error)) { | ||
| // Legacy host: fall back to the long-held synchronous create. | ||
| const result = await client.workspaces.create.mutate(payload); | ||
| return result; | ||
| } | ||
| // A timed-out enqueue may still have reached the host (reachable but | ||
| // slow — e.g. a saturated socket pool). Hard-failing here could tear | ||
| // down a workspace the host is actually creating; fall through and | ||
| // let the settled event / backstop decide. Anything else (network | ||
| // error, rejection) is a definitive no-enqueue: fail now. | ||
| if (!isTimeoutAbort(error)) throw error; | ||
| } | ||
|
|
||
| let backstopTimer: ReturnType<typeof setTimeout> | undefined; | ||
| const outcome = await Promise.race([ | ||
| settled, | ||
| new Promise<"timeout">((resolve) => { | ||
| backstopTimer = setTimeout( | ||
| () => resolve("timeout"), | ||
| CREATE_SETTLE_TIMEOUT_MS, | ||
| ); | ||
| }), | ||
| ]).finally(() => clearTimeout(backstopTimer)); | ||
|
|
||
| if (outcome === "timeout") { | ||
| // The event was lost, not necessarily the create. If the row exists | ||
| // the workspace is real — recover with an EMPTY pane seed rather | ||
| // than tearing down a workspace the user can already see. Launched | ||
| // terminals/agents aren't recoverable without the event; the | ||
| // background-session auto-adopt path surfaces them on open. | ||
| const row = await probeWorkspaceRow(client, workspaceId); | ||
| if (row === "exists") { | ||
| return { | ||
| workspace: { id: workspaceId, projectId: payload.projectId }, | ||
| terminals: [], | ||
| agents: [], | ||
| }; | ||
| } | ||
| throw new Error( | ||
| row === "unknown" | ||
| ? "Timed out waiting for the host to create the workspace, and the host could not be reached to verify it" | ||
| : "Timed out waiting for the host to create the workspace", | ||
| ); | ||
| } | ||
|
|
||
| if (!outcome.ok) { | ||
| throw new Error(outcome.error ?? "Workspace creation failed"); | ||
| } | ||
| return { | ||
| workspace: { | ||
| id: outcome.canonicalWorkspaceId ?? workspaceId, | ||
| projectId: outcome.projectId, | ||
| }, | ||
| terminals: outcome.terminals, | ||
| agents: outcome.agents, | ||
| }; | ||
| } finally { | ||
| unsubscribe(); | ||
| releaseBus(); | ||
| } | ||
| } | ||
|
|
||
| export function useWorkspaceCreates(): UseWorkspaceCreatesApi { | ||
| const hostService = useLocalHostService(); | ||
| const { machineId, activeHostUrl } = hostService; | ||
|
|
@@ -138,14 +320,37 @@ export function useWorkspaceCreates(): UseWorkspaceCreatesApi { | |
| // agent behind the setup commands. On a cold cache the hook value is | ||
| // still undefined — resolve it directly so an early create can't | ||
| // silently skip the gate (failures fall back to default-off). | ||
| const createPromise = (async () => { | ||
| const createPromise: Promise<CreateOutcome> = (async () => { | ||
| const client = getHostServiceClientByUrl(hostUrl); | ||
| if (snapshot.projectId === null) { | ||
| // Sessions have no setup scripts, so no wait-for-setup gate. | ||
| // Sessions have no setup scripts, so no wait-for-setup gate — | ||
| // and no git worktree work, so the synchronous mutation stays. | ||
| const { projectId: _null, ...sessionInput } = snapshot; | ||
| const result = | ||
| await client.workspaces.createSession.mutate(sessionInput); | ||
| return { ...result, alreadyExists: false }; | ||
| try { | ||
| const result = await client.workspaces.createSession.mutate( | ||
| sessionInput, | ||
| { signal: AbortSignal.timeout(SESSION_CREATE_TIMEOUT_MS) }, | ||
| ); | ||
| return result; | ||
| } catch (error) { | ||
| // The abort cancels the client, not the host — a slow but | ||
| // ultimately successful session create must not be recorded | ||
| // as failed (the row would pop back via workspace:changed). | ||
| // Spaced probes cover a create that hasn't inserted its row | ||
| // yet; the empty pane seed is healed by session auto-adopt. | ||
| if ( | ||
| isTimeoutAbort(error) && | ||
| (await probeWorkspaceRowWithRetries(client, workspaceId)) === | ||
| "exists" | ||
| ) { | ||
| return { | ||
| workspace: { id: workspaceId, projectId: null }, | ||
| terminals: [], | ||
|
Contributor
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. P1: A timeout after row insertion but before launch completion writes an empty pane layout while host still creates requested terminal/agent sessions. Keep recovery pending until launch metadata is available, or provide a settled/recovery result containing those launches. Prompt for AI agents |
||
| agents: [], | ||
| }; | ||
| } | ||
| throw error; | ||
| } | ||
| } | ||
| let waitForSetup = waitForSetupBeforeAgent; | ||
| if (waitForSetup === undefined && snapshot.agents?.length) { | ||
|
|
@@ -158,7 +363,7 @@ export function useWorkspaceCreates(): UseWorkspaceCreatesApi { | |
| snapshot.agents?.length && waitForSetup | ||
| ? { ...snapshot, waitForSetupBeforeAgents: true } | ||
| : snapshot; | ||
| return client.workspaces.create.mutate(payload); | ||
| return createViaEnqueue(client, hostUrl, workspaceId, payload); | ||
| })(); | ||
|
|
||
| writeWorkspacePaneLayout( | ||
|
|
||
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.