Merged
Conversation
Some S3 compatible setups may have a different limit for maximum file size in multipart uploads, so allow for overriding it. Signed-off-by: Lukasz Jernas <lukasz.jernas@allegro.pl>
Signed-off-by: Lukasz Jernas <lukasz.jernas@allegro.pl>
Signed-off-by: Lukasz Jernas <lukasz.jernas@allegro.pl>
Signed-off-by: Lukasz Jernas <lukasz.jernas@allegro.pl>
benethor
approved these changes
Jul 29, 2019
bwplotka
approved these changes
Jul 29, 2019
Member
bwplotka
left a comment
bwplotka
left a comment
There was a problem hiding this comment.
Nice! Thank you.
Small suggestions and LGTM!
pkg/objstore/s3/s3.go
Outdated
|
|
||
| // Use the default minio minPartSize if not set | ||
| if config.PartSize == 0 { | ||
| config.PartSize = 1024 * 1024 * 128 |
Member
There was a problem hiding this comment.
putting this 1024 * 1024 * 128 in some meaningfully named constant would be nice for a reader.
Contributor
Author
There was a problem hiding this comment.
hope it's better now, although I couldn't really find a meaningful variable name which wouldn't require an explanation anyway.
Extract calculation to a constant and fixup comments.
bwplotka
approved these changes
Jul 29, 2019
Signed-off-by: Lukasz Jernas <lukasz.jernas@allegro.pl>
Member
|
Great, thanks for this! We actually lately hit scenario where we need this feature 👍 |
Member
|
maybe worth to add to the changelog? |
FUSAKLA
approved these changes
Jul 29, 2019
GiedriusS
approved these changes
Jul 30, 2019
Member
GiedriusS
left a comment
GiedriusS
left a comment
There was a problem hiding this comment.
Well tested and makes sense, LGTM!
paulfantom
added a commit
to paulfantom/thanos
that referenced
this pull request
Aug 7, 2019
* master: iter.go: error message typo correction (thanos-io#1376) Fix usage of $GOPATH in Makefile (thanos-io#1379) Moved Prometheus 2.11.1 and TSDB to 0.9.1 (thanos-io#1380) Store latest config hash and timestamp as metrics (thanos-io#1378) pkg/receive/handler.go: log errors (thanos-io#1372) receive: Hash-ring metrics (thanos-io#1363) receiver: avoid race of hashring (thanos-io#1371) feat compact: added readiness Prober (thanos-io#1297) Add changelog entry for S3 option (thanos-io#1361) Multipart features (thanos-io#1358) Added katacoda.yaml (thanos-io#1359) Remove deprecated option from example (thanos-io#1351) Move suggestion about admin API to appropriate place (thanos-io#1355)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some S3 compatible setups may have a different limit for maximum
file size in multipart uploads, so allow for overriding it.
Signed-off-by: Lukasz Jernas lukasz.jernas@allegro.pl
Changes
Added
part_sizeparameter to S3 YAML config file in bytes to allow for setting different multipart part size for chunk upload.Verification
Added a config test and tested in a real world scenario.