Skip to content

fix(orchestrator): warn once for unset NFS cache path instead of on every build/launch - #3787

Open
AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/nfs-cache-path-warn-once
Open

AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/nfs-cache-path-warn-once

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

What

When an NFS-cache feature flag is enabled but the cache path is unset, the orchestrator logged WARN "NFSCache feature flag is enabled but cache path is not set" on the hot path — every template build, sandbox launch, and snapshot resume — while NFS caching is silently disabled. On a dev-default deployment (the three use-nfs-* flags default to env.IsDevelopment() = on) with no cache dir set, this floods the logs with hundreds of identical lines.

This emits the warning once per process and makes it state the consequence.

Refs #3786.

How

  • Add storage.WarnNFSCachePathUnsetOnce(ctx) in packages/shared/pkg/storage (already imported by both call sites, which call storage.WrapInNFSCache), guarded by sync.Once.
  • Replace the duplicated inline logger.L().Warn(...) in builder.go:304 (useNFSCache) and cache.go:1218 (useNFSCache) with a call to it.
  • Message now reads: "NFS cache feature flag is enabled but its cache path is unset; NFS caching is disabled until the shared chunk cache dir is configured" — the path is immutable config, so one line is enough.

Behavior is unchanged: caching is still disabled in this case; only the log frequency and wording change.

Test

TestWarnNFSCachePathUnsetOnce calls the helper 5× and asserts exactly one warning is emitted (zap observer). go test ./packages/shared/pkg/storage/ passes; the orchestrator build/sandbox/template packages build clean.

When an NFS-cache flag is on but the cache path is unset, builder.go and
cache.go each logged the same WARN on every build/launch/resume (hot path),
flooding logs while NFS caching is silently disabled. The path is immutable
config, so one line suffices. Add storage.WarnNFSCachePathUnsetOnce (sync.Once)
and call it from both sites; message now states caching is disabled. Behavior
unchanged. Adds a unit test.

Refs e2b-dev#3786

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant