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?

  • TestDownloadArtifactsParallelFallbackStartsFromEmptyFile in network: through DownloadArtifacts with FF_USE_PARALLEL_ARTIFACT_TRANSFER, the middle chunk of three is answered with 503 while the other two arrive as requested, and the GET without Range answers with a 48-byte body. It runs twice, into the file and into meter.NewWriter around it, as artifacts-downloader passes it when TRANSFER_METER_FREQUENCY is set. Each time the result is DownloadSucceeded, with chunk requests and then one GET without Range, and a file equal to that body.
  • TestDownloadArtifactsParallelFailsWhenFileCannotBeEmptied in network: the same download into a file whose Truncate fails gives DownloadFailed after the chunk requests, and no GET without Range.
  • TestTryArtifactParallelDownloadRejectsChunkOutsideRange in network now also checks that the file is empty after the rejected chunk.
  • TestWriter_Truncate_underlyingFile and TestWriter_Truncate_unsupported in commands/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 writes

3 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/transfer

Environment: Linux amd64, Go 1.26.5 (from go.mod).

What are the relevant issue numbers?

Closes #40110 (closed)

Edited by José M. Requena Plens

Merge request reports

Loading
Loading