Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multipart artifact upload #2991

Merged
merged 5 commits into from
Oct 3, 2024
Merged

Multipart artifact upload #2991

merged 5 commits into from
Oct 3, 2024

Conversation

DrJosh9000
Copy link
Contributor

@DrJosh9000 DrJosh9000 commented Sep 16, 2024

Description

Add a multipart-upload mechanism for Buildkite-hosted artifacts, and upload parts in parallel (with each other, and with other artifacts).

Context

https://linear.app/buildkite/issue/PIPE-180/implement-agent-half-of-multipart-uploads

Changes

The changes to artifact uploading is substantial, but basically:

  • Some tidyups to the existing code, such as context propagation, un-exporting the Collect method
  • FormUploader is renamed to BKUploader, since it will manage both form uploads and multipart uploads to BK
  • Changing all the existing uploaders to return one or more units of work, which are ultimately responsible for performing an upload
  • Running work units concurrently on a pool of worker goroutines, and communicating between goroutines using channels
  • Actually implementing multipart uploads:
    • Alter the batch artifact creation API to signal that multipart is supported and enabled
    • Alter the creation response to handle per-artifact instructions and multiple actions per instruction
    • Alter the batch update API to accept part number-ETag mappings
    • Implement multipart upload work units that are responsible for uploading a single part each

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go fmt ./...)

@DrJosh9000 DrJosh9000 force-pushed the multipart-upload branch 20 times, most recently from 9380664 to 2a3de4b Compare September 25, 2024 01:41
@DrJosh9000 DrJosh9000 force-pushed the multipart-upload branch 5 times, most recently from b4acdc7 to 3dca5a9 Compare September 30, 2024 03:33
@DrJosh9000 DrJosh9000 changed the title [WIP] Multipart artifact upload Multipart artifact upload Sep 30, 2024
@DrJosh9000 DrJosh9000 marked this pull request as ready for review September 30, 2024 03:45
@DrJosh9000 DrJosh9000 force-pushed the multipart-upload branch 3 times, most recently from 6b69e32 to 64c9c93 Compare September 30, 2024 04:27
@DrJosh9000 DrJosh9000 requested a review from a team September 30, 2024 07:09
@DrJosh9000 DrJosh9000 force-pushed the multipart-upload branch 3 times, most recently from b52ee5a to 6261dd7 Compare October 1, 2024 00:32
Copy link
Contributor

@CerealBoy CerealBoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@DrJosh9000 DrJosh9000 merged commit 79ed39a into main Oct 3, 2024
1 check passed
@DrJosh9000 DrJosh9000 deleted the multipart-upload branch October 3, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants