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.

Sign up for GitHub

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

Jump to bottom

PubObject - possible data race ? #1355

Closed
findmyname666 opened this issue Jul 25, 2020 · 9 comments · Fixed by #1357
Closed

PubObject - possible data race ? #1355

findmyname666 opened this issue Jul 25, 2020 · 9 comments · Fixed by #1357
Assignees
@harshavardhana

Comments

@findmyname666
Copy link

I tested concurrent uploading of objects into MinIO. MinIO client should be thread safe as state here.

I tested function PubObject only.
There was detected data race during runtime when i compiled code with flag go build -race.

Sample steps to reproduce error:

  1. Set-up MinIO server.
  2. Create test file of size 1GB.
dd if=/dev/urandom of=./1GB_file.img bs=1024b count=2000
  1. Compile code to include race detection.
go build -race
  1. Run code and data race should be detected.

Sample code:

package main

import (
	"fmt"
	"os"
	"strconv"
	"sync"

	"github.com/minio/minio-go"
)

var (
	endpoint        = "localhost:9001"
	accessKeyID     = "some_key"
	secretAccessKey = "some_secret"
	useSSL          = false
	bName           = "test"
	bLocation       = ""
	toUpload        = "1GB_file.img"
	workers         = 5
)

// initializes MinIO client
func createMC() (*minio.Client, error) {
	mc, err := minio.New(
		endpoint,
		accessKeyID,
		secretAccessKey,
		useSSL,
	)

	return mc, err
}

// creates new bucket
func createBucket(mc *minio.Client) error {
	exists, err := mc.BucketExists(bName)
	if err == nil && exists {
		return nil
	} else if err != nil {
		return err
	}
	if err := mc.MakeBucket(bName, bLocation); err != nil {
		return err
	}
	return nil
}

func chanPopulate(ch chan<- string) {
	defer close(ch)
	for i := 0; i < 20; i++ {
		fmt.Printf("populating ch ID %d\n", i)
		ch <- toUpload
	}
}

func main() {
	mc, err := createMC()
	if err != nil {
		fmt.Printf("failed to create MinIO client, %v\n", err)
		os.Exit(1)
	}
	if err := createBucket(mc); err != nil {
		fmt.Printf("failed to create bucket, %v\n", err)
		os.Exit(1)
	}

	files := make(chan string, 5)
	go chanPopulate(files)

	var wg sync.WaitGroup
	for i := 1; i < 6; i++ {
		fmt.Printf("worker is starting ID %d\n", i)
		wg.Add(1)
		go func(i int) {
			defer wg.Done()
			fmt.Printf("worker started ID %d\n", i)
			for p := range files {
				fInfo, err := os.Stat(p)
				if err != nil {
					fmt.Printf("failed to stat file, path %s, %v\n", p, err)
					return
				}
				f, err := os.Open(p)
				if err != nil {
					fmt.Printf("failed to open file, path %s, %v\n", p, err)
					return
				}
				n, err := mc.PutObject(
					bName,
					toUpload+strconv.Itoa(i),
					f,
					fInfo.Size(),
					minio.PutObjectOptions{},
				)
				if err != nil {
					fmt.Printf("failed to upload file, path %s, %v\n", p, err)
					return
				}
				fmt.Printf("file uploaded, size '%d'\n", n)
			}
		}(i)
	}
	wg.Wait()
}

Detected error:

worker is starting ID 1
populating ch ID 0
populating ch ID 1
populating ch ID 2
populating ch ID 3
worker is starting ID 2
worker started ID 1
populating ch ID 4
populating ch ID 5
populating ch ID 6
worker is starting ID 3
worker started ID 2
worker is starting ID 4
populating ch ID 7
worker started ID 3
populating ch ID 8
worker is starting ID 5
worker started ID 4
populating ch ID 9
worker started ID 5
populating ch ID 10
==================
WARNING: DATA RACE
Write at 0x00c00009a1e0 by goroutine 117:
  github.com/minio/minio-go.Client.putObjectMultipartStreamFromReadAt.func2()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-streaming.go:168 +0x565

Previous write at 0x00c00009a1e0 by goroutine 116:
  github.com/minio/minio-go.Client.putObjectMultipartStreamFromReadAt.func2()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-streaming.go:168 +0x565

Goroutine 117 (running) created at:
  github.com/minio/minio-go.Client.putObjectMultipartStreamFromReadAt()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-streaming.go:147 +0x7e3
  github.com/minio/minio-go.Client.putObjectMultipartStream()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-streaming.go:45 +0x1ba
  github.com/minio/minio-go.Client.putObjectCommon()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object.go:164 +0x765
  github.com/minio/minio-go.Client.PutObjectWithContext()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-context.go:32 +0x205
  github.com/minio/minio-go.Client.PutObject()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object.go:135 +0x568
  main.main.func1()
      /home/findmyname/Documents/projects/testing/minio-go-race/main.go:89 +0x29d

Goroutine 116 (finished) created at:
  github.com/minio/minio-go.Client.putObjectMultipartStreamFromReadAt()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-streaming.go:147 +0x7e3
  github.com/minio/minio-go.Client.putObjectMultipartStream()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-streaming.go:45 +0x1ba
  github.com/minio/minio-go.Client.putObjectCommon()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object.go:164 +0x765
  github.com/minio/minio-go.Client.PutObjectWithContext()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-context.go:32 +0x205
  github.com/minio/minio-go.Client.PutObject()
      /home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object.go:135 +0x568
  main.main.func1()
      /home/findmyname/Documents/projects/testing/minio-go-race/main.go:89 +0x29d
==================
file uploaded, size '1048576000'
populating ch ID 11
file uploaded, size '1048576000'
populating ch ID 12
file uploaded, size '1048576000'
populating ch ID 13
file uploaded, size '1048576000'
populating ch ID 14
file uploaded, size '1048576000'
populating ch ID 15
file uploaded, size '1048576000'
file uploaded, size '1048576000'
populating ch ID 16
populating ch ID 17
file uploaded, size '1048576000'
populating ch ID 18
file uploaded, size '1048576000'
populating ch ID 19
file uploaded, size '1048576000'
file uploaded, size '1048576000'
file uploaded, size '1048576000'
file uploaded, size '1048576000'
file uploaded, size '1048576000'
file uploaded, size '1048576000'
file uploaded, size '1048576000'
file uploaded, size '1048576000'
file uploaded, size '1048576000'
file uploaded, size '1048576000'
file uploaded, size '1048576000'
Found 1 data race(s)

Could you advise whether is there anyting wrong on my end ?
@harshavardhana
Copy link
Member

Goroutine 117 (running) created at:
github.com/minio/minio-go.Client.putObjectMultipartStreamFromReadAt()
/home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-streaming.go:147 +0x7e3
github.com/minio/minio-go.Client.putObjectMultipartStream()
/home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-streaming.go:45 +0x1ba
github.com/minio/minio-go.Client.putObjectCommon()
/home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object.go:164 +0x765
github.com/minio/minio-go.Client.PutObjectWithContext()
/home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object-context.go:32 +0x205
github.com/minio/minio-go.Client.PutObject()
/home/findmyname/go/pkg/mod/github.com/minio/[email protected]+incompatible/api-put-object.go:135 +0x568
main.main.func1()
/home/findmyname/Documents/projects/testing/minio-go-race/main.go:89 +0x29d

This has been long fixed you are using an older version here stick to go modules and stick to v7

"github.com/minio/minio-go/v7"

@harshavardhana harshavardhana self-assigned this Jul 25, 2020
@findmyname666
Copy link
Author

@harshavardhana just did quick test of v7 and it seems to be fixed there. Thx.

@findmyname666
Copy link
Author

findmyname666 commented Jul 29, 2020 •
edited
Loading

@harshavardhana at the end i got the data race in my app :(

=================
WARNING: DATA RACE
Write at 0x00c000682408 by goroutine 67:
  io.(*SectionReader).Seek()
      /usr/local/go/src/io/io.go:500 +0xff
  github.com/minio/minio-go/v7.Client.executeMethod()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api.go:564 +0x1326
  github.com/minio/minio-go/v7.Client.uploadPart()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-multipart.go:282 +0x529
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt.func2()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:171 +0x504

Previous read at 0x00c000682408 by goroutine 36:
  io.(*SectionReader).Read()
      /usr/local/go/src/io/io.go:472 +0x4a
  io.(*LimitedReader).Read()
      /usr/local/go/src/io/io.go:451 +0xc7
  io.copyBuffer()
      /usr/local/go/src/io/io.go:405 +0x143
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb0
  net.genericReadFrom()
      /usr/local/go/src/net/net.go:626 +0x32
  net.(*TCPConn).readFrom()
      /usr/local/go/src/net/tcpsock_posix.go:54 +0x171
  net.(*TCPConn).ReadFrom()
      /usr/local/go/src/net/tcpsock.go:103 +0x79
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb5
  net/http.persistConnWriter.ReadFrom()
      /usr/local/go/src/net/http/transport.go:1672 +0x32
  bufio.(*Writer).ReadFrom()
      /usr/local/go/src/bufio/bufio.go:714 +0x52c
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0x78
  net/http.(*transferWriter).doBodyCopy()
      /usr/local/go/src/net/http/transfer.go:400 +0x2f
  net/http.(*transferWriter).writeBody()
      /usr/local/go/src/net/http/transfer.go:359 +0xc11
  net/http.(*Request).write()
      /usr/local/go/src/net/http/request.go:685 +0x859
  net/http.(*persistConn).writeLoop()
      /usr/local/go/src/net/http/transport.go:2280 +0x331

Goroutine 67 (running) created at:
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:151 +0x6e7
  github.com/minio/minio-go/v7.Client.putObjectMultipartStream()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:49 +0x7a6
  github.com/minio/minio-go/v7.Client.putObjectCommon()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:241 +0x929
  github.com/minio/minio-go/v7.Client.PutObject()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:208 +0x2a1
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).uploadFile()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:79 +0x386
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).ExecUpload()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:114 +0x15f
  main.main.func1()
      /home/findmyname/Documents/showmax/ops/minio-uploader/main.go:99 +0x4be

Goroutine 36 (running) created at:
  net/http.(*Transport).dialConn()
      /usr/local/go/src/net/http/transport.go:1648 +0xc33
  net/http.(*Transport).dialConnFor()
      /usr/local/go/src/net/http/transport.go:1365 +0x14f
==================
==================
WARNING: DATA RACE
Write at 0x00c00023a588 by goroutine 59:
  io.(*SectionReader).Seek()
      /usr/local/go/src/io/io.go:500 +0xff
  github.com/minio/minio-go/v7.Client.executeMethod()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api.go:564 +0x1326
  github.com/minio/minio-go/v7.Client.uploadPart()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-multipart.go:282 +0x529
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt.func2()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:171 +0x504

Previous write at 0x00c00023a588 by goroutine 107:
  io.(*SectionReader).Read()
      /usr/local/go/src/io/io.go:479 +0x1b3
  io.(*LimitedReader).Read()
      /usr/local/go/src/io/io.go:451 +0xc7
  io.copyBuffer()
      /usr/local/go/src/io/io.go:405 +0x143
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb0
  net.genericReadFrom()
      /usr/local/go/src/net/net.go:626 +0x32
  net.(*TCPConn).readFrom()
      /usr/local/go/src/net/tcpsock_posix.go:54 +0x171
  net.(*TCPConn).ReadFrom()
      /usr/local/go/src/net/tcpsock.go:103 +0x79
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb5
  net/http.persistConnWriter.ReadFrom()
      /usr/local/go/src/net/http/transport.go:1672 +0x32
  bufio.(*Writer).ReadFrom()
      /usr/local/go/src/bufio/bufio.go:714 +0x52c
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0x78
  net/http.(*transferWriter).doBodyCopy()
      /usr/local/go/src/net/http/transfer.go:400 +0x2f
  net/http.(*transferWriter).writeBody()
      /usr/local/go/src/net/http/transfer.go:359 +0xc11
  net/http.(*Request).write()
      /usr/local/go/src/net/http/request.go:685 +0x859
  net/http.(*persistConn).writeLoop()
      /usr/local/go/src/net/http/transport.go:2280 +0x331

Goroutine 59 (running) created at:
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:151 +0x6e7
  github.com/minio/minio-go/v7.Client.putObjectMultipartStream()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:49 +0x7a6
  github.com/minio/minio-go/v7.Client.putObjectCommon()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:241 +0x929
  github.com/minio/minio-go/v7.Client.PutObject()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:208 +0x2a1
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).uploadFile()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:79 +0x386
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).ExecUpload()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:114 +0x15f
  main.main.func1()
      /home/findmyname/Documents/showmax/ops/minio-uploader/main.go:99 +0x4be

Goroutine 107 (finished) created at:
  net/http.(*Transport).dialConn()
      /usr/local/go/src/net/http/transport.go:1648 +0xc33
  net/http.(*Transport).dialConnFor()
      /usr/local/go/src/net/http/transport.go:1365 +0x14f
==================
==================
WARNING: DATA RACE
Write at 0x00c00023af18 by goroutine 56:
  io.(*SectionReader).Seek()
      /usr/local/go/src/io/io.go:500 +0xff
  github.com/minio/minio-go/v7.Client.executeMethod()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api.go:564 +0x1326
  github.com/minio/minio-go/v7.Client.uploadPart()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-multipart.go:282 +0x529
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt.func2()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:171 +0x504

Previous write at 0x00c00023af18 by goroutine 138:
  ??()
      -:0 +0xffffffffffffffff
  net.(*netFD).Write()
      /usr/local/go/src/net/fd_unix.go:220 +0x65
  net.(*conn).Write()
      /usr/local/go/src/net/net.go:196 +0xa0
  net.(*TCPConn).Write()
      <autogenerated>:1 +0x69
  net.(*writerOnly).Write()
      <autogenerated>:1 +0x87
  io.copyBuffer()
      /usr/local/go/src/io/io.go:407 +0x282
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb0
  net.genericReadFrom()
      /usr/local/go/src/net/net.go:626 +0x32
  net.(*TCPConn).readFrom()
      /usr/local/go/src/net/tcpsock_posix.go:54 +0x171
  net.(*TCPConn).ReadFrom()
      /usr/local/go/src/net/tcpsock.go:103 +0x79
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb5
  net/http.persistConnWriter.ReadFrom()
      /usr/local/go/src/net/http/transport.go:1672 +0x32
  bufio.(*Writer).ReadFrom()
      /usr/local/go/src/bufio/bufio.go:714 +0x52c
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0x78
  net/http.(*transferWriter).doBodyCopy()
      /usr/local/go/src/net/http/transfer.go:400 +0x2f
  net/http.(*transferWriter).writeBody()
      /usr/local/go/src/net/http/transfer.go:359 +0xc11
  net/http.(*Request).write()
      /usr/local/go/src/net/http/request.go:685 +0x859
  net/http.(*persistConn).writeLoop()
      /usr/local/go/src/net/http/transport.go:2280 +0x331

Goroutine 56 (running) created at:
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:151 +0x6e7
  github.com/minio/minio-go/v7.Client.putObjectMultipartStream()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:49 +0x7a6
  github.com/minio/minio-go/v7.Client.putObjectCommon()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:241 +0x929
  github.com/minio/minio-go/v7.Client.PutObject()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:208 +0x2a1
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).uploadFile()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:79 +0x386
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).ExecUpload()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:114 +0x15f
  main.main.func1()
      /home/findmyname/Documents/showmax/ops/minio-uploader/main.go:99 +0x4be

Goroutine 138 (finished) created at:
  net/http.(*Transport).dialConn()
      /usr/local/go/src/net/http/transport.go:1648 +0xc33
  net/http.(*Transport).dialConnFor()
      /usr/local/go/src/net/http/transport.go:1365 +0x14f
==================

@harshavardhana
Copy link
Member

@harshavardhana at the end i got the data race in my app :(

=================
WARNING: DATA RACE
Write at 0x00c000682408 by goroutine 67:
  io.(*SectionReader).Seek()
      /usr/local/go/src/io/io.go:500 +0xff
  github.com/minio/minio-go/v7.Client.executeMethod()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api.go:564 +0x1326
  github.com/minio/minio-go/v7.Client.uploadPart()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-multipart.go:282 +0x529
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt.func2()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:171 +0x504

Previous read at 0x00c000682408 by goroutine 36:
  io.(*SectionReader).Read()
      /usr/local/go/src/io/io.go:472 +0x4a
  io.(*LimitedReader).Read()
      /usr/local/go/src/io/io.go:451 +0xc7
  io.copyBuffer()
      /usr/local/go/src/io/io.go:405 +0x143
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb0
  net.genericReadFrom()
      /usr/local/go/src/net/net.go:626 +0x32
  net.(*TCPConn).readFrom()
      /usr/local/go/src/net/tcpsock_posix.go:54 +0x171
  net.(*TCPConn).ReadFrom()
      /usr/local/go/src/net/tcpsock.go:103 +0x79
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb5
  net/http.persistConnWriter.ReadFrom()
      /usr/local/go/src/net/http/transport.go:1672 +0x32
  bufio.(*Writer).ReadFrom()
      /usr/local/go/src/bufio/bufio.go:714 +0x52c
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0x78
  net/http.(*transferWriter).doBodyCopy()
      /usr/local/go/src/net/http/transfer.go:400 +0x2f
  net/http.(*transferWriter).writeBody()
      /usr/local/go/src/net/http/transfer.go:359 +0xc11
  net/http.(*Request).write()
      /usr/local/go/src/net/http/request.go:685 +0x859
  net/http.(*persistConn).writeLoop()
      /usr/local/go/src/net/http/transport.go:2280 +0x331

Goroutine 67 (running) created at:
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:151 +0x6e7
  github.com/minio/minio-go/v7.Client.putObjectMultipartStream()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:49 +0x7a6
  github.com/minio/minio-go/v7.Client.putObjectCommon()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:241 +0x929
  github.com/minio/minio-go/v7.Client.PutObject()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:208 +0x2a1
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).uploadFile()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:79 +0x386
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).ExecUpload()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:114 +0x15f
  main.main.func1()
      /home/findmyname/Documents/showmax/ops/minio-uploader/main.go:99 +0x4be

