Repository navigation
Conversation
…nt retries it Delete stops the sandbox in a goroutine and returns before Firecracker exits. The killed sandbox keeps its hugepages for up to about 0.5 s. A create in that window fails to mmap the guest memfd with ENOMEM. Create returned codes.Internal, so placement excluded the node and the API answered 500 on a single-node cluster. Return codes.ResourceExhausted when the create error is ENOMEM. Placement then retries the node with backoff, and the create lands when the pages come back. Run the start rollback before that refusal and release the sandbox ID at once. The rollback holds only the template pin at that point. Placement retries within a millisecond with the same sandbox ID, and an ID that is still reserved gives AlreadyExists, which excludes the node. The wait for uffd ready now wraps context.Cause instead of ctx.Err, so the ENOMEM also reaches Create when the uffd handler exits after Firecracker loaded the snapshot. Fixes e2b-dev#3689
|
We require contributors to sign our Contributor License Agreement, and we don't have @eren23 on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
Author
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
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.
Fixes #3689
A create right after a kill can fail with a 500 when the node's hugepage pool holds only one sandbox of that size. Delete returns before the killed Firecracker exits, and that process keeps its hugepages for up to about 0.5 s. The new sandbox then gets ENOMEM when the orchestrator maps its guest memfd. Create returned this as codes.Internal, so placement excluded the node and gave up.
What I changed
errors.Is(err, syscall.ENOMEM), not with the message text. Placement already treats ResourceExhausted as a refusal and retries the same node with backoff, so the create lands when the old process is gone.loadSnapshotwrappedctx.Err()while it waited for uffd ready, so a uffd exit after a good snapshot load became a plain context.Canceled. It now wrapscontext.Cause(ctx), and the ENOMEM reaches Create on that path too.I did not add a guard for a template that can never fit the pool. With this change such a create retries until the request deadline and ends as a 503. On my host that took 70 s and about 134 placement attempts. The other ResourceExhausted refusals in Create (max sandboxes, too many starting) work the same way, and
TestPlaceSandbox_DeadlineEndsRefusalBackoffcovers that path. A guard would need the pool size in the server, which it does not have today. The placement hugepage check from 16ec5e6 already skips such a node when it is on. I can add a guard if you prefer one.How I tested
TestLoadSnapshotKeepsUffdExitCausefails on main withcontext canceled while waiting for uffd ready: context canceledand passes with the change, 30 of 30 runs.TestMemoryRefusalchecks that the error chain from the lab log gives ResourceExhausted, and that a plain error with the same text does not.go test -race -count=1 ./pkg/server/ ./pkg/sandbox/fc/in a linux/arm64 container with Go 1.26.8. All 231 tests pass.golangci-lint runandgolangci-lint fmt --diff(v2.14.0) on both packages report no issues.The existing placement tests
TestPlaceSandbox_RefusedNodeIsRetriedandTestPlaceSandbox_DeadlineEndsRefusalBackoffpass.I built three orchestrators from 7546658, swapped only the binary on one Embed host and ran the same cycles on each. The host runs compose 311f9fc with nested KVM, and its hugepage pool fits one 8 GiB sandbox. The middle column is this change without step 2.
The numbers count creates that worked on the first API call. The one 500 without step 2 is the race above. The retry came 4 ms after the refusal and got AlreadyExists. With this PR the 6 GiB case was ready 0.56 s after the kill (median) with no retry in the client. On main the client had to retry after the 500, and it was ready after 1.00 s. After each case all hugepages were free, no Firecracker process ran and the API listed no sandbox, on all three binaries.