Athens: Unify implementation for s3-compatible storage backends

Created on 1 Nov 2018  路  8Comments  路  Source: gomods/athens

Is your feature request related to a problem? Please describe.
Minio and AWS s3 are compatible and so we could unify the implementation of this storages.

Describe the solution you'd like
At the moment there is this:
https://github.com/gomods/athens/tree/master/pkg/storage/s3
and this:
https://github.com/gomods/athens/tree/master/pkg/storage/minio
We could probably replace one with another. The question is which one should we choose. One important aspect is how minio and aws-s3 clients handle steaming.

proposal refactor storage

Most helpful comment

Yeah I'm interested, cheers! I'll take this one. Thanks all

All 8 comments

It seems that if we do _, err = s.minioClient.PutObject(s.bucketName, zipFileName, zip, -1, minio.PutObjectOptions{}) (passing size=-1) minio will determine that the part size is 5TB/10000 and will allocate a buffer of this size (~600 MB). There is currently no way to change that.
https://github.com/minio/minio-go/blob/55c9b2e90ef38c5962d872ebc34b5d7c0e04974c/api-put-object-common.go#L71:6
For ref: https://github.com/minio/minio-go/issues/989

With aws-sdk-go the part size is customizable but the minimum size is 5MB.

@chriscoffee you might be interested in this one?

Yeah I'm interested, cheers! I'll take this one. Thanks all

This seems related to #489.. Also could be worth holding off on this if the plan is to replace it all with Go Cloud :thought_balloon:

Go cloud is still alpha and not suitable for production. I think removing/simplifying code can be still beneficial. Also, if I'm not mistaken and minio client really allocates 600mb on each module save then it might not be a workable solution.

I actually forgot about this, 馃槶

Given the recent issues with performance problems from minio #1106, #1064, #1060 it might be worth as @leitzler rightly suggests:

Probably use their [minio] core api instead and do a single put object
or even toss out minio-go and use aws s3 sdk instead
_https://gophers.slack.com/team/U28141ZK6_

Pontus also suggested using Go CDK which is 'a bit more mature' https://blog.golang.org/gcdk-whats-new-in-march-2019

FYI: this would break current users of Minio as their modules will be stored in different paths.
Our s3 implementation uses the module.Uploader (whose path is not what we want anyway), and minio uses its own Uploader (whose path is more ideal)

I'm going to close this b/c of what @marwan-at-work said above. Also, since we last checked in on this issue, we've done a fair bit of customization for the s3 backend to handle various auth situations. I don't think it's worth writing a migrator for Minio and handle the s3 specific things, just so we can unify the 2 storage backends.

Was this page helpful?
0 / 5 - 0 ratings