e2b-dev / e2b-dev/runtime

orchestrator: FC metrics flusher goroutine may emit spurious Warn after Firecracker exit due to select race

Open Beginner friendly
#3,275 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
1.6k
Forks
438
PR merge metrics
No merged PRs in 30d

Description

Describe the bug

The periodic FC metrics flusher goroutine in packages/orchestrator/pkg/sandbox/fc/fc_metrics.go can emit a spurious "failed to flush fc metrics" Warn log after the Firecracker process has already exited, due to a Go select race between p.Exit.Done() and ticker.C.

Observed log line

level=warn msg="failed to flush fc metrics" error="error flushing fc metrics: Put \"http://localhost/actions\": read unix @->/data0/tmp/fc-<id>.sock: read: connection reset by peer"

Root cause

The flusher goroutine (fc_metrics.go line ~279):

go func() {
    ticker := time.NewTicker(metricsFlushInterval)
    defer ticker.Stop()
    for {
        select {
        case <-p.Exit.Done():
            return
        case <-ticker.C:
            if err := p.client.flushMetrics(ctx); err != nil {
                logger.L().Warn(ctx, "failed to flush fc metrics", zap.Error(err), ...)
            }
        }
    }
}()

When Firecracker exits, p.Exit.Done() is closed. If ticker.C fires at the same instant, Go's select picks one case at random. If it picks ticker.C, flushMetrics() is called against the now-dead FC Unix socket and gets connection reset by peer. The goroutine then loops back to select, sees Exit.Done() is closed, and returns — but the spurious Warn has already been emitted.

Under high load (many simultaneous VM exits), this race fires frequently and generates log noise that looks like a real FC API failure.

Expected behavior

No Warn log should be emitted after the Firecracker process has exited. The flusher should treat Exit.Done() as authoritative and skip the flush if the process is already gone.

Suggested fix

Add a non-blocking Exit.Done() check after ticker.C fires:

case <-ticker.C:
    // Guard against the race where Exit.Done() and ticker.C are both ready.
    select {
    case <-p.Exit.Done():
        return
    default:
    }
    if err := p.client.flushMetrics(ctx); err != nil {
        logger.L().Warn(ctx, "failed to flush fc metrics", zap.Error(err), ...)
    }

Environment

  • Bare-metal deployment with Nomad
  • Observed at high sandbox creation rates causing mass VM OOM-kills

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/orchestrator/pkg/sandbox/fc/fc_metrics.go around the flusher goroutine near line 279, and inspect how p.Exit.Done(), ticker.C, and flushMetrics are handled. Verify the exit signal remains authoritative when both cases are ready, and confirm that no spurious "failed to flush fc metrics" warning is emitted after Firecracker exits.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
infrastructure
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.