perf(ci): isolate bottlecap build-directory cache per concurrent build - #1369
lucaspimentel wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
The added full-line comments are embedded inside continued RUN --mount=... instructions and can be interpreted as part of the shell command on some Dockerfile frontends, risking broken builds or dropping the intended mount behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR aims to reduce CI wall time for Bottlecap compile jobs by preventing concurrent builds from contending on Cargo’s shared target build-directory lock when using BuildKit cache mounts.
Changes:
- Updates the
targetBuildKit cache mount to usesharing=privatein the AL2 compile Dockerfile. - Updates the
targetBuildKit cache mount to usesharing=privatein the Alpine compile Dockerfile.
File summaries
| File | Description |
|---|---|
| images/Dockerfile.bottlecap.compile | Sets the target cache mount to sharing=private to avoid cross-job contention on Cargo’s build-directory lock. |
| images/Dockerfile.bottlecap.alpine.compile | Same sharing=private change for the Alpine-based compile image to isolate concurrent builds. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
🔗 Commit SHA: 76a8c01 | Docs | View more details | Give us feedback! |
1253e16 to
ae742dd
Compare
|
@DataDog review |
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
BuildKit private cache mounts give each concurrent compile job a separate Cargo target directory. The change keeps the build output path and the shared registry and Git caches unchanged.
🤖 Datadog Autotest · Commit ae742dd · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
The target cache mount in the compile Dockerfiles had no sharing mode, so concurrent bottlecap compile jobs using the same Dockerfile shared one cache and serialized on cargo's build-directory lock. Measured across 240 compile attempts on 30 main pipelines: 14.2% of attempts waited on the lock (median wait 4.0m, max 13.1m), adding 2.8m to the median job duration when hit. sharing=private gives each concurrent build its own target cache instead of blocking. Cache reuse is barely affected: 89% of attempts are already fully cold. The cargo registry and git mounts stay shared, since the measured contention was only on the target directory.
ae742dd to
76a8c01
Compare
Overview
The two bottlecap compile Dockerfiles mount the cargo build directory (
target) as a BuildKit cache mount with nosharingmode. All concurrently-scheduledbottlecap (*)compile jobs that use the same Dockerfile therefore share one cache mount, and cargo serializes them on its build-directory lock: while one build holds the lock, the others logBlocking waiting for file lock on build directoryand sit idle.Measured over 240 successful compile attempts across 30
mainpipelines (2026-06-25 to 2026-09-09):amd6433%,arm64, fips23%This adds
sharing=privateto thetargetcache mount only, in bothimages/Dockerfile.bottlecap.compileandimages/Dockerfile.bottlecap.alpine.compile. Each concurrent build now gets its own cache instance instead of blocking.Why not a per-flavor
id=? A per-flavor cacheid=(e.g.id=target-${PLATFORM}-fips${FIPS}) would also isolate the caches, but neither Dockerfile has a# syntax=directive and CI builds underdocker:20.10, so build-arg expansion inside a mountid=may silently not apply on that frontend. If expansion failed, all flavors would collapse into one literal id, which is worse than the current behavior and hard to notice.sharing=privateneeds no variable expansion.Why the registry/git mounts are unchanged: the cargo registry and git cache mounts are read-mostly and shared registry cache is genuinely useful across flavors. The exclusive lock that caused every measured wait is on the build directory, not the registry. Keeping this to one variable also means any change in the lock-wait rate after merge is attributable to this change.
Known risk (worth flagging, not buried): 5 of 10 analyzed compile-job stalls occur at the final LTO link (
lto = true,codegen-units = 1,bottlecap/Cargo.toml:129-131). If those stalls are memory pressure from concurrent LTO links on a shared runner, the cargo lock has been accidentally serializing them, and this change lets up to 4 same-Dockerfile flavors hit LTO simultaneously, which could raise thejob_execution_timeoutrate. This is an unvalidated hypothesis (would need runner co-tenancy data; 9-10 events is likely underpowered), but it means the timeout rate should be watched after merge rather than assumed to improve.Testing
docker build --check(Docker 29.8.0, current buildkit frontend) parses both modified Dockerfiles with no warnings. The Dockerfile parser strips full-line comments inside continuedRUNinstructions, so the added comment lines are inert.CI validation on this branch
Validated across all 4 GitLab pipelines for this branch (Sep 10-16, all containing the fix), 34 compile attempts (32 first attempts + 2 retries):
Blocking waiting for file lock on build directoryappeared in 0 of 34 job traces (baseline on main: 14.2% of attempts).The 2 failed attempts, both unrelated to the target-mount lock and both green on retry:
bottlecap (arm64, alpine)(Sep 11, 117s,script_failure): transient race in the shared/root/.cargo/gitcache mount; cargo crashed removingserverless-components-.../config.lock(No such file or directory, thenDirectory not empty) while a concurrent build held it. This is the mount family this PR deliberately leaves shared; it is a candidate for the same treatment if it recurs.bottlecap (arm64, fips)(Sep 16, 1207s,job_execution_timeout): trace shows the final LTO link (-C lto -C codegen-units=1) start ~3m into the job, then silence until the 20m runner timeout. One data point consistent with the LTO-stall risk above (~3% of attempts, vs 23% lock-hit rate for this flavor on main); the retry completed in 321s.One caveat to watch after merge: with
sharing=private, all 4 same-Dockerfile flavors run concurrently, so each builds with a cold target-cache instance, reducing cross-run warm-cache reuse. 4 pipelines is too small a sample to detect that effect; watch per-flavor durations alongside the timeout rate.