fix(artifacts): keep each retried attempt's profile and exec log - #1434
cristifalcas wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Aspect Workflows Tasks📅 Thu Sep 10 15:33:45 UTC 2026 Task Results
Reproduce❌ delivery (delivery-uncacheable · delivery-gha-debug · delivery-gha)Install ⏱ Last updated Thu Sep 10 15:40:56 UTC 2026 · 📊 GitHub API quota 42/15,000 (0% used, resets in 58m) |
7994d12 to
24e3023
Compare
`--profile` and the compact execution log are one path per task, reused by
every retry attempt, so attempt N+1 truncates attempt N's file. Two costs.
The evidence for the failure is destroyed. A task only retries because an
attempt went wrong, and that attempt's timing profile and execution log are
exactly what you would read to find out why — but the retry overwrites both
before either is uploaded, so only the attempt that succeeded is ever
collected.
It also breaks the previous attempt's upload mid-stream. Bazel digests the
finished profile, then streams it asynchronously under
`--build_event_binary_file_upload_mode=fully_async`; a retry that starts
during that window replaces the file with a fresh 10-byte gzip header, and
the CAS rejects the mismatch:
WARNING: Uploading BEP referenced local file …profile.gz
hash: "b1f4c12b…" size_bytes: 723045
: INVALID_ARGUMENT: Buffer is 10 bytes in size, while 723045 bytes were expected
Ten bytes is a `GZIPOutputStream` header with no trailer — an empty gzip
file is 20 — which is what identifies the writer as a starting attempt
rather than a truncated remnant. The error names the CAS, so it reads as a
remote-cache fault and sends anyone debugging it in the wrong direction.
A `bazel_attempt_end` hook now moves both files aside as each attempt ends:
that hook runs after `wait()` returns and before the retry decision, so the
file is complete and closed. The final attempt still uploads under the
canonical `profile.gz` / `execlog.zstd`, so a task that never retried is
unchanged; earlier attempts arrive as `attempt-N.profile.gz`.
The attempt number goes before the extension because silo's
`deployments/automation/deploy-helm.sh` collects the profile with
`ls -t /workflows/*.profile.gz`, which a trailing `.attempt-N` would miss.
`bep_path` is left alone: the CLI owns that sink, not Bazel.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
24e3023 to
23578e3
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24e3023fa7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| is_last = i == len(completed) - 1 | ||
| for kind, src, name in saved: | ||
| singletons.append((kind, src, name if is_last else "attempt-%d.%s" % (i + 1, name))) |
There was a problem hiding this comment.
When a delivery task runs its normal phase-1/phase-2 flow with upload_exec_log, only phase 1 receives bazel_trait.execution_log_sinks (delivery.axl:558-596), while phase 2 still dispatches bazel_attempt_end (delivery.axl:704-741). The latter appends an empty final entry to attempts, so this position-based check classifies the sole execution log as non-final and uploads it as attempt-1.execlog.zstd even though no retry occurred, removing the previously guaranteed canonical execlog.zstd; determine the final saved entry per artifact kind rather than using the last hook invocation.
Useful? React with 👍 / 👎.
1 participant
--profileand the compact execution log are one path per task, reused by every retry attempt, so attempt N+1 truncates attempt N's file.The evidence for the failure is destroyed. A task only retries because an attempt went wrong, and that attempt's timing profile and execution log are exactly what you would read to find out why. The retry overwrites both before either is uploaded, so only the attempt that succeeded is ever collected.
It also breaks the previous attempt's upload mid-stream. Bazel digests the finished profile, then streams it asynchronously under
--build_event_binary_file_upload_mode=fully_async. A retry starting inside that window replaces the file with a fresh 10-byte gzip header, and the CAS rejects the mismatch:Ten bytes is a
GZIPOutputStreamheader with no trailer — an empty gzip file is 20 — which is what identifies the writer as a starting attempt rather than a truncated remnant. Because the error names the CAS, it reads as a remote-cache fault; it cost me two wrong hypotheses before the byte count gave it away.The fix. A
bazel_attempt_endhook moves both files aside as each attempt ends — that hook runs afterwait()returns and before the retry decision, so the file is complete and closed. The final attempt still uploads under the canonicalprofile.gz/execlog.zstd, so a task that never retried is unchanged; earlier attempts arrive asattempt-N.profile.gz.The attempt number goes before the extension because silo's
deployments/automation/deploy-helm.shcollects the profile withls -t /workflows/*.profile.gz, which a trailing.attempt-Nwould miss.bep_pathis left alone: the CLI owns that sink, not Bazel.Test plan
//crates/aspect-cli:testpasses; the AXL is compile_data, so the change is typechecked by the build.artifacts_test.axl, and I could not run thelib/*_test.axlsuite locally becausetools/bazel's forward toaspectfails on this checkout withModule has no symbol denied_terminal_post_warningfrom.aspect/axl.axl:13(pre-existing, unrelated). CI will be the first to run those.Follow-ups
attempt-1.profile.gzand no canonicalprofile.gz. Honest, but worth knowing.FAILED:banners carried no message, and the only error in the invocation was this upload artefact.🤖 Generated with Claude Code