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

Add support for multipart uploads #595

Merged
merged 21 commits into from
Sep 19, 2023
Merged

Conversation

ChrisSchinnerl
Copy link
Member

@ChrisSchinnerl ChrisSchinnerl commented Sep 11, 2023

This PR adds support for multipart uploads to the S3 API without storing the whole upload in memory.

NOTE: Migration code will be added and tested upon approval in case things still change.

Limitations

  • disablePreshardingEncryption has to be 'true' when uploading a part. This will be fixed asap in a f/u with appropriate testing.
  • Individual parts will be padded according to the used erasure coding settings unless upload packing is enabled.

@ChrisSchinnerl ChrisSchinnerl marked this pull request as ready for review September 18, 2023 07:38
@ChrisSchinnerl ChrisSchinnerl self-assigned this Sep 18, 2023
Copy link
Member

@peterjan peterjan left a comment

Choose a reason for hiding this comment

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

nothing but nits 👍 good stuff!!

api/bus.go Outdated Show resolved Hide resolved
api/bus.go Outdated Show resolved Hide resolved
api/bus.go Outdated Show resolved Hide resolved
api/bus.go Outdated Show resolved Hide resolved
worker/worker.go Outdated Show resolved Hide resolved
stores/multipart.go Show resolved Hide resolved
stores/multipart.go Outdated Show resolved Hide resolved
stores/multipart.go Outdated Show resolved Hide resolved
return err
}

// TODO: f/u with support for 'prefix', 'keyMarker' and 'uploadIDMarker'
Copy link
Member

Choose a reason for hiding this comment

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

reminder

Copy link
Member Author

Choose a reason for hiding this comment

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

It's on my list of things to f/u with.

worker/upload.go Show resolved Hide resolved
api/bus.go Show resolved Hide resolved
MultipartListUploadsResponse struct {
Uploads []MultipartListUploadItem `json:"uploads"`
}
MultipartListUploadItem struct {
Copy link
Member

Choose a reason for hiding this comment

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

is bucket missing here?

Copy link
Member Author

Choose a reason for hiding this comment

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

So it could be in MultipartListUploadsResponse but I didn't add it since we don't need it to satisfy the corresponding S3 endpoint. We can just take the bucket from the request.

api/bus.go Outdated Show resolved Hide resolved
object/object_test.go Outdated Show resolved Hide resolved
@peterjan
Copy link
Member

So etags are great, usually they're used to be able to return a 304 not modified response right (so the client can use its cached version), maybe we should extend renterd (in separate PR) to return etags for regular uploads as well and then look for the If-None-Match request header.

return n, err
}

func (e *hashReader) Etag() string {
Copy link
Member

Choose a reason for hiding this comment

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

nit Hash() string?

@ChrisSchinnerl ChrisSchinnerl merged commit 7faa3a3 into master Sep 19, 2023
6 checks passed
@ChrisSchinnerl ChrisSchinnerl deleted the chris/multipart-uploads branch September 19, 2023 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants