Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/api/internal/orchestrator/create_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,23 @@ func (o *Orchestrator) CreateSandbox(
o.maybeRemapResumeOriginNode(ctx, snapshotSandboxID, team, sbxData.NodeID, placed.WarmedNode)
}

// A create the request context cancelled mid-flight may still have
// completed on the node, leaving an instance the API never registered
// (no running-store record, no index, no catalog entry). Compensate
// immediately with a best-effort kill of this exact (id, execution)
// rather than waiting a full orphan grace period for reconcile to
// reclaim it. Detached from the cancelled request context.
if placed.InterruptedNode != nil {
o.compensateInterruptedCreate(
context.WithoutCancel(ctx),
placed.InterruptedNode,
sandboxID,
executionID,
sbxData.Build.Vcpu,
sbxData.Build.RamMb,
)
}

return sandbox.Sandbox{}, placementAPIError(err)
}

Expand Down
34 changes: 34 additions & 0 deletions packages/api/internal/orchestrator/delete_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ const refusalRetryAfter = 10 * time.Second

const pauseTimeout = 80 * time.Second

// interruptedCreateKillTimeout bounds the best-effort kill of an instance a
// node may have created for a request that was cancelled before registration.
const interruptedCreateKillTimeout = 30 * time.Second

func (o *Orchestrator) RemoveSandbox(ctx context.Context, teamID uuid.UUID, sandboxID string, opts sandbox.RemoveOpts) error {
ctx, span := tracer.Start(ctx, "remove-sandbox")
defer span.End()
Expand Down Expand Up @@ -395,6 +399,36 @@ func (o *Orchestrator) killOrphanSandbox(ctx context.Context, sbx sandbox.NodeSa
}
}

// compensateInterruptedCreate best-effort kills an instance a node may have
// created for a request whose context was cancelled before the API registered
// it. It runs the same node-side kill as the orphan reconciler, just eagerly on
// the failure path so the leak window is near-zero instead of an orphan grace
// period. A no-op on the node (NotFound) is handled by killSandboxOnNode, so a
// create the node never actually completed costs only a cheap delete RPC. The
// caller must pass a context detached from the cancelled request.
func (o *Orchestrator) compensateInterruptedCreate(ctx context.Context, node *nodemanager.Node, sandboxID, executionID string, vcpu, ramMB int64) {
ctx, cancel := context.WithTimeout(ctx, interruptedCreateKillTimeout)
defer cancel()

nodeSbx := sandbox.NodeSandbox{
SandboxID: sandboxID,
ExecutionID: executionID,
NodeID: node.ID,
ClusterID: node.ClusterID,
VCpu: vcpu,
RamMB: ramMB,
}

if err := o.killSandboxOnNode(ctx, node, nodeSbx, sandbox.KillReasonOrphaned); err != nil {
logger.L().Error(ctx, "Failed to compensate interrupted sandbox create on node",
zap.Error(err),
logger.WithSandboxID(sandboxID),
logger.WithNodeID(node.ID),
zap.String("kill_reason", sandbox.KillReasonOrphaned.String()),
)
}
}

func (o *Orchestrator) killSandboxOnNode(
ctx context.Context,
node *nodemanager.Node,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package placement

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/e2b-dev/infra/packages/api/internal/api"
"github.com/e2b-dev/infra/packages/api/internal/orchestrator/nodemanager"
)

// TestPlaceSandbox_InterruptedCreateReportsNode: when a node's SandboxCreate is
// interrupted by the request context being cancelled, that node is reported as
// InterruptedNode so the caller can compensate for an instance the node may
// have completed server-side (issue #3637).
func TestPlaceSandbox_InterruptedCreateReportsNode(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithCancel(t.Context())
defer cancel()

node := nodemanager.NewTestNode("node1", api.NodeStatusReady, 0, 4)
// Cancels the request context, then returns as the node create would when
// the deadline lands mid-flight (the orchestrator collapses this to Internal).
node.SetSandboxClient(erroringClient(cancel, status.Error(codes.Internal, "context canceled")))

result, err := PlaceSandbox(ctx, failIfCalled(t), []*nodemanager.Node{node}, node, testSbxRequest("test-sandbox"), CPURequirement{}, false, nil)

require.Error(t, err)
assert.True(t, result.TimedOut)
require.NotNil(t, result.InterruptedNode, "the interrupted node must be reported for compensation")
assert.Equal(t, node.ID, result.InterruptedNode.ID)
}

// TestPlaceSandbox_ResourceExhaustedInterruptNotCompensated: a node that refused
// with ResourceExhausted never started a create, so even when the deadline
// lands on it, it must NOT be reported as InterruptedNode — killing it would be
// a pointless RPC against a node that holds nothing.
func TestPlaceSandbox_ResourceExhaustedInterruptNotCompensated(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithCancel(t.Context())
defer cancel()

node := nodemanager.NewTestNode("node1", api.NodeStatusReady, 0, 4)
node.SetSandboxClient(erroringClient(cancel, status.Error(codes.ResourceExhausted, "no capacity")))

result, err := PlaceSandbox(ctx, failIfCalled(t), []*nodemanager.Node{node}, node, testSbxRequest("test-sandbox"), CPURequirement{}, false, nil)

require.Error(t, err)
assert.True(t, result.TimedOut)
assert.Nil(t, result.InterruptedNode, "a ResourceExhausted refusal must not be compensated")
}

// TestPlaceSandbox_HardFailureNoCancelNoInterruptedNode: a hard create failure
// while the context is still live is a genuine node failure, not an interrupt.
// It must not be reported for compensation (the node cleaned up itself).
func TestPlaceSandbox_HardFailureNoCancelNoInterruptedNode(t *testing.T) {
t.Parallel()

node := nodemanager.NewTestNode("node1", api.NodeStatusReady, 0, 4,
nodemanager.WithSandboxCreateError(status.Error(codes.Internal, "create failed")))

result, err := PlaceSandbox(t.Context(), failIfCalled(t), []*nodemanager.Node{node}, node, testSbxRequest("test-sandbox"), CPURequirement{}, false, nil)

require.Error(t, err)
assert.False(t, result.TimedOut)
assert.Nil(t, result.InterruptedNode)
}
26 changes: 25 additions & 1 deletion packages/api/internal/orchestrator/placement/placement.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ type PlacementResult struct {
WarmedNode *nodemanager.Node
// TimedOut reports whether placement failed due to context cancellation/deadline.
TimedOut bool
// InterruptedNode is the node whose in-flight SandboxCreate was interrupted
// by the request context being cancelled/timing out. The node may have
// completed the create server-side even though the RPC returned Canceled, so
// it can hold an instance the API never registered. Set only on such a
// failure; callers should issue a best-effort kill of the (sandboxID,
// executionID) on it to avoid leaving an orphan until reconcile reclaims it.
InterruptedNode *nodemanager.Node
// Response is the successful create's RPC response; nil on failure.
Response *orchestrator.SandboxCreateResponse
}
Expand Down Expand Up @@ -89,13 +96,21 @@ func placeSandbox(
// First node that attempted the create (not a fast ResourceExhausted refusal).
var firstTriedNode *nodemanager.Node

// Node whose in-flight SandboxCreate returned because the request context
// was cancelled/timed out. That node may still have completed the create
// server-side, so it is the one that can hold an unregistered instance.
var interruptedNode *nodemanager.Node

var lastCreateErr error

// failed reports the warming node only when the failure was caused by the
// request context being cancelled or timing out (ctx.Err() != nil). Hard
// failures (where the context is still live) carry no node, so callers never
// pin a retry to a node that genuinely refused the sandbox.
//
// It also surfaces the interrupted node so the caller can compensate for a
// create the node may have finished after the RPC was cancelled.
//
// TODO [EN-1099]: We key off ctx.Err() rather than the gRPC status code because
// the orchestrator currently collapses a timed-out resume into codes.Internal
// (it folds the deadline cause into the message, not the code),
Expand All @@ -105,7 +120,7 @@ func placeSandbox(
return PlacementResult{}, err
}

return PlacementResult{WarmedNode: firstTriedNode, TimedOut: true}, err
return PlacementResult{WarmedNode: firstTriedNode, InterruptedNode: interruptedNode, TimedOut: true}, err
}

attempt := 0
Expand Down Expand Up @@ -194,6 +209,15 @@ func placeSandbox(
firstTriedNode = failedNode
}

// If the request context was cancelled while this node's create was in
// flight, the node may have finished creating the instance even though
// the RPC returned. A ResourceExhausted refusal never started a create,
// so it cannot leak. Track the most recent such node as the compensation
// target; the loop exits on the next ctx.Done() check.
if ctx.Err() != nil && statusCode != codes.ResourceExhausted {
interruptedNode = failedNode
}

switch statusCode {
case codes.ResourceExhausted:
refusals++
Expand Down