Repository navigation
Conversation
GetExclusiveBuildsForTemplateDeletion had no Go caller, so deleting a template only soft-deleted the DB row and left its build artifacts (rootfs/kernel/FC snapshots) in storage forever. Fixes e2b-dev#3239. - SQL: JOIN envs for cluster_id (route DeleteBuild to the right cluster); joins raw envs, not active_envs, so it resolves after soft-delete. - Handler: startTemplateArtifactCleanup runs after the DB commit in a detached goroutine that is panic-safe (recover), deadline-bounded (2m), and fans out with bounded concurrency (errgroup SetLimit 8). Best-effort: nil-node builds skipped, per-build failures never abort siblings, outcomes counted via new metrics so orphaned storage is alertable. Storage errors never affect the delete response. - Logging uses the internal logger API (logger.WithError/WithBuildID); no raw zap import in the handler (adds logger.WithError helper). - processExclusiveBuilds is DB-free and unit-tested: nil-node skip, error non-fatal, nil-cluster fallback, empty input. Metrics: api.template.artifacts.deleted / .delete_failed. Note: generated sql.go hand-updated (ClusterID) since sqlc is not in CI. Snapshot-deletion artifact cleanup is deliberately a separate follow-up.
AdaAibaby
requested review from
ValentaTomas,
dobrac and
jakubno
as code owners
November 4, 2026 02:25
… reconciliation metrics Deleting a template now deletes its GCS/S3 build artifacts safely. Being DB-exclusive is NOT sufficient: intermediate cache layers are never written to the DB, so a build that is DB-exclusive to the doomed template can still be a header-mapping ancestor of another live build. Deleting its prefix orphans that ancestor and surfaces later as a silent, unrecoverable Input/output error. Changes: - Reachability guard folded into processExclusiveBuilds: a candidate is deleted only if it is NOT reachable from any live root's memfile/rootfs header closure. Fails closed (deletes nothing) when the reader is unconfigured, the live-root set can't be queried, or any live root's header can't be read. - Live-root set completion: GetLiveBuildRootsForTeamExcludingTemplate now UNIONs the team's in-progress/pending builds (status_group) on top of other active templates' assigned builds, closing the gap where a concurrent build reusing the doomed template's exclusive ancestor had no assignment row yet. - Reconciliation metrics so orphaned storage is derivable, not guessed: candidates = deleted + delete_failed + skipped_nil_node + guard_kept (success), or fail_closed>0 when nothing was deleted. fail_closed carries a low-cardinality reason (reader_unconfigured|live_roots_query|header_unreadable) and is the alert signal. deleted/failed carry a cluster attribute. team_id/build_id stay on logs/trace, not metric attributes (cardinality). - Keeps the panic-safe / deadline-bounded / bounded-fan-out background cleanup. Supersedes the separate reachability-guard PR, which had the guard but lacked the background hardening and reconciliation metrics. Tests: processExclusiveBuilds guard/skip/fail paths, fail-closed classification, and DB-backed live-root tests (in-progress-without-assignment is a root, soft-deleted/ready-unassigned excluded, team-scoped). go build/vet/gofmt clean.
A snapshot is backed by its own template, which deleteSnapshot already soft-deletes. Reuse the template-deletion artifact cleanup for the snapshot's template so its exclusively-owned build artifacts are removed from GCS/S3 instead of being orphaned. Stacked on the reachability-guarded template cleanup: the snapshot path inherits the same guard, reconciliation metrics, and panic-safe/bounded/best-effort background behavior. The guard is what makes the base-deleted direction safe — if another live build reaches a base layer now DB-exclusive to this snapshot-template, it is kept, not orphaned. teamID is threaded through to scope the live-root set the guard consults. Test: DB-backed TestSnapshotArtifactExclusivity proves the snapshot's own build and an extra exclusive build are returned for cleanup, a build shared with another template is excluded, and the query resolves after soft-delete.
AdaAibaby
force-pushed
the
fix/snapshot-artifact-cleanup
branch
from
November 5, 2026 08:38
24c8645 to
caccd1a
Compare
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Deletes the storage artifacts of a snapshot's exclusively-owned builds when the snapshot is deleted (
DELETE /sandboxes/{id}), instead of orphaning them in GCS/S3.A snapshot is backed by its own template, and
deleteSnapshotalready soft-deletes that template. This reuses the exact template-deletion cleanup for the snapshot's template, so the same guarantees apply.Inherits the reachability guard
Because it reuses #3797's
startTemplateArtifactCleanup, the snapshot path inherits the reachability guard, reconciliation metrics, and panic-safe/bounded/best-effort background behavior. The guard is what makes the base-deleted direction safe: if another live build reaches a base layer now DB-exclusive to this snapshot-template, it is kept, not orphaned.teamIDis threaded through to scope the live-root set.Test
DB-backed
TestSnapshotArtifactExclusivity: the snapshot's own build and an extra exclusive build are returned for cleanup; a build shared with another template is excluded; the query resolves after soft-delete; every returned build carries thecluster_node_idneeded to routeDeleteBuild.