session: make healthcheck tolerance configurable via env vars#33
Open
Trent-TSE wants to merge 5 commits into
Open
session: make healthcheck tolerance configurable via env vars#33Trent-TSE wants to merge 5 commits into
Trent-TSE wants to merge 5 commits into
Conversation
Under severe memory pressure (e.g. a build maxing out RAM and driving iowait >90%), the buildx client can stall long enough to miss two consecutive healthchecks, causing buildkitd to drop the session that carries registry credentials. The subsequent push then fails with 'no active session ... context deadline exceeded'. Allow tuning the session healthcheck interval, timeout, and consecutive failure threshold via BUILDKIT_SESSION_HEALTHCHECK_INTERVAL, BUILDKIT_SESSION_HEALTHCHECK_TIMEOUT, and BUILDKIT_SESSION_HEALTHCHECK_MAX_FAILURES. Defaults are unchanged. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f704ffd. Configure here.
The per-check cancel left the shadowed loop context done, so the error branch returned on the first failed healthcheck and the failure tolerance never applied. Use a dedicated checkCtx for the rpc so only monitor shutdown short-circuits the loop. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
js-yaml 5.x throws on empty input; the unpinned npm install now pulls 5.x, breaking test/prepare for callers that pass no includes. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Port of moby#6925; the stale VAGRANT_DISABLE_STRICT_DEPENDENCY_ENFORCEMENT now causes a gem conflict when installing vagrant-libvirt on the freebsd test job. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The generic/freebsd14 box tracks the EOL 14.0 release whose release_2 pkg repo has been removed, breaking the freebsd test job. Upstream switched to a pinned bento/freebsd-14 box. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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
1 participant
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
On memory-constrained runners a build can drive the VM into page-cache thrashing (observed: sidestream-tech
build linux/arm64on 4vcpu ARM — 0.04GB mem available, 93% iowait for ~4 min). The stalled buildx client then misses two consecutive session healthchecks and buildkitd tears down the session that carries registry credentials, so the eventual push fails withno active session for <id>: context deadline exceeded(session/auth/auth.go:104).This makes the daemon-side session healthcheck tunable via env vars, defaults unchanged:
BUILDKIT_SESSION_HEALTHCHECK_INTERVAL(default5s)BUILDKIT_SESSION_HEALTHCHECK_TIMEOUT(default30s)BUILDKIT_SESSION_HEALTHCHECK_MAX_FAILURES(default2)monitorHealthnow countsconsecutiveFailuresagainstmaxFailuresinstead of the booleanfailedBefore(2-strike) logic; a config is parsed once viasync.OnceValue. Invalid values log a warning and fall back to defaults.setup-docker-builder will set generous values through its existing
env.*driver-opt pass-through so cold-cache builds that temporarily max out memory can survive to the push.Verified with
go build ./session/andgo test -vet=off ./session/...(pass;go vetfailures onWithTimeoutCauseare pre-existing upstream nolint'd lines).Link to Devin session: https://app.devin.ai/sessions/47b6abfcecd948dcbae7d55badefa46c
Requested by: @Trent-TSE
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled. (Staging)