Repository navigation
Conversation
…objects When a build object is permanently absent from the store, the Linux NBD kernel driver retries the failing block read in a tight loop. Each retry reaches DiffStore.GetOrCreate, which does not cache negative results, so every retry triggers a fresh object-store 404 lookup - producing a high-frequency ERROR log flood and burning object-store quota. - Negative cache in GetOrCreate: store a short-lived (60s) negative entry when create returns storage.ErrObjectNotExist. Subsequent calls for the same key short-circuit immediately without hitting storage. The TTL allows recovery if the build is re-uploaded; Add clears the entry so a fresh upload is visible at once. A double-check inside the singleflight closure, plus a positive-cache recheck before installing the negative entry, prevent a race with a concurrent Add shadowing an available diff. - Log-level split in the nbd dispatcher: ErrObjectNotExist (permanent, expected when a build is gone) is logged at WARN with a clearer message; transient backend errors stay at ERROR, keeping dashboards and alerting focused on actionable signals. Rebased onto current main (the diff cache and NBD dispatch were refactored since the original attempt). Fixes e2b-dev#3294.
AdaAibaby
requested review from
ValentaTomas,
dobrac and
jakubno
as code owners
October 22, 2026 02:51
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
Stops the NBD read flood (high-frequency
ERRORlogs + repeated object-store 404s) that occurs when a build object is permanently missing from the store and the Linux NBD kernel driver retries the failing block read in a tight loop.Rebased replacement for the earlier attempt (#3295), which was closed as no-longer-applying after the diff cache / NBD dispatch refactors. This PR is written against current
main; the root causes it fixes are still present there.Fixes #3294.
Root cause (unchanged, still on main)
DiffStore.GetOrCreatede-duplicates concurrent calls withsingleflightbut does not cache failed results: whencreatereturnsstorage.ErrObjectNotExistthe error is discarded, so the next kernel retry callscreateagain → storage 404 →EIO→ kernel retry → … Anddispatch.gologs every read failure atERROR, so the expected-404 case floods dashboards.Changes
build/cache.go— negative cache. Oncreatereturningstorage.ErrObjectNotExist, install a short-lived (60s) entry in a dedicatedttlcache; subsequentGetOrCreatefor that key short-circuits without touching storage. The TTL lets a re-uploaded build recover;Addclears the entry immediately. Guards against a concurrentAdd: a double-check inside thesingleflightclosure, and a positive-cache recheck before installing the negative entry, so a diffAddpublished mid-probe is never shadowed.nbd/dispatch.go— log-level split.ErrObjectNotExist(permanent, expected when a build is gone) is logged atWARNwith a clearer message; transient backend errors stay atERROR.Test
Added 4 tests in
cache_test.go: negative cache short-circuits (create called once), non-ErrObjectNotExisterrors are not negatively cached (retried),Addclears the negative entry, and the Add-vs-probe race never shadows an available diff.go build,go vet,gofmt, and the fullpkg/sandbox/buildsuite pass./cc @ryan-sana PTAL when free, thanks.