Goroutine 36 (running) created at:
  net/http.(*Transport).dialConn()
      /usr/local/go/src/net/http/transport.go:1648 +0xc33
  net/http.(*Transport).dialConnFor()
      /usr/local/go/src/net/http/transport.go:1365 +0x14f
==================
==================
WARNING: DATA RACE
Write at 0x00c00023a588 by goroutine 59:
  io.(*SectionReader).Seek()
      /usr/local/go/src/io/io.go:500 +0xff
  github.com/minio/minio-go/v7.Client.executeMethod()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api.go:564 +0x1326
  github.com/minio/minio-go/v7.Client.uploadPart()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-multipart.go:282 +0x529
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt.func2()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:171 +0x504

Previous write at 0x00c00023a588 by goroutine 107:
  io.(*SectionReader).Read()
      /usr/local/go/src/io/io.go:479 +0x1b3
  io.(*LimitedReader).Read()
      /usr/local/go/src/io/io.go:451 +0xc7
  io.copyBuffer()
      /usr/local/go/src/io/io.go:405 +0x143
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb0
  net.genericReadFrom()
      /usr/local/go/src/net/net.go:626 +0x32
  net.(*TCPConn).readFrom()
      /usr/local/go/src/net/tcpsock_posix.go:54 +0x171
  net.(*TCPConn).ReadFrom()
      /usr/local/go/src/net/tcpsock.go:103 +0x79
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb5
  net/http.persistConnWriter.ReadFrom()
      /usr/local/go/src/net/http/transport.go:1672 +0x32
  bufio.(*Writer).ReadFrom()
      /usr/local/go/src/bufio/bufio.go:714 +0x52c
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0x78
  net/http.(*transferWriter).doBodyCopy()
      /usr/local/go/src/net/http/transfer.go:400 +0x2f
  net/http.(*transferWriter).writeBody()
      /usr/local/go/src/net/http/transfer.go:359 +0xc11
  net/http.(*Request).write()
      /usr/local/go/src/net/http/request.go:685 +0x859
  net/http.(*persistConn).writeLoop()
      /usr/local/go/src/net/http/transport.go:2280 +0x331

Goroutine 59 (running) created at:
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:151 +0x6e7
  github.com/minio/minio-go/v7.Client.putObjectMultipartStream()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:49 +0x7a6
  github.com/minio/minio-go/v7.Client.putObjectCommon()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:241 +0x929
  github.com/minio/minio-go/v7.Client.PutObject()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:208 +0x2a1
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).uploadFile()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:79 +0x386
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).ExecUpload()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:114 +0x15f
  main.main.func1()
      /home/findmyname/Documents/showmax/ops/minio-uploader/main.go:99 +0x4be

Goroutine 107 (finished) created at:
  net/http.(*Transport).dialConn()
      /usr/local/go/src/net/http/transport.go:1648 +0xc33
  net/http.(*Transport).dialConnFor()
      /usr/local/go/src/net/http/transport.go:1365 +0x14f
==================
==================
WARNING: DATA RACE
Write at 0x00c00023af18 by goroutine 56:
  io.(*SectionReader).Seek()
      /usr/local/go/src/io/io.go:500 +0xff
  github.com/minio/minio-go/v7.Client.executeMethod()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api.go:564 +0x1326
  github.com/minio/minio-go/v7.Client.uploadPart()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-multipart.go:282 +0x529
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt.func2()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:171 +0x504

