Skip to content

fix(api): compensate a create the node finished after the request was cancelled - #3643

Open
AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/create-cancel-orphan-compensation
Open

AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/create-cancel-orphan-compensation

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Problem

If a sandbox create request is cancelled or times out on the client side while the node-side create is still in progress, the node can finish creating the instance even though the gRPC create returns Canceled. The API classifies placement as timed out and returns the error without ever registering the sandbox. The instance is left running on the node with no running-store record, no team index entry, and no catalog entry.

An existing safety net reclaims it — Store.Reconcile → killOrphanSandbox kills instances present on the node but absent from the store — but only after orphanGracePeriod (1 minute). During that window a VM and its network slot run completely unregistered and invisible to the API, and the create failure path itself performs no compensation.

Fixes #3637.

Root cause

The node-side create and the API-side registration are separate steps, and the only compensation (removeSandboxFromNode) runs on the sandboxStore.Add failure branch — i.e. after a successful placement. When placement.PlaceSandbox returns because the request context was cancelled, CreateSandbox returns placementAPIError(err) immediately; the placement-failure branch never compensated. The gRPC create already dispatched to the node is not cancelled atomically with the API request, so the node completes it.

Fix

Compensate on the placement-failure branch, targeting the exact node that leaked:

  • placeSandbox now records the node whose in-flight SandboxCreate was interrupted by the context being cancelled/timing out, and returns it as PlacementResult.InterruptedNode. A ResourceExhausted refusal never started a create, so it is explicitly skipped — only a node that actually began a create can hold an instance.
  • On the placement-failure branch, CreateSandbox issues a best-effort kill of that exact (sandboxID, executionID) on InterruptedNode, using a context detached from the cancelled request (the request context is already dead).

Why this shape

  • Precise, not blanket. The issue suggests recording the intended id and compensating on any failure. This pins compensation to the one node whose create was actually interrupted, rather than firstTriedNode (which is tracked for retry-warming and can differ in a multi-node retry) or every node tried. A hard-failed node (context still live) cleaned up itself and is left alone.
  • Reuses the existing kill path. compensateInterruptedCreate runs the same killSandboxOnNode the orphan reconciler uses — just eagerly. An unregistered sandbox has no catalog entry, so the heavier removeSandboxFromNode (which deletes a routing entry) is unnecessary. killSandboxOnNode already treats NotFound as success, so if the node never actually completed the create, the kill is a cheap no-op.
  • Reconcile stays as the backstop. This shrinks the leak window from an orphan grace period (~1 min) to ~0 for the common case, without removing the periodic safety net that also covers API crashes.

The alternative in the issue — making node-side create self-cleaning within a bounded window if the caller never confirms registration — is more robust to an API crash mid-create, but it is a node/protocol change with a much larger blast radius, and reconcile already covers the crash case. Not pursued here.

Testing

  • placement/interrupted_create_test.go — new: an interrupted create reports the node as InterruptedNode; a ResourceExhausted refusal does not (would be a pointless kill); a hard failure with a live context does not (genuine node failure, self-cleaned).
  • go build ./..., go vet, full placement package tests, and golangci-lint (v2, pinned) all clean.
  • Reproduced the bug's failure signature on a dev cluster: a create cancelled mid-flight logs Failed to create sandbox … "[Canceled] context canceled" / failed to place sandbox: request timed out, returns HTTP 499, and the sandbox is absent from the running store and team index (GET 404) — the exact conditions this fix now compensates.

… cancelled

When a create request is cancelled or times out on the client side while the
node-side create is still running, the node can finish creating the instance
even though the gRPC create returns Canceled. The API classifies placement as
timed out and returns the error without ever registering the sandbox, so the
instance is left running on the node with no running-store record, no team
index entry, and no catalog entry. The periodic reconcile reclaims it, but only
after a full orphan grace period, and the create failure path itself does
nothing.

The compensation that removes a node instance only runs on the
sandboxStore.Add failure branch, i.e. after a successful placement. The
placement-failure branch never compensated.

placeSandbox now remembers the node whose in-flight create was interrupted by
the context being cancelled (a ResourceExhausted refusal never started a
create, so it is skipped) and returns it as PlacementResult.InterruptedNode.
On the placement-failure branch, CreateSandbox issues a best-effort kill of
that exact (sandboxID, executionID) on that node, detached from the cancelled
request context. It reuses the same node-side kill the orphan reconciler runs,
just eagerly, so the leak window drops from an orphan grace period to ~0.
killSandboxOnNode already treats NotFound as success, so if the node never
actually completed the create the kill is a cheap no-op. Reconcile stays as the
backstop.

Fixes e2b-dev#3637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cancelled/timed-out sandbox create leaves an unregistered orphan instance on the node

2 participants