From 8b398aacf985ca01dbb4b6ed7e1744f4261e914d Mon Sep 17 00:00:00 2001 From: bwplotka Date: Thu, 7 Jul 2022 15:39:05 +0200 Subject: [PATCH 1/3] Updated minio-go fork to latest. NOTE: Optimization is propopsed to upstream to avoid fork in future. Relates to https://github.com/thanos-io/thanos/issues/5101 and https://github.com/thanos-io/thanos/issues/5130 Signed-off-by: bwplotka # Conflicts: # go.mod # go.sum --- go.mod | 5 --- go.sum | 7 +++- pkg/objstore/s3/s3.go | 1 + pkg/objstore/s3/s3_e2e_test.go | 75 ++++++++++++++++++++++++++++++++-- 4 files changed, 77 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index cdac721665..42a2ad59ff 100644 --- a/go.mod +++ b/go.mod @@ -229,11 +229,6 @@ replace ( // go 1.18 github.com/json-iterator/go => github.com/json-iterator/go v1.1.12 - - // TODO: Remove this: https://github.com/thanos-io/thanos/issues/3967. - github.com/minio/minio-go/v7 => github.com/bwplotka/minio-go/v7 v7.0.11-0.20210324165441-f9927e5255a6 - - // go 1.18 github.com/modern-go/reflect2 => github.com/modern-go/reflect2 v1.0.2 // Make sure Prometheus version is pinned as Prometheus semver does not include Go APIs. diff --git a/go.sum b/go.sum index 0ac474c08e..137b0d4948 100644 --- a/go.sum +++ b/go.sum @@ -329,8 +329,6 @@ github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx2 github.com/bugsnag/bugsnag-go v0.0.0-20141110184014-b1d153021fcd/go.mod h1:2oa8nejYd4cQ/b0hMIopN0lCRxU0bueqREvZLWFrtK8= github.com/bugsnag/osext v0.0.0-20130617224835-0dd3f918b21b/go.mod h1:obH5gd0BsqsP2LwDJ9aOkm/6J86V6lyAXCoQWGw3K50= github.com/bugsnag/panicwrap v0.0.0-20151223152923-e2c28503fcd0/go.mod h1:D/8v3kj0zr8ZAKg1AQ6crr+5VwKN5eIywRkfhyM/+dE= -github.com/bwplotka/minio-go/v7 v7.0.11-0.20210324165441-f9927e5255a6 h1:h9SZ0jmAKjtrZF6iZ77/jdXdHr+Usn29itI669SVRp4= -github.com/bwplotka/minio-go/v7 v7.0.11-0.20210324165441-f9927e5255a6/go.mod h1:td4gW1ldOsj1PbSNS+WYK43j+P1XVhX/8W8awaYlBFo= github.com/c-bata/go-prompt v0.2.2/go.mod h1:VzqtzE2ksDBcdln8G7mk2RX9QyGjH+OVqOCSiVIqS34= github.com/cactus/go-statsd-client/statsd v0.0.0-20191106001114-12b4e2b38748/go.mod h1:l/bIBLeOl9eX+wxJAzxS4TveKRtAqlyDpHjhkfO0MEI= github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ= @@ -1230,6 +1228,7 @@ github.com/klauspost/compress v1.11.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYs github.com/klauspost/compress v1.11.13/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= github.com/klauspost/compress v1.13.1/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg= github.com/klauspost/compress v1.13.4/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg= +github.com/klauspost/compress v1.13.5/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= github.com/klauspost/compress v1.15.6 h1:6D9PcO8QWu0JyaQ2zUMmu16T1T+zjjEpP91guRsvDfY= github.com/klauspost/compress v1.15.6/go.mod h1:PhcZ0MbTNciWF3rruxRgKxI5NkcHHrHUDtV4Yw2GlzU= @@ -1372,6 +1371,10 @@ github.com/minio/md5-simd v1.1.2 h1:Gdi1DZK69+ZVMoNHRXJyNcxrMA4dSxoYHZSQbirFg34= github.com/minio/md5-simd v1.1.2/go.mod h1:MzdKDxYpY2BT9XQFocsiZf/NKVtR7nkE4RoEpN+20RM= github.com/minio/minio-go/v6 v6.0.44/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg= github.com/minio/minio-go/v6 v6.0.56/go.mod h1:KQMM+/44DSlSGSQWSfRrAZ12FVMmpWNuX37i2AX0jfI= +github.com/minio/minio-go/v7 v7.0.2/go.mod h1:dJ80Mv2HeGkYLH1sqS/ksz07ON6csH3S6JUMSQ2zAns= +github.com/minio/minio-go/v7 v7.0.10/go.mod h1:td4gW1ldOsj1PbSNS+WYK43j+P1XVhX/8W8awaYlBFo= +github.com/minio/minio-go/v7 v7.0.30 h1:Re+qlwA+LB3mgFGYbztVPzlEjKtGzRVV5Sk38np858k= +github.com/minio/minio-go/v7 v7.0.30/go.mod h1:/sjRKkKIA75CKh1iu8E3qBy7ktBmCCDGII0zbXGwbUk= github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM= github.com/minio/sha256-simd v1.0.0 h1:v1ta+49hkWZyvaKwrQB8elexRqm6Y0aMLjCNsrYxo6g= github.com/minio/sha256-simd v1.0.0/go.mod h1:OuYzVNI5vcoYIAmbIvHPl3N3jUzVedXbKy5RFepssQM= diff --git a/pkg/objstore/s3/s3.go b/pkg/objstore/s3/s3.go index e8b0f80dea..442266dda6 100644 --- a/pkg/objstore/s3/s3.go +++ b/pkg/objstore/s3/s3.go @@ -485,6 +485,7 @@ func (b *Bucket) Upload(ctx context.Context, name string, r io.Reader) error { PartSize: partSize, ServerSideEncryption: sse, UserMetadata: b.putUserMetadata, + NumThreads: 4, }, ); err != nil { return errors.Wrap(err, "upload s3 object") diff --git a/pkg/objstore/s3/s3_e2e_test.go b/pkg/objstore/s3/s3_e2e_test.go index eaf2ae7033..28176ff520 100644 --- a/pkg/objstore/s3/s3_e2e_test.go +++ b/pkg/objstore/s3/s3_e2e_test.go @@ -6,7 +6,12 @@ package s3_test import ( "bytes" "context" + "crypto/sha256" + "fmt" + "io" + "os" "strings" + "sync" "testing" "github.com/efficientgo/e2e" @@ -18,12 +23,71 @@ import ( "github.com/thanos-io/thanos/pkg/testutil" ) -// Regression benchmark for https://github.com/thanos-io/thanos/issues/3917. +// TestUploadConcurrent tries to reproduce race; unsuccessful, no race detected for cases mentioned in +// https://github.com/minio/mc/issues/3376#issuecomment-1158001018 . +// $ go test -v -race ./pkg/objstore/s3 -run='TestUploadConcurrent$'. +func TestUploadConcurrent(t *testing.T) { + t.Skip("// NOTE(bwplotka): Prerequsite: 'dd if=/dev/urandom of=./test-file bs=1024b count=1000'") + ctx := context.Background() + + e, err := e2e.NewDockerEnvironment("e2e_bench_mino_client") + testutil.Ok(t, err) + t.Cleanup(e2ethanos.CleanScenario(t, e)) + + const bucket = "benchmark" + m := e2ethanos.NewMinio(e, "benchmark", bucket) + testutil.Ok(t, e2e.StartAndWaitReady(m)) + + bkt, err := s3.NewBucketWithConfig( + log.NewLogfmtLogger(os.Stderr), + e2ethanos.NewS3Config(bucket, m.Endpoint("https"), m.Dir()), + "test-feed", + ) + testutil.Ok(t, err) + + wg := sync.WaitGroup{} + + // 500MB file created via `dd if=/dev/urandom of=./test-file bs=1024b count=1000` + f, err := os.Open("../../../test-file") + testutil.Ok(t, err) + + h := sha256.New() + _, err = io.Copy(h, f) + testutil.Ok(t, err) + expectedHash := h.Sum(nil) + + for i := 0; i < 6; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + + for k := 0; k < 6; k++ { + testutil.Ok(t, bkt.Upload(ctx, fmt.Sprintf("test-%v_%v", i, k), f)) + fmt.Printf("uploaded\n") + + // Verify if object is not malformed. + r, err := bkt.Get(ctx, fmt.Sprintf("test-%v_%v", i, k)) + testutil.Ok(t, err) + + h := sha256.New() + _, err = io.Copy(h, r) + testutil.Ok(t, err) + testutil.Equals(t, expectedHash, h.Sum(nil)) + } + }(i) + } + + wg.Wait() +} + +// Regression benchmark for https://github.com/thanos-io/thanos/issues/3917 and https://github.com/thanos-io/thanos/issues/3967. +// $ export ver=v1 && go test ./pkg/objstore/s3/... -run '^$' -bench '^BenchmarkUpload' -benchtime 5s -count 5 \ +// -memprofile=${ver}.mem.pprof -cpuprofile=${ver}.cpu.pprof | tee ${ver}.txt . func BenchmarkUpload(b *testing.B) { b.ReportAllocs() ctx := context.Background() - e, err := e2e.NewDockerEnvironment("e2e_bench_mino_client") + e, err := e2e.NewDockerEnvironment("e2e_bench_mino_client", e2e.WithLogger(log.NewNopLogger())) testutil.Ok(b, err) b.Cleanup(e2ethanos.CleanScenario(b, e)) @@ -31,8 +95,11 @@ func BenchmarkUpload(b *testing.B) { m := e2ethanos.NewMinio(e, "benchmark", bucket) testutil.Ok(b, e2e.StartAndWaitReady(m)) - bkt, err := s3.NewBucketWithConfig(log.NewNopLogger(), - e2ethanos.NewS3Config(bucket, m.Endpoint("https"), m.Dir()), "test-feed") + bkt, err := s3.NewBucketWithConfig( + log.NewNopLogger(), + e2ethanos.NewS3Config(bucket, m.Endpoint("https"), m.Dir()), + "test-feed", + ) testutil.Ok(b, err) buf := bytes.Buffer{} From 5f0cb51c045328658ee72c78f3df8c718a0b4504 Mon Sep 17 00:00:00 2001 From: bwplotka Date: Thu, 7 Jul 2022 15:37:23 +0200 Subject: [PATCH 2/3] Removed fork. Signed-off-by: bwplotka --- go.mod | 2 +- go.sum | 4 +-- pkg/objstore/s3/s3_e2e_test.go | 62 ---------------------------------- 3 files changed, 3 insertions(+), 65 deletions(-) diff --git a/go.mod b/go.mod index 42a2ad59ff..50a99b0f6e 100644 --- a/go.mod +++ b/go.mod @@ -52,7 +52,7 @@ require ( github.com/lightstep/lightstep-tracer-go v0.18.1 github.com/lovoo/gcloud-opentracing v0.3.0 github.com/miekg/dns v1.1.48 - github.com/minio/minio-go/v7 v7.0.30 + github.com/minio/minio-go/v7 v7.0.32-0.20220706200439-ef3e45ed9cdb github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f github.com/ncw/swift v1.0.52 github.com/oklog/run v1.1.0 diff --git a/go.sum b/go.sum index 137b0d4948..18dff24bc9 100644 --- a/go.sum +++ b/go.sum @@ -1373,8 +1373,8 @@ github.com/minio/minio-go/v6 v6.0.44/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tB github.com/minio/minio-go/v6 v6.0.56/go.mod h1:KQMM+/44DSlSGSQWSfRrAZ12FVMmpWNuX37i2AX0jfI= github.com/minio/minio-go/v7 v7.0.2/go.mod h1:dJ80Mv2HeGkYLH1sqS/ksz07ON6csH3S6JUMSQ2zAns= github.com/minio/minio-go/v7 v7.0.10/go.mod h1:td4gW1ldOsj1PbSNS+WYK43j+P1XVhX/8W8awaYlBFo= -github.com/minio/minio-go/v7 v7.0.30 h1:Re+qlwA+LB3mgFGYbztVPzlEjKtGzRVV5Sk38np858k= -github.com/minio/minio-go/v7 v7.0.30/go.mod h1:/sjRKkKIA75CKh1iu8E3qBy7ktBmCCDGII0zbXGwbUk= +github.com/minio/minio-go/v7 v7.0.32-0.20220706200439-ef3e45ed9cdb h1:J7jRWqlD+K3Tp4YbLWcyBKiHoNRy49JR5HA4RetFrAY= +github.com/minio/minio-go/v7 v7.0.32-0.20220706200439-ef3e45ed9cdb/go.mod h1:/sjRKkKIA75CKh1iu8E3qBy7ktBmCCDGII0zbXGwbUk= github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM= github.com/minio/sha256-simd v1.0.0 h1:v1ta+49hkWZyvaKwrQB8elexRqm6Y0aMLjCNsrYxo6g= github.com/minio/sha256-simd v1.0.0/go.mod h1:OuYzVNI5vcoYIAmbIvHPl3N3jUzVedXbKy5RFepssQM= diff --git a/pkg/objstore/s3/s3_e2e_test.go b/pkg/objstore/s3/s3_e2e_test.go index 28176ff520..0fef4a71e4 100644 --- a/pkg/objstore/s3/s3_e2e_test.go +++ b/pkg/objstore/s3/s3_e2e_test.go @@ -6,12 +6,7 @@ package s3_test import ( "bytes" "context" - "crypto/sha256" - "fmt" - "io" - "os" "strings" - "sync" "testing" "github.com/efficientgo/e2e" @@ -23,63 +18,6 @@ import ( "github.com/thanos-io/thanos/pkg/testutil" ) -// TestUploadConcurrent tries to reproduce race; unsuccessful, no race detected for cases mentioned in -// https://github.com/minio/mc/issues/3376#issuecomment-1158001018 . -// $ go test -v -race ./pkg/objstore/s3 -run='TestUploadConcurrent$'. -func TestUploadConcurrent(t *testing.T) { - t.Skip("// NOTE(bwplotka): Prerequsite: 'dd if=/dev/urandom of=./test-file bs=1024b count=1000'") - ctx := context.Background() - - e, err := e2e.NewDockerEnvironment("e2e_bench_mino_client") - testutil.Ok(t, err) - t.Cleanup(e2ethanos.CleanScenario(t, e)) - - const bucket = "benchmark" - m := e2ethanos.NewMinio(e, "benchmark", bucket) - testutil.Ok(t, e2e.StartAndWaitReady(m)) - - bkt, err := s3.NewBucketWithConfig( - log.NewLogfmtLogger(os.Stderr), - e2ethanos.NewS3Config(bucket, m.Endpoint("https"), m.Dir()), - "test-feed", - ) - testutil.Ok(t, err) - - wg := sync.WaitGroup{} - - // 500MB file created via `dd if=/dev/urandom of=./test-file bs=1024b count=1000` - f, err := os.Open("../../../test-file") - testutil.Ok(t, err) - - h := sha256.New() - _, err = io.Copy(h, f) - testutil.Ok(t, err) - expectedHash := h.Sum(nil) - - for i := 0; i < 6; i++ { - wg.Add(1) - go func(i int) { - defer wg.Done() - - for k := 0; k < 6; k++ { - testutil.Ok(t, bkt.Upload(ctx, fmt.Sprintf("test-%v_%v", i, k), f)) - fmt.Printf("uploaded\n") - - // Verify if object is not malformed. - r, err := bkt.Get(ctx, fmt.Sprintf("test-%v_%v", i, k)) - testutil.Ok(t, err) - - h := sha256.New() - _, err = io.Copy(h, r) - testutil.Ok(t, err) - testutil.Equals(t, expectedHash, h.Sum(nil)) - } - }(i) - } - - wg.Wait() -} - // Regression benchmark for https://github.com/thanos-io/thanos/issues/3917 and https://github.com/thanos-io/thanos/issues/3967. // $ export ver=v1 && go test ./pkg/objstore/s3/... -run '^$' -bench '^BenchmarkUpload' -benchtime 5s -count 5 \ // -memprofile=${ver}.mem.pprof -cpuprofile=${ver}.cpu.pprof | tee ${ver}.txt . From 4fc8378c7bc829df099fc19220e36698786829cd Mon Sep 17 00:00:00 2001 From: bwplotka Date: Thu, 7 Jul 2022 16:03:42 +0200 Subject: [PATCH 3/3] Added comment. Signed-off-by: bwplotka --- pkg/objstore/s3/s3.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/objstore/s3/s3.go b/pkg/objstore/s3/s3.go index 442266dda6..1fdc5347dc 100644 --- a/pkg/objstore/s3/s3.go +++ b/pkg/objstore/s3/s3.go @@ -485,7 +485,10 @@ func (b *Bucket) Upload(ctx context.Context, name string, r io.Reader) error { PartSize: partSize, ServerSideEncryption: sse, UserMetadata: b.putUserMetadata, - NumThreads: 4, + // 4 is what minio-go have as the default. To be certain we do micro benchmark before any changes we + // ensure we pin this number to four. + // TODO(bwplotka): Consider adjusting this number to GOMAXPROCS or to expose this in config if it becomes bottleneck. + NumThreads: 4, }, ); err != nil { return errors.Wrap(err, "upload s3 object")