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

deps/objstore: upgrade to minio-go/v7 #2970

Merged
merged 3 commits into from
Aug 3, 2020

Conversation

heimweh
Copy link
Contributor

@heimweh heimweh commented Aug 3, 2020

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

First time contributor so might need some handholding here :)

Upgraded from minio-go/v6 to minio-go/v7 and adjusted a couple functions to resolve the following issue: #2097

Example timings before (compactor):

level=info ts=2020-08-03T06:35:40.584914398Z caller=fetcher.go:452 component=block.BaseFetcher msg="successfully synchronized block metadata" duration=13.712880984s cached=61 returned=61 partial=0
level=info ts=2020-08-03T06:35:47.210427484Z caller=fetcher.go:452 component=block.BaseFetcher msg="successfully synchronized block metadata" duration=6.624377812s cached=61 returned=61 partial=0
level=info ts=2020-08-03T06:35:54.223933248Z caller=fetcher.go:452 component=block.BaseFetcher msg="successfully synchronized block metadata" duration=27.351742657s cached=61 returned=31 partial=0

Example timings after (compactor):

level=info ts=2020-08-03T06:47:50.632665389Z caller=fetcher.go:453 component=block.BaseFetcher msg="successfully synchronized block metadata" duration=135.100236ms cached=61 returned=61 partial=0
level=info ts=2020-08-03T06:48:50.682027929Z caller=fetcher.go:453 component=block.BaseFetcher msg="successfully synchronized block metadata" duration=184.352188ms cached=61 returned=61 partial=0
level=info ts=2020-08-03T06:49:50.590307937Z caller=fetcher.go:453 component=block.BaseFetcher msg="successfully synchronized block metadata" duration=92.617787ms cached=61 returned=61 partial=0

Fixes: #2097
Related: minio/minio-go#1321

Verification

Existing tests and from actual usage.
Happy to provide more information and/or add tests if necessary :)

Signed-off-by: Alexander Hellbom <alexander.hellbom@gmail.com>
Signed-off-by: Alexander Hellbom <alexander.hellbom@gmail.com>
Signed-off-by: Alexander Hellbom <alexander.hellbom@gmail.com>
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

This looks very solid, thanks. Only one nit, but so small I will just merge, let's fix next time. 💪

It looks like the 100th contribution not first!

I really tried to find any suggestion to make, but could not ;p

}).DialContext,

MaxIdleConns: 100,
MaxIdleConnsPerHost: 100,
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -228,7 +233,12 @@ func (b *Bucket) Iter(ctx context.Context, dir string, f func(string) error) err
dir = strings.TrimSuffix(dir, DirDelim) + DirDelim
}

for object := range b.client.ListObjects(b.name, dir, false, ctx.Done()) {
opts := minio.ListObjectsOptions{
Copy link
Member

Choose a reason for hiding this comment

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

@@ -314,7 +324,7 @@ func (b *Bucket) Upload(ctx context.Context, name string, r io.Reader) error {
if size < int64(partSize) {
partSize = 0
}
if _, err := b.client.PutObjectWithContext(
if _, err := b.client.PutObject(
Copy link
Member

Choose a reason for hiding this comment

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

Wow (: They did this finally

@bwplotka bwplotka merged commit 040b69b into thanos-io:master Aug 3, 2020
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.

Store Gateway: S3 very slow when using IAM Roles for Service Accounts on EKS
2 participants