Skip to content

executor: fix data race in TestFastAnalyze#12910

Merged
sre-bot merged 2 commits intopingcap:masterfrom
SunRunAway:datarace
Oct 23, 2019
Merged

executor: fix data race in TestFastAnalyze#12910
sre-bot merged 2 commits intopingcap:masterfrom
SunRunAway:datarace

Conversation

@SunRunAway
Copy link
Contributor

@SunRunAway SunRunAway commented Oct 23, 2019

What problem does this PR solve?

Fix a data race.

[2019-10-23T13:18:58.735Z] ==================
[2019-10-23T13:18:58.735Z] WARNING: DATA RACE
[2019-10-23T13:18:58.735Z] Write at 0x000004641498 by goroutine 595:
[2019-10-23T13:18:58.735Z]   github.com/pingcap/tidb/executor_test.(*testFastAnalyze).TestFastAnalyze()
[2019-10-23T13:18:58.735Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/tidb.go:134 +0x273
[2019-10-23T13:18:58.735Z]   runtime.call32()
[2019-10-23T13:18:58.735Z]       /usr/local/go/src/runtime/asm_amd64.s:539 +0x3a
[2019-10-23T13:18:58.735Z]   reflect.Value.Call()
[2019-10-23T13:18:58.735Z]       /usr/local/go/src/reflect/value.go:321 +0xd3
[2019-10-23T13:18:58.735Z]   github.com/pingcap/check.(*suiteRunner).forkTest.func1()
[2019-10-23T13:18:58.735Z]       /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-10-23T13:18:58.735Z]   github.com/pingcap/check.(*suiteRunner).forkCall.func1()
[2019-10-23T13:18:58.735Z]       /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-10-23T13:18:58.735Z] 
[2019-10-23T13:18:58.735Z] Previous read at 0x000004641498 by goroutine 504:
[2019-10-23T13:18:58.735Z]   github.com/pingcap/tidb/session.(*domainMap).Get()
[2019-10-23T13:18:58.735Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/tidb.go:68 +0x1d4
[2019-10-23T13:18:58.735Z]   github.com/pingcap/tidb/session.createSession()
[2019-10-23T13:18:58.735Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:1670 +0x7c
[2019-10-23T13:18:58.736Z]   github.com/pingcap/tidb/session.runInBootstrapSession()
[2019-10-23T13:18:58.736Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:1653 +0x60
[2019-10-23T13:18:58.736Z]   github.com/pingcap/tidb/session.BootstrapSession()
[2019-10-23T13:18:58.736Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:1574 +0x9fa
[2019-10-23T13:18:58.736Z]   github.com/pingcap/tidb/executor_test.(*testBypassSuite).TestLatch()
[2019-10-23T13:18:58.736Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/write_test.go:2142 +0x18b
[2019-10-23T13:18:58.736Z]   runtime.call32()
[2019-10-23T13:18:58.736Z]       /usr/local/go/src/runtime/asm_amd64.s:539 +0x3a
[2019-10-23T13:18:58.736Z]   reflect.Value.Call()
[2019-10-23T13:18:58.736Z]       /usr/local/go/src/reflect/value.go:321 +0xd3
[2019-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).forkTest.func1()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).forkCall.func1()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z] 
[2019-10-23T13:18:58.736Z] Goroutine 595 (running) created at:
[2019-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).forkCall()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).forkTest()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).doRun()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).asyncRun.func1()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z] 
[2019-10-23T13:18:58.736Z] Goroutine 504 (running) created at:
[2019-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).forkCall()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).forkTest()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).doRun()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z]   github.com/pingcap/check.(*suiteRunner).asyncRun.func1()
[2019-10-23T13:18:58.736Z]       /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-10-23T13:18:58.736Z] ==================

What is changed and how it works?

Use atomic to get/set the global value.

Check List

Tests

  • None

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Oct 23, 2019

Codecov Report

Merging #12910 into master will decrease coverage by 0.0832%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #12910        +/-   ##
================================================
- Coverage   80.0367%   79.9534%   -0.0833%     
================================================
  Files           465        465                
  Lines        107257     107016       -241     
================================================
- Hits          85845      85563       -282     
- Misses        14955      14995        +40     
- Partials       6457       6458         +1

@bb7133
Copy link
Member

bb7133 commented Oct 23, 2019

/run-unit-test

Copy link
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

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

LGTM

@bb7133 bb7133 added status/LGT2 Indicates that a PR has LGTM 2. status/can-merge Indicates a PR has been approved by a committer. labels Oct 23, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 23, 2019

/run-all-tests

@sre-bot sre-bot merged commit d627008 into pingcap:master Oct 23, 2019
@SunRunAway SunRunAway deleted the datarace branch October 23, 2019 16:06
lfkdsk added a commit to JustProject/tidb that referenced this pull request Oct 26, 2019
…ect/tidb into feature-add-udf-support

* 'feature-add-udf-support' of https://github.com/JustProject/tidb: (26 commits)
  *: fix bug that the kill command doesn't work when the killed session is waiting for the pessimistic lock (pingcap#12852)
  executor: fix the projection upon the indexLookUp in indexLookUpJoin can't get result. (pingcap#12889)
  planner, executor: support create view on union (pingcap#12595)
  planner/cascades: introduce TransformationID in cascades planner (pingcap#12879)
  executor: fix data race in test (pingcap#12910)
  executor: reuse chunk row for insert on duplicate update (pingcap#12847)
  ddl: speed up tests (pingcap#12888)
  executor: speed up test (pingcap#12896)
  expression: implement vectorized evaluation for `builtinSecondSig` (pingcap#12886)
  expression: implement vectorized evaluation for `builtinJSONObjectSig` (pingcap#12663)
  expression: speed up builtinRepeatSig by using MergeNulls (pingcap#12674)
  expression: speed up unit tests under the expression package (pingcap#12887)
  store,kv: snapshot doesn't cache the non-exists kv entries lead to poor 'insert ignore' performance (pingcap#12872)
  executor: fix data race in `GetDirtyTable()` (pingcap#12767)
  domain: increase TTL to reduce the occurrence of reporting min startTS errors (pingcap#12578)
  executor: split test for speed up (pingcap#12881)
  executor: fix inconsistent of grants privileges with MySQL when executing `grant all on ...` (pingcap#12330)
  expression: implement vectorized evaluation for `builtinJSONUnquoteSig` (pingcap#12841)
  tune grpc connection count between tidb and tikv (pingcap#12884)
  Makefile: change test parallel to 8 (pingcap#12885)
  ...
@SunRunAway SunRunAway changed the title executor: fix data race in test executor: fix data race in TestFastAnalyze Nov 18, 2019
@SunRunAway SunRunAway added the type/bugfix This PR fixes a bug. label Nov 18, 2019
@tiancaiamao
Copy link
Contributor

@SunRunAway Would you like to cherry-pick this one to the release-3.0 branch?

@SunRunAway
Copy link
Contributor Author

/run-cherry-picker

@sre-bot
Copy link
Contributor

sre-bot commented Dec 3, 2019

cherry pick to release-3.0 in PR #13872

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/test status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants