Skip to content

planner/core: fix CI data race in core_test.newStoreWithBootstrap#13860

Closed
tiancaiamao wants to merge 1 commit into
pingcap:masterfrom
tiancaiamao:bootstrap-data-race
Closed

planner/core: fix CI data race in core_test.newStoreWithBootstrap#13860
tiancaiamao wants to merge 1 commit into
pingcap:masterfrom
tiancaiamao:bootstrap-data-race

Conversation

@tiancaiamao
Copy link
Copy Markdown
Contributor

What problem does this PR solve?

[2019-12-02T09:27:09.510Z] ==================
[2019-12-02T09:27:09.510Z] WARNING: DATA RACE
[2019-12-02T09:27:09.510Z] Write at 0x000003f46268 by goroutine 179:
[2019-12-02T09:27:09.510Z]   github.com/pingcap/tidb/planner/core_test.newStoreWithBootstrap()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/tidb.go:125 +0x7b
[2019-12-02T09:27:09.510Z]   github.com/pingcap/tidb/planner/core_test.(*testPlanSuite).TestAggEliminator()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/planner/core/physical_plan_test.go:380 +0x89
[2019-12-02T09:27:09.510Z]   runtime.call32()
[2019-12-02T09:27:09.510Z]       /usr/local/go/src/runtime/asm_amd64.s:539 +0x3a
[2019-12-02T09:27:09.510Z]   reflect.Value.Call()
[2019-12-02T09:27:09.510Z]       /usr/local/go/src/reflect/value.go:321 +0xd3
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).forkTest.func1()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:836 +0x9aa
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).forkCall.func1()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:730 +0xc4
[2019-12-02T09:27:09.510Z] 
[2019-12-02T09:27:09.510Z] Previous read at 0x000003f46268 by goroutine 143:
[2019-12-02T09:27:09.510Z]   github.com/pingcap/tidb/session.(*domainMap).Get()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/tidb.go:68 +0x1b8
[2019-12-02T09:27:09.510Z]   github.com/pingcap/tidb/session.createSession()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:1582 +0x7c
[2019-12-02T09:27:09.510Z]   github.com/pingcap/tidb/session.runInBootstrapSession()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:1565 +0x60
[2019-12-02T09:27:09.510Z]   github.com/pingcap/tidb/session.BootstrapSession()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:1486 +0xa00
[2019-12-02T09:27:09.510Z]   github.com/pingcap/tidb/planner/core_test.newStoreWithBootstrap()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/planner/core/cbo_test.go:771 +0xba
[2019-12-02T09:27:09.510Z]   github.com/pingcap/tidb/planner/core_test.(*testAnalyzeSuite).TestAnalyze()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/planner/core/cbo_test.go:454 +0x89
[2019-12-02T09:27:09.510Z]   runtime.call32()
[2019-12-02T09:27:09.510Z]       /usr/local/go/src/runtime/asm_amd64.s:539 +0x3a
[2019-12-02T09:27:09.510Z]   reflect.Value.Call()
[2019-12-02T09:27:09.510Z]       /usr/local/go/src/reflect/value.go:321 +0xd3
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).forkTest.func1()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:836 +0x9aa
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).forkCall.func1()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:730 +0xc4
[2019-12-02T09:27:09.510Z] 
[2019-12-02T09:27:09.510Z] Goroutine 179 (running) created at:
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).forkCall()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:727 +0x4a3
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).forkTest()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:818 +0x1b9
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).doRun()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:659 +0x13a
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).asyncRun.func1()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:643 +0xae
[2019-12-02T09:27:09.510Z] 
[2019-12-02T09:27:09.510Z] Goroutine 143 (running) created at:
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).forkCall()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:727 +0x4a3
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).forkTest()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:818 +0x1b9
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).doRun()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:659 +0x13a
[2019-12-02T09:27:09.510Z]   github.com/pingcap/check.(*suiteRunner).asyncRun.func1()
[2019-12-02T09:27:09.510Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:643 +0xae
[2019-12-02T09:27:09.510Z] ==================

What is changed and how it works?

session.SetSchemaLease() is not thread safe.
Move it away from the newStoreWithBootstrap function.
Make sure it's only called once in the test package planner/core.

Check List

Tests

  • Unit test

Related changes

  • Need to cherry-pick to the release branch

@tiancaiamao
Copy link
Copy Markdown
Contributor Author

PTAL @zz-jason @lamxTyler

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 2, 2019

Codecov Report

Merging #13860 into master will decrease coverage by 0.2919%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             master     #13860       +/-   ##
===============================================
- Coverage   80.4239%   80.1319%   -0.292%     
===============================================
  Files           474        474               
  Lines        118512     117022     -1490     
===============================================
- Hits          95312      93772     -1540     
- Misses        15765      15835       +70     
+ Partials       7435       7415       -20

Comment thread planner/core/cbo_test.go

func init() {
// Init once here to avoid data race.
session.SetSchemaLease(0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

master 分支里面这个已经是 atomic 操作了,所以应该没影响。 release-3.x 里面这个还是直接赋值的,所以会冲突。。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If so, we should cherry-pick that one to release-3.0 branch.
Do you know which PR and who's the author @glorv

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#12910 should be this one

@tiancaiamao
Copy link
Copy Markdown
Contributor Author

Fixed in #12910

@tiancaiamao tiancaiamao closed this Dec 5, 2019
@tiancaiamao tiancaiamao deleted the bootstrap-data-race branch December 5, 2019 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants