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

fix conn data race #128

Merged
merged 2 commits into from
Aug 28, 2023
Merged

fix conn data race #128

merged 2 commits into from
Aug 28, 2023

Conversation

liangjunmo
Copy link

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

Fix conn DARA RACE issue. There are some related issues like #96.

User Case Description

I use this package for sharding bill table. It runs normally on test environment, but occur DATA RACE issue on online environment.

I write unit test for this issue:

  • sharding_test.go
func TestDataRace(t *testing.T) {
	ctx, cancel := context.WithCancel(context.Background())
	ch := make(chan error)

	for i := 0; i < 2; i++ {
		go func() {
			for {
				select {
				case <-ctx.Done():
					return
				default:
					err := db.Model(&Order{}).Where("user_id", 100).Find(&[]Order{}).Error
					if err != nil {
						ch <- err
						return
					}
				}
			}
		}()
	}

	select {
	case <-time.After(time.Millisecond * 50):
		cancel()
	case err := <-ch:
		cancel()
		t.Fatal(err)
	}
}

Run this unit test on main branch with go test --race -count=1 -v --run=TestDataRace, it will output:

=== RUN   TestDataRace
==================
WARNING: DATA RACE
Write at 0x00c0000b3048 by goroutine 15:
  command-line-arguments.(*Sharding).switchConn()
      /Users/liangjunmo/workspace/gorm-sharding/sharding.go:268 +0x168
  command-line-arguments.(*Sharding).switchConn-fm()
      <autogenerated>:1 +0x44
  gorm.io/gorm.(*processor).Execute()
      /Users/liangjunmo/.gvm/pkgsets/go1.19.9/global/pkg/mod/gorm.io/[email protected]/callbacks.go:130 +0xbe1
  gorm.io/gorm.(*DB).Find()
      /Users/liangjunmo/.gvm/pkgsets/go1.19.9/global/pkg/mod/gorm.io/[email protected]/finisher_api.go:172 +0x238
  command-line-arguments.TestDataRace.func1()
      /Users/liangjunmo/workspace/gorm-sharding/sharding_test.go:413 +0x20d

Previous write at 0x00c0000b3048 by goroutine 14:
  command-line-arguments.(*Sharding).switchConn()
      /Users/liangjunmo/workspace/gorm-sharding/sharding.go:268 +0x168
  command-line-arguments.(*Sharding).switchConn-fm()
      <autogenerated>:1 +0x44
  gorm.io/gorm.(*processor).Execute()
      /Users/liangjunmo/.gvm/pkgsets/go1.19.9/global/pkg/mod/gorm.io/[email protected]/callbacks.go:130 +0xbe1
  gorm.io/gorm.(*DB).Find()
      /Users/liangjunmo/.gvm/pkgsets/go1.19.9/global/pkg/mod/gorm.io/[email protected]/finisher_api.go:172 +0x238
  command-line-arguments.TestDataRace.func1()
      /Users/liangjunmo/workspace/gorm-sharding/sharding_test.go:413 +0x20d

Goroutine 15 (running) created at:
  command-line-arguments.TestDataRace()
      /Users/liangjunmo/workspace/gorm-sharding/sharding_test.go:407 +0x87
  testing.tRunner()
      /Users/liangjunmo/.gvm/gos/go1.19.9/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /Users/liangjunmo/.gvm/gos/go1.19.9/src/testing/testing.go:1493 +0x47

Goroutine 14 (running) created at:
  command-line-arguments.TestDataRace()
      /Users/liangjunmo/workspace/gorm-sharding/sharding_test.go:407 +0x87
  testing.tRunner()
      /Users/liangjunmo/.gvm/gos/go1.19.9/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /Users/liangjunmo/.gvm/gos/go1.19.9/src/testing/testing.go:1493 +0x47
==================
    testing.go:1319: race detected during execution of test
--- FAIL: TestDataRace (0.05s)
=== CONT
    testing.go:1319: race detected during execution of test
FAIL
FAIL	command-line-arguments	0.867s
FAIL

sharding.go Outdated Show resolved Hide resolved
@huacnlee huacnlee merged commit e55cded into go-gorm:main Aug 28, 2023
8 checks passed
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.

2 participants