tikv / pd

Placement driver for TiKV
Apache License 2.0
1.03k stars 714 forks source link

schedule: fix datarace in `operator.check` #8264

Closed lhy1024 closed 3 weeks ago

lhy1024 commented 3 weeks ago

What problem does this PR solve?

Issue Number: Close #8263

What is changed and how does it work?

Check List

Tests

Previous write at 0x00c0004dc240 by goroutine 215: sync/atomic.CompareAndSwapInt64() /usr/lib/go/src/runtime/race_amd64.s:316 +0xb sync/atomic.CompareAndSwapInt64()

:1 +0x1a github.com/tikv/pd/pkg/schedule/operator.TestOperatorCheckConcurrently.func1() /home/lhy1024/pd/pkg/schedule/operator/operator_test.go:593 +0xbe Goroutine 212 (running) created at: github.com/tikv/pd/pkg/schedule/operator.TestOperatorCheckConcurrently() /home/lhy1024/pd/pkg/schedule/operator/operator_test.go:591 +0x487 testing.tRunner() /usr/lib/go/src/testing/testing.go:1689 +0x21e testing.(*T).Run.gowrap1() /usr/lib/go/src/testing/testing.go:1742 +0x44 Goroutine 215 (running) created at: github.com/tikv/pd/pkg/schedule/operator.TestOperatorCheckConcurrently() /home/lhy1024/pd/pkg/schedule/operator/operator_test.go:591 +0x487 testing.tRunner() /usr/lib/go/src/testing/testing.go:1689 +0x21e testing.(*T).Run.gowrap1() /usr/lib/go/src/testing/testing.go:1742 +0x44 ================== --- FAIL: TestOperatorCheckConcurrently (0.00s) testing.go:1398: race detected during execution of test FAIL FAIL github.com/tikv/pd/pkg/schedule/operator 0.103s FAIL ``` - with this pr ``` ok github.com/tikv/pd/pkg/schedule/operator 1.098s ``` ### Release note ```release-note None. ```
ti-chi-bot[bot] commented 3 weeks ago

[REVIEW NOTIFICATION]

This pull request has been approved by:

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment. After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review. Reviewer can cancel approval by submitting a request changes review.
codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.40%. Comparing base (f69d600) to head (849163a).

:exclamation: Current head 849163a differs from pull request most recent head 1665065

Please upload reports for the commit 1665065 to get more accurate results.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #8264 +/- ## ========================================== + Coverage 77.32% 77.40% +0.07% ========================================== Files 471 471 Lines 61380 61381 +1 ========================================== + Hits 47463 47512 +49 + Misses 10365 10309 -56 - Partials 3552 3560 +8 ``` | [Flag](https://app.codecov.io/gh/tikv/pd/pull/8264/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/tikv/pd/pull/8264/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) | `77.40% <75.00%> (+0.07%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#carryforward-flags-in-the-pull-request-comment) to find out more.
ti-chi-bot[bot] commented 3 weeks ago

@okJiang: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to [this](https://github.com/tikv/pd/pull/8264#pullrequestreview-2103778565): > Instructions for interacting with me using PR comments are available [here](https://prow.tidb.net/command-help). If you have questions or suggestions related to my behavior, please file an issue against the [ti-community-infra/tichi](https://github.com/ti-community-infra/tichi/issues/new?title=Prow%20issue:) repository.
HuSharp commented 3 weeks ago

please add affect label

lhy1024 commented 3 weeks ago

/merge

ti-chi-bot[bot] commented 3 weeks ago

@lhy1024: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available [here](https://prow.tidb.net/command-help). If you have questions or suggestions related to my behavior, please file an issue against the [ti-community-infra/tichi](https://github.com/ti-community-infra/tichi/issues/new?title=Prow%20issue:) repository.
ti-chi-bot[bot] commented 3 weeks ago

This pull request has been accepted and is ready to merge.

Commit hash: 849163a519735f7345894391d1d24362c59b298a