fix(orchestrator): downgrade expected VM-exit cascade errors from ERROR to Warn#3277
fix(orchestrator): downgrade expected VM-exit cascade errors from ERROR to Warn#3277AdaAibaby wants to merge 2 commits into
Conversation
After a VM exits, in-flight NBD operations fail with context.Canceled and socket writes fail with "use of closed network connection" or "broken pipe". All of these were logged at ERROR level, generating noise that drowns out real failures. dispatch.go: - Downgrade "nbd backend read/write failed" to Warn when the error is context.Canceled or context.DeadlineExceeded. - Downgrade "nbd error cmd read/write" (fatal-channel-full path) to Warn; shutdown is already in progress at that point. fc_metrics.go: - Add a non-blocking Exit.Done() check after ticker.C fires to handle the Go select race where both channels become ready simultaneously. This prevents a spurious "failed to flush fc metrics" Warn log when Firecracker has already exited. Fixes: e2b-dev#3274 Fixes: e2b-dev#3275
There was a problem hiding this comment.
Code Review
This pull request introduces improvements to the orchestrator's sandbox metrics reader and NBD dispatch logging. Specifically, it adds a guard in the metrics reader to prevent a race condition during process exit, and downgrades expected errors (such as context cancellations during shutdown or response-write failures on a closing socket) from Error to Warn level in the NBD dispatch loop. I have no feedback to provide as there are no review comments to assess.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
Closes #3274
Closes #3275
After a Firecracker VM exits (normally or abnormally), two sources of spurious ERROR-level logs appear:
NBD dispatch (
dispatch.go): In-flight reads/writes fail withcontext.Canceledand response writes fail withuse of closed network connection/broken pipe. All were logged at ERROR, generating noise that drowns out real backend failures.FC metrics flusher (
fc_metrics.go): A Goselectrace betweenp.Exit.Done()andticker.Ccan causeflushMetrics()to be called against a dead Firecracker socket, emitting a spurious"failed to flush fc metrics: connection reset by peer"Warn log even after the process has exited.Changes
packages/orchestrator/pkg/sandbox/nbd/dispatch.gonbd backend read/write failed: downgrade to Warn whenerrors.Is(err, context.Canceled || context.DeadlineExceeded); keep ERROR for real backend failuresnbd error cmd read/write(fatal-channel-full path): downgrade to Warn; by the time this path is reached, shutdown is already in progresspackages/orchestrator/pkg/sandbox/fc/fc_metrics.gocase <-ticker.Cfires, add a non-blockingselect { case <-p.Exit.Done(): return; default: }guard to handle the race where both channels are simultaneously readyBefore / After
Before (every sandbox teardown with in-flight I/O):
After:
Real backend errors (non-canceled) still surface at ERROR level.
Test plan
kill -9while I/O is in-flight; confirm no ERROR logs fornbd backend read/write failedwithcontext canceled"failed to flush fc metrics"Warn disappears after VM exit