Truncate the artifacts file when a parallel download falls back
What does this MR do?
Makes a failed parallel artifact download empty the file before DownloadArtifacts falls back to a single download stream. tryArtifactParallelDownload now truncates the destination to zero when transfer.ParallelRangeDownload fails, and it takes the parallel path only when the destination can be truncated as well as written at offsets. The transfer meter that artifacts-downloader wraps the file in when TRANSFER_METER_FREQUENCY is set forwards Truncate to the file, as it already forwards WriteAt.
Why was this MR needed?
With FF_USE_PARALLEL_ARTIFACT_TRANSFER enabled and direct_download redirected to object storage, the parallel attempt writes each chunk with WriteAt at its offset (gitlab.go#L1243). When a chunk fails, DownloadArtifacts falls back to a single GET and copies its body into the same file (#L1359-L1375, #L1451). WriteAt does not move the file offset, so that copy starts at byte 0, but nothing removed what the parallel attempt had written. A body shorter than the size the probe reported, for example because the object was replaced in between, left the rest of the failed attempt after it, and the download still reported DownloadSucceeded. The caller creates the file once per attempt (artifacts_downloader.go#L61), so the parallel attempt and the fallback share it.
I truncate in tryArtifactParallelDownload because that is where the chunks are written, and every caller of the fallback then gets an empty file. If the truncation itself fails, tryArtifactParallelDownload returns an error and DownloadArtifacts reports DownloadFailed instead of writing the single stream over the failed attempt. artifacts-downloader then retries with a new file, as it does for any failed download.
Behavior change: a destination that implements io.WriterAt but not Truncate now goes straight to the single download stream. In the runner the destination is the *os.File that artifacts-downloader creates, or the meter around it, and both can be truncated.
This is the fallback that #40109 relies on for the artifact path: a chunk answered with 412 because the object was replaced falls back to the single stream, and the new object can be shorter.
What's the best way to test this MR?
TestDownloadArtifactsParallelFallbackStartsFromEmptyFileinnetwork: throughDownloadArtifactswithFF_USE_PARALLEL_ARTIFACT_TRANSFER, the middle chunk of three is answered with503while the other two arrive as requested, and the GET withoutRangeanswers with a 48-byte body. It runs twice, into the file and intometer.NewWriteraround it, asartifacts-downloaderpasses it whenTRANSFER_METER_FREQUENCYis set. Each time the result isDownloadSucceeded, with chunk requests and then one GET withoutRange, and a file equal to that body.TestDownloadArtifactsParallelFailsWhenFileCannotBeEmptiedinnetwork: the same download into a file whoseTruncatefails givesDownloadFailedafter the chunk requests, and no GET withoutRange.TestTryArtifactParallelDownloadRejectsChunkOutsideRangeinnetworknow also checks that the file is empty after the rejected chunk.TestWriter_Truncate_underlyingFileandTestWriter_Truncate_unsupportedincommands/helpers/meter: the meter truncates the file it writes, and reports an error when the writer it wraps cannot be truncated.
go test -race -count=1 -run 'TestDownloadArtifactsParallel|TestTryArtifactParallelDownload' ./network/
go test -race -count=1 -run 'TestWriter' ./commands/helpers/meter/With main, the two DownloadArtifacts tests (both cases of the first), the three cases of TestTryArtifactParallelDownloadRejectsChunkOutsideRange and both meter tests fail. I also checked that each test catches a change it is there for: leaving the file as it is after a failed attempt fails the first three, ignoring a failed truncation fails TestDownloadArtifactsParallelFailsWhenFileCannotBeEmptied (whether it is ignored in tryArtifactParallelDownload or in DownloadArtifacts), a meter whose Truncate does not truncate fails TestWriter_Truncate_underlyingFile and the meter case of TestDownloadArtifactsParallelFallbackStartsFromEmptyFile, and a meter without a Truncate method fails that case too, because the download then skips the parallel attempt.
I ran TZ=UTC go test -race -count=1 on ./network/, ./commands/helpers/meter/, ./commands/helpers/ and ./helpers/transfer/, GOOS=linux, GOOS=windows and GOOS=darwin go vet on those packages, and, in the runner-linters:2.12.2-go1.26.5 image the go:lint job uses, make lint on the first commit and golangci-lint run on ./network/... and ./commands/helpers/meter/... with make check_test_directives on the second (0 issues. both times).
Test output on main
With the code from main at 5d62e0cc:
--- FAIL: TestTryArtifactParallelDownloadRejectsChunkOutsideRange
--- FAIL: .../200_with_the_whole_object
--- FAIL: .../206_for_another_range
--- FAIL: .../206_without_Content-Range
Error: Should be zero, but was 16777216
Messages: the failed parallel download should leave the file empty for the single download stream
--- FAIL: TestDownloadArtifactsParallelFallbackStartsFromEmptyFile
--- FAIL: .../file
--- FAIL: .../transfer_meter_around_the_file
Messages: the file differs from the body of the single download stream (33555456 bytes, want 48)
--- FAIL: TestDownloadArtifactsParallelFailsWhenFileCannotBeEmptied
Error: Not equal: expected: 3, actual: 0
Error: Should be zero, but was 1
Messages: DownloadArtifacts should not write a single download stream over the failed attempt
--- FAIL: TestWriter_Truncate_underlyingFile
--- FAIL: TestWriter_Truncate_unsupported
Messages: the meter should let a caller truncate the file it writes3 is DownloadFailed and 0 is DownloadSucceeded: on main the download into the file that cannot be truncated succeeds with one GET without Range, over what the failed attempt wrote.
With this MR:
ok gitlab.com/gitlab-org/gitlab-runner/network
ok gitlab.com/gitlab-org/gitlab-runner/commands/helpers/meter
ok gitlab.com/gitlab-org/gitlab-runner/commands/helpers
ok gitlab.com/gitlab-org/gitlab-runner/helpers/transferEnvironment: Linux amd64, Go 1.26.5 (from go.mod).
What are the relevant issue numbers?
Closes #40110 (closed)