Previous write at 0x00c00023af18 by goroutine 138:
  ??()
      -:0 +0xffffffffffffffff
  net.(*netFD).Write()
      /usr/local/go/src/net/fd_unix.go:220 +0x65
  net.(*conn).Write()
      /usr/local/go/src/net/net.go:196 +0xa0
  net.(*TCPConn).Write()
      <autogenerated>:1 +0x69
  net.(*writerOnly).Write()
      <autogenerated>:1 +0x87
  io.copyBuffer()
      /usr/local/go/src/io/io.go:407 +0x282
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb0
  net.genericReadFrom()
      /usr/local/go/src/net/net.go:626 +0x32
  net.(*TCPConn).readFrom()
      /usr/local/go/src/net/tcpsock_posix.go:54 +0x171
  net.(*TCPConn).ReadFrom()
      /usr/local/go/src/net/tcpsock.go:103 +0x79
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0xb5
  net/http.persistConnWriter.ReadFrom()
      /usr/local/go/src/net/http/transport.go:1672 +0x32
  bufio.(*Writer).ReadFrom()
      /usr/local/go/src/bufio/bufio.go:714 +0x52c
  io.copyBuffer()
      /usr/local/go/src/io/io.go:391 +0x3fa
  io.Copy()
      /usr/local/go/src/io/io.go:364 +0x78
  net/http.(*transferWriter).doBodyCopy()
      /usr/local/go/src/net/http/transfer.go:400 +0x2f
  net/http.(*transferWriter).writeBody()
      /usr/local/go/src/net/http/transfer.go:359 +0xc11
  net/http.(*Request).write()
      /usr/local/go/src/net/http/request.go:685 +0x859
  net/http.(*persistConn).writeLoop()
      /usr/local/go/src/net/http/transport.go:2280 +0x331

Goroutine 56 (running) created at:
  github.com/minio/minio-go/v7.Client.putObjectMultipartStreamFromReadAt()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:151 +0x6e7
  github.com/minio/minio-go/v7.Client.putObjectMultipartStream()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object-streaming.go:49 +0x7a6
  github.com/minio/minio-go/v7.Client.putObjectCommon()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:241 +0x929
  github.com/minio/minio-go/v7.Client.PutObject()
      /home/findmyname/go/pkg/mod/github.com/minio/minio-go/[email protected]/api-put-object.go:208 +0x2a1
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).uploadFile()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:79 +0x386
  go.showmax.cc/ops/minio-uploader/minio.(*MinioOpt).ExecUpload()
      /home/findmyname/Documents/showmax/ops/minio-uploader/minio/minio.go:114 +0x15f
  main.main.func1()
      /home/findmyname/Documents/showmax/ops/minio-uploader/main.go:99 +0x4be

Goroutine 138 (finished) created at:
  net/http.(*Transport).dialConn()
      /usr/local/go/src/net/http/transport.go:1648 +0xc33
  net/http.(*Transport).dialConnFor()
      /usr/local/go/src/net/http/transport.go:1365 +0x14f
==================

@findmyname666 this looks like an application bug, can you provide a sample code with the reader that you are passing to us?

@findmyname666
Copy link
Author

@harshavardhana sorry for later response.
I was able to replicate DATA RACE with following code:

package main

import (
	"context"
	"fmt"
	"os"
        "path/filepath"
	"sync"

	"github.com/minio/minio-go/v7"
	"github.com/minio/minio-go/v7/pkg/credentials"
)

var (
	endpoint        = "localhost:9001"
	accessKeyID     = "admin"
	secretAccessKey = "some_secret"
	useSSL          = false
	bName           = "test"
	bLocation       = ""
	workers         = 5
	basePath        = "/run/logs/dump/2019/06/06" // override the path
	c               = context.Background()
)

// initializes MinIO client
func createMC() (*minio.Client, error) {
	mc, err := minio.New(
		endpoint,
		&minio.Options{
			Creds:  credentials.NewStaticV4(accessKeyID, secretAccessKey, ""),
			Secure: useSSL,
		},
	)

	return mc, err
}

// creates new bucket
func createBucket(mc *minio.Client) error {
	exists, err := mc.BucketExists(c, bName)
	if err == nil && exists {
		return nil
	} else if err != nil {
		return err
	}
	if err := mc.MakeBucket(c, bName, minio.MakeBucketOptions{}); err != nil {
		return err
	}
	return nil
}

