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

stats: fix the data race in test #7434

Merged
merged 2 commits into from
Aug 20, 2018
Merged

stats: fix the data race in test #7434

merged 2 commits into from
Aug 20, 2018

Conversation

alivxxx
Copy link
Contributor

@alivxxx alivxxx commented Aug 20, 2018

What problem does this PR solve?

Fix the data race:

WARNING: DATA RACE
Read at 0x00c4200dc570 by goroutine 59:
 runtime.mapaccess1_fast32()
     /usr/local/go/src/runtime/hashmap_fast.go:12 +0x0
 github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.LevelHooks.Fire()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/hooks.go:27 +0x50
 github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.Entry.log()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/entry.go:96 +0x1ba
 github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.(*Entry).Debug()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/entry.go:130 +0x110
 github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.(*Entry).Debugf()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/entry.go:178 +0x140
 github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.(*Logger).Debugf()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/logger.go:118 +0xa7
 github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.Debugf()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/exported.go:117 +0x7c
 github.com/pingcap/tidb/ddl.(*worker).start()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/ddl/ddl_worker.go:138 +0x528


Previous write at 0x00c4200dc570 by goroutine 19:
 runtime.mapassign_fast32()
     /usr/local/go/src/runtime/hashmap_fast.go:350 +0x0
 github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.LevelHooks.Add()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/hooks.go:20 +0x1a0
 github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus.AddHook()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/vendor/github.com/sirupsen/logrus/exported.go:48 +0xde
 github.com/pingcap/tidb/statistics_test.(*testStatsUpdateSuite).TestLogDetailedInfo()
     /home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/statistics/update_test.go:800 +0x59f

What is changed and how it works?

Add the hook at SetUpSuite, so there is no concurrent usage of the hook.

Check List

Tests

  • Integration test

Code changes

  • Has unexported function/method change

Side effects

  • None

Related changes

  • None

@alivxxx
Copy link
Contributor Author

alivxxx commented Aug 20, 2018

/run-all-tests

@coocood
Copy link
Member

coocood commented Aug 20, 2018

LGTM

@coocood coocood added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 20, 2018
@alivxxx
Copy link
Contributor Author

alivxxx commented Aug 20, 2018

/run-common-test

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

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 20, 2018
@alivxxx alivxxx merged commit b1aef89 into pingcap:master Aug 20, 2018
@alivxxx alivxxx deleted the data-race branch August 20, 2018 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/statistics status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants