Skip to content

fix(orchestrator): guard route cleanup by lifecycle owner - #3743

Open
tttboy123 wants to merge 1 commit into
e2b-dev:mainfrom
tttboy123:fix/routing-owner-cleanup-3695
Open

tttboy123 wants to merge 1 commit into
e2b-dev:mainfrom
tttboy123:fix/routing-owner-cleanup-3695

Conversation

@tttboy123

Copy link
Copy Markdown
Contributor

Summary

Refs #3695. This is a focused routing-cleanup safety fix found while investigating stale routes, not a complete fix for hard-node-loss recovery.

The publisher currently deletes by execution ID alone. A replacement VM lifecycle can retain the execution ID (for example, a resume-fresh checkpoint), so delayed cleanup from the old lifecycle can delete the replacement route. The publisher also only includes lifecycle_id when network placement supplies it.

  • Publish the actual sandbox lifecycle ID for every network mode.
  • Delete atomically in Redis only when the nonempty execution, lifecycle, and orchestrator process-instance IDs all match.
  • Preserve the record's exact value and expiry when ownership differs or cannot be verified.
  • Keep legacy records readable and execution-only deletion available for legacy records; refuse execution-only deletion of lifecycle-bearing records.
  • Distinguish deleted, absent, superseded, unverifiable, and error cleanup metrics. Existing stop/publish serialization is retained.

Validation

The new assertions fail against the original catalog/publisher behavior and pass with this patch. Final publication validation uses Go 1.26.10 on Linux/arm64, real isolated Redis 8.2.2, and golangci-lint 2.14.0 with the full repository configuration:

go test -race -p 1 -shuffle=20261024 -count=10 -timeout=3m \
  ./packages/shared/pkg/sandbox-catalog ./packages/orchestrator/pkg/routing
# Run inside each respective module:
golangci-lint run --timeout=4m ./pkg/sandbox-catalog
golangci-lint run --timeout=4m ./pkg/routing

Coverage includes exact-owner deletion, absent-key idempotence, each identity mismatch, same-execution replacement, malformed/missing/empty owner fields, Redis errors, legacy compatibility, unchanged raw value/absolute expiry on rejection, stop-before-insert, in-flight/ambiguous store outcomes, cleanup metrics, and the real Publisher-to-Redis old-stop/new-stop sequence.

Scope and rollout boundaries

  • This does not invalidate stale hits on dial failure, delete running state, create/recover VMs, shorten TTLs, or replay requests. Stale sandbox routing record after ungraceful node loss causes persistent 502 until TTL (max_length_in_hours) #3695 should remain open for the recovery design.
  • Store/refresh writes are still unconditional; this is cleanup ownership checking, not write fencing or proof that an old VM has died.
  • Older deployed binaries retain their old Lua script. Publishers must be upgraded; mixed-version deployment is not fenced by this change.
  • No dependency versions or module manifests are changed. The temporary local workspace checksum overlay only supplies an existing dependency checksum.
  • Validation is component-level, not a whole-monorepo or KVM/fault-injection claim. The separate API error-classification experiment is excluded from this PR.

This branch has not been deployed

No deployments
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.

1 participant