func uploadFile(c context.Context, mc *minio.Client, f *os.File, path string, size int64) (err error) {
	defer f.Close()
	// tag object with expiration
	oOptions := minio.PutObjectOptions{
		UserTags: map[string]string{
			"weekly_expiration": "1",
		},
	}

	_, err = mc.PutObject(
		c,
		bName,
		path[1:], // strip leading slash
		f,
		size,
		oOptions,
	)

	return err
}

// generate WalkFunc for filepath.Walk
// WalkFunc is the type of the function called for each file or directory visited by Walk see https://golang.org/pkg/path/filepath/#WalkFunc
// we want to have channel available in WalkFunc therefore it is implemented as closure
// file path is send to channel for upload to MinIO
func genWalkFunc(toUpload chan<- string) filepath.WalkFunc {
	return func(path string, info os.FileInfo, err error) error {
		if err != nil {
			fmt.Printf(
				"[walker] incoming error detected, path '%s', err '%s'\n", path, err,
			)
			return nil
		}
		if info.IsDir() {
			fmt.Printf(
				"[walker] skipping path '%s', isDir '%v'\n", path, info.IsDir(),
			)
		} else {
			fmt.Printf("[walker] valid file found '%s'\n", path)
			toUpload <- path
		}
		return nil
	}
}

// this is entry point for execution of recursive walk through filesystem starting at base path
func walkerExec(basePath string, toUpload chan<- string) {
	defer func() {
		fmt.Println("[walker] closing channel")
		close(toUpload)
	}()

	err := filepath.Walk(
		basePath,
		genWalkFunc(toUpload),
	)
	if err != nil {
		fmt.Printf(
			"[walker] failed to recursively walk through base path '%s', %s\n",
			basePath, err,
		)
	}
}

func main() {
	mc, err := createMC()
	if err != nil {
		fmt.Printf("failed to create MinIO client, %v\n", err)
		os.Exit(1)
	}
	if err := createBucket(mc); err != nil {
		fmt.Printf("failed to create bucket, %v\n", err)
		os.Exit(1)
	}

	toUpload := make(chan string, 5)
	go walkerExec(basePath, toUpload)

	var wg sync.WaitGroup
	for i := 1; i < 6; i++ {
		fmt.Printf("worker is starting ID %d\n", i)
		wg.Add(1)
		go func(i int) {
			defer wg.Done()
			fmt.Printf("worker started ID %d\n", i)
			for path := range toUpload {
				fInfo, err := os.Stat(path)
				if err != nil {
					fmt.Printf("failed to stat file '%s'\n", path)
					continue
				}
				f, err := os.Open(path)
				if err != nil {
					fmt.Printf("failed to open file, path %s, %v\n", path, err)
					continue
				}
				fmt.Printf("uploading file '%s'\n", path)
				if err := uploadFile(c, mc, f, path, fInfo.Size()); err != nil {
					fmt.Printf("upload failed, path %s, %v\n", path, err)
					continue
				}
				fmt.Printf("file uploaded, path '%s'\n", path)
			}
		}(i)
	}
	wg.Wait()
}

Please note that size of uploading files is ranging from several KB to 200GB.
Looking forward for your reply.

@harshavardhana
Copy link
Member

@harshavardhana sorry for later response.
I was able to replicate DATA RACE with following code:

package main

import (
	"context"
	"fmt"
	"os"
        "path/filepath"
	"sync"

	"github.com/minio/minio-go/v7"
	"github.com/minio/minio-go/v7/pkg/credentials"
)

var (
	endpoint        = "localhost:9001"
	accessKeyID     = "admin"
	secretAccessKey = "some_secret"
	useSSL          = false
	bName           = "test"
	bLocation       = ""
	workers         = 5
	basePath        = "/run/logs/dump/2019/06/06" // override the path
	c               = context.Background()
)

// initializes MinIO client
func createMC() (*minio.Client, error) {
	mc, err := minio.New(
		endpoint,
		&minio.Options{
			Creds:  credentials.NewStaticV4(accessKeyID, secretAccessKey, ""),
			Secure: useSSL,
		},
	)

	return mc, err
}

// creates new bucket
func createBucket(mc *minio.Client) error {
	exists, err := mc.BucketExists(c, bName)
	if err == nil && exists {
		return nil
	} else if err != nil {
		return err
	}
	if err := mc.MakeBucket(c, bName, minio.MakeBucketOptions{}); err != nil {
		return err
	}
	return nil
}

func uploadFile(c context.Context, mc *minio.Client, f *os.File, path string, size int64) (err error) {
	defer f.Close()
	// tag object with expiration
	oOptions := minio.PutObjectOptions{
		UserTags: map[string]string{
			"weekly_expiration": "1",
		},
	}

	_, err = mc.PutObject(
		c,
		bName,
		path[1:], // strip leading slash
		f,
		size,
		oOptions,
	)

	return err
}

// generate WalkFunc for filepath.Walk
// WalkFunc is the type of the function called for each file or directory visited by Walk see https://golang.org/pkg/path/filepath/#WalkFunc
// we want to have channel available in WalkFunc therefore it is implemented as closure
// file path is send to channel for upload to MinIO
func genWalkFunc(toUpload chan<- string) filepath.WalkFunc {
	return func(path string, info os.FileInfo, err error) error {
		if err != nil {
			fmt.Printf(
				"[walker] incoming error detected, path '%s', err '%s'\n", path, err,
			)
			return nil
		}
		if info.IsDir() {
			fmt.Printf(
				"[walker] skipping path '%s', isDir '%v'\n", path, info.IsDir(),
			)
		} else {
			fmt.Printf("[walker] valid file found '%s'\n", path)
			toUpload <- path
		}
		return nil
	}
}

// this is entry point for execution of recursive walk through filesystem starting at base path
func walkerExec(basePath string, toUpload chan<- string) {
	defer func() {
		fmt.Println("[walker] closing channel")
		close(toUpload)
	}()

	err := filepath.Walk(
		basePath,
		genWalkFunc(toUpload),
	)
	if err != nil {
		fmt.Printf(
			"[walker] failed to recursively walk through base path '%s', %s\n",
			basePath, err,
		)
	}
}

func main() {
	mc, err := createMC()
	if err != nil {
		fmt.Printf("failed to create MinIO client, %v\n", err)
		os.Exit(1)
	}
	if err := createBucket(mc); err != nil {
		fmt.Printf("failed to create bucket, %v\n", err)
		os.Exit(1)
	}

	toUpload := make(chan string, 5)
	go walkerExec(basePath, toUpload)

	var wg sync.WaitGroup
	for i := 1; i < 6; i++ {
		fmt.Printf("worker is starting ID %d\n", i)
		wg.Add(1)
		go func(i int) {
			defer wg.Done()
			fmt.Printf("worker started ID %d\n", i)
			for path := range toUpload {
				fInfo, err := os.Stat(path)
				if err != nil {
					fmt.Printf("failed to stat file '%s'\n", path)
					continue
				}
				f, err := os.Open(path)
				if err != nil {
					fmt.Printf("failed to open file, path %s, %v\n", path, err)
					continue
				}
				fmt.Printf("uploading file '%s'\n", path)
				if err := uploadFile(c, mc, f, path, fInfo.Size()); err != nil {
					fmt.Printf("upload failed, path %s, %v\n", path, err)
					continue
				}
				fmt.Printf("file uploaded, path '%s'\n", path)
			}
		}(i)
	}
	wg.Wait()
}

Please note that size of uploading files is ranging from several KB to 200GB.
Looking forward for your reply.

I will take a look soon thanks for the reproducer

harshavardhana added a commit to harshavardhana/minio-go that referenced this issue Aug 2, 2020
@harshavardhana
fix: avoid data race by copying the buffer
60d78b9 
fixes minio#1355
@harshavardhana harshavardhana mentioned this issue Aug 2, 2020
Merged
harshavardhana added a commit to harshavardhana/minio-go that referenced this issue Aug 2, 2020
@harshavardhana
fix: avoid data race by copying the buffer
9dabb50 
fixes minio#1355
harshavardhana added a commit that referenced this issue Aug 4, 2020
@harshavardhana
fix: avoid data race by copying the buffer (#1357)
f0c6c8f 
fixes #1355
@findmyname666
Copy link
Author

@harshavardhana is there date for release ? cannot way ;)

@klauspost
Copy link
Contributor

@findmyname666 It has been released.

@findmyname666
Copy link
Author

@klauspost yeeah i noticed e.g. it was released a few hours after i asked by @harshavardhana.
Thanks for that.

P.S.: So far it works without race 👍

@bwplotka bwplotka mentioned this issue Mar 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@harshavardhana harshavardhana

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: avoid data race by copying the buffer harshavardhana/minio-go
3 participants
@harshavardhana @klauspost @findmyname666