Skip to content

Reduce number of goroutines in tests that became flaky after Go 1.14 migration#248

Merged
yiranwang52 merged 3 commits into
masterfrom
yiran/unit
May 6, 2020
Merged

Reduce number of goroutines in tests that became flaky after Go 1.14 migration#248
yiranwang52 merged 3 commits into
masterfrom
yiran/unit

Conversation

@yiranwang52

@yiranwang52 yiranwang52 commented May 6, 2020

Copy link
Copy Markdown
Collaborator

A couple of tests launching 1000 go routines became noticeably slower and sometimes timeout after migrating to Go 1.14.
Reducing to 200.

When testing github.com/uber/kraken/lib/persistedretry/tagreplication::TestDatabaseNotLocked locally with 1000 goroutines using Docker on Mac:

1.11: 37.844s
1.13: 37.968s
1.14: 20.447s

It seems 1.14 is faster.

But in travis CI, test in 1.11 finishes under 27sec, but in 1.14 takes more than 30sec half of the time, and there would be multiple goroutines stuck at:

goroutine 43 [semacquire]:
sync.runtime_SemacquireMutex(0xc0000903a4, 0x71c00000700, 0x1)
	/home/travis/.gimme/versions/go1.14.2.linux.amd64/src/runtime/sema.go:71 +0x47
sync.(*Mutex).lockSlow(0xc0000903a0)
	/home/travis/.gimme/versions/go1.14.2.linux.amd64/src/sync/mutex.go:138 +0x1c1
sync.(*Mutex).Lock(0xc0000903a0)
	/home/travis/.gimme/versions/go1.14.2.linux.amd64/src/sync/mutex.go:81 +0x7d
math/rand.(*lockedSource).Int63(0xc0000903a0, 0x4eb3f2301c9f5343)
	/home/travis/.gimme/versions/go1.14.2.linux.amd64/src/math/rand/rand.go:388 +0x3f
math/rand.(*Rand).Int63(...)
	/home/travis/.gimme/versions/go1.14.2.linux.amd64/src/math/rand/rand.go:85
math/rand.(*Rand).Int31(...)
	/home/travis/.gimme/versions/go1.14.2.linux.amd64/src/math/rand/rand.go:99
math/rand.(*Rand).Int31n(0xc0000961b0, 0x3e, 0x38)
	/home/travis/.gimme/versions/go1.14.2.linux.amd64/src/math/rand/rand.go:134 +0x84
math/rand.(*Rand).Intn(0xc0000961b0, 0x3e, 0x38)
	/home/travis/.gimme/versions/go1.14.2.linux.amd64/src/math/rand/rand.go:172 +0x53
math/rand.Intn(...)
	/home/travis/.gimme/versions/go1.14.2.linux.amd64/src/math/rand/rand.go:337
github.com/uber/kraken/utils/randutil.choose(0x100, 0xc40280, 0x3e, 0x40, 0x58, 0x60)
	/home/travis/gopath/src/github.com/uber/kraken/utils/randutil/randutil.go:31 +0xd2

Maybe we are competing with other processes on some syscall, and this got affected by asynchronous preemption in Go 1.14.

@codecov

codecov Bot commented May 6, 2020

Copy link
Copy Markdown

Codecov Report

Merging #248 into master will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #248      +/-   ##
==========================================
+ Coverage   66.01%   66.07%   +0.05%     
==========================================
  Files         184      184              
  Lines        9105     9105              
==========================================
+ Hits         6011     6016       +5     
+ Misses       2328     2325       -3     
+ Partials      766      764       -2     
Impacted Files Coverage Δ
lib/torrent/scheduler/events.go 72.59% <0.00%> (+1.44%) ⬆️
utils/dedup/interval_trap.go 100.00% <0.00%> (+10.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e95dce8...96ba673. Read the comment docs.

@yiranwang52 yiranwang52 changed the title Reduce number of go routines in some tests since they are slower in Go 1.14 Reduce number of go routines in some tests May 6, 2020
@yiranwang52 yiranwang52 changed the title Reduce number of go routines in some tests Reduce number of go routines in tests that became flaky after Go 1.14 migration May 6, 2020
@yiranwang52 yiranwang52 changed the title Reduce number of go routines in tests that became flaky after Go 1.14 migration Reduce number of goroutines in tests that became flaky after Go 1.14 migration May 6, 2020
@yiranwang52 yiranwang52 merged commit 78baa5e into master May 6, 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.

2 participants