Skip to content

Commit 24e3023

Browse files
cristifalcasclaude
andcommitted
fix(artifacts): keep each retried attempt's profile and exec log
`--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>
1 parent fdf5ecc commit 24e3023

1 file changed

Lines changed: 58 additions & 8 deletions

File tree

‎crates/aspect-cli/src/builtins/aspect/private/lib/artifacts.axl‎

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -723,16 +723,20 @@ def _new_upload_group():
723723
"label_urls": {},
724724
}
725725

726-
def _trace_summary(ctx, items, channels, skipped_files, profile_path, execlog_path, bep_path, uploader_active):
726+
def _trace_summary(ctx, items, channels, skipped_files, attempts, bep_path, uploader_active):
727727
"""Optional per-task debug summary printed when `trace.enabled`."""
728728
if not trace.enabled:
729729
return
730730
trace.log("")
731731
trace.log("------------------------------------")
732732
trace.log("artifact upload debug summary:")
733733
trace.log("uploader: " + ("active" if uploader_active else "none"))
734-
trace.log("profile: " + profile_path + " (exists=" + str(ctx.std.fs.exists(profile_path)) + ")")
735-
trace.log("execlog: " + execlog_path + " (exists=" + str(ctx.std.fs.exists(execlog_path)) + ")")
734+
735+
# Per attempt, because a retry moves each attempt's profile and exec log to
736+
# its own path; the canonical ones no longer exist by this point.
737+
for i, saved in enumerate(attempts):
738+
for kind, path, _name in saved:
739+
trace.log("%s (attempt %d): %s (exists=%s)" % (kind, i + 1, path, str(ctx.std.fs.exists(path))))
736740
trace.log("bes/bep: " + bep_path + " (exists=" + str(ctx.std.fs.exists(bep_path)) + ")")
737741
for channel in channels:
738742
trace.log(channel.basename + " entries: " + str(len(channel.entries)))
@@ -839,6 +843,43 @@ def _artifact_upload_impl(ctx: FeatureContext):
839843
if upload_bep:
840844
bazel_trait.build_event_sinks.append(bazel.build_events.file(path = bep_path))
841845

846+
# Bazel writes these two itself from the flags above, to one path that every
847+
# retry attempt reuses, so attempt N+1 truncates attempt N's file. The
848+
# attempt worth analysing is precisely the one that failed, and it is the
849+
# one lost. `bep_path` is not here: the CLI owns that sink, not Bazel.
850+
retained = []
851+
if upload_profile:
852+
retained.append(("profile", profile_path, "profile.gz"))
853+
if upload_exec_log:
854+
retained.append(("execlog", execlog_path, "execlog.zstd"))
855+
856+
# One entry per completed attempt, each a list of (kind, path, name).
857+
attempts = []
858+
859+
def _retain_attempt(ctx, _exit_code) -> None:
860+
"""Move this attempt's Bazel-written files aside before the next one
861+
truncates them. Fires from `bazel_attempt_end`, after `wait()` returns
862+
and before the retry decision, so the file is complete and closed."""
863+
saved = []
864+
for kind, src, name in retained:
865+
if not ctx.std.fs.exists(src):
866+
continue
867+
868+
# Attempt number goes before the extension, not after: silo's
869+
# deployments/automation/deploy-helm.sh picks the profile up with
870+
# `ls -t /workflows/*.profile.gz`, and a trailing `.attempt-N`
871+
# would stop matching that glob.
872+
#
873+
# Same directory, so the rename cannot cross filesystems. AXL has
874+
# no try/except: a failure here would fail an otherwise fine task.
875+
dest = "%s/%s.attempt-%d.%s" % (tmpdir, uuid, len(attempts) + 1, name)
876+
ctx.std.fs.rename(src, dest)
877+
saved.append((kind, dest, name))
878+
attempts.append(saved)
879+
880+
if retained:
881+
bazel_trait.bazel_attempt_end.append(_retain_attempt)
882+
842883
# One `_LogChannel` per collected-log stream (test logs, build logs). Each
843884
# owns an entry list + upload group so they batch, flush, and roll-replace
844885
# independently through the same per-CI strategies.
@@ -903,6 +944,10 @@ def _artifact_upload_impl(ctx: FeatureContext):
903944
_flush(ctx, channel, final = False)
904945

905946
def _on_build_end(ctx, exit_code):
947+
# A bazel driver that dispatches no attempt-end hook leaves `attempts`
948+
# empty, so fall back to reading the un-moved canonical paths.
949+
completed = attempts if attempts else [retained]
950+
906951
if not is_local:
907952
reason = uploader.check(ctx)
908953
if reason:
@@ -915,19 +960,24 @@ def _artifact_upload_impl(ctx: FeatureContext):
915960
# removed on success AND failure (no retry, so a leftover
916961
# file just leaks a potentially-sensitive artifact).
917962
singletons = []
918-
if upload_profile:
919-
singletons.append(("profile", profile_path, "profile.gz"))
920963
if upload_bep:
921964
singletons.append(("bep", bep_path, "bep.binpb"))
922-
if upload_exec_log:
923-
singletons.append(("execlog", execlog_path, "execlog.zstd"))
965+
966+
# The last attempt keeps the canonical artifact name, so a task
967+
# that never retried uploads exactly what it always did; the
968+
# earlier ones are numbered rather than dropped.
969+
for i, saved in enumerate(completed):
970+
is_last = i == len(completed) - 1
971+
for kind, src, name in saved:
972+
singletons.append((kind, src, name if is_last else "attempt-%d.%s" % (i + 1, name)))
973+
924974
for kind, src, filename in singletons:
925975
if not ctx.std.fs.exists(src):
926976
continue
927977
_upload_file(ctx, kind, filename, src)
928978
ctx.std.fs.remove_file(src)
929979

930-
_trace_summary(ctx, items, channels, skipped_files, profile_path, execlog_path, bep_path, not is_local)
980+
_trace_summary(ctx, items, channels, skipped_files, completed, bep_path, not is_local)
931981

932982
# Reap channel scratch unconditionally — even when check() skipped the
933983
# upload, build events may have already staged log files.

0 commit comments

Comments
 (0)