Skip to content

*: fix the data race problem#3603

Merged
ti-chi-bot merged 5 commits intotikv:masterfrom
rleungx:fix-race1
Apr 19, 2021
Merged

*: fix the data race problem#3603
ti-chi-bot merged 5 commits intotikv:masterfrom
rleungx:fix-race1

Conversation

@rleungx
Copy link
Copy Markdown
Member

@rleungx rleungx commented Apr 16, 2021

What problem does this PR solve?

Closes #3601

What is changed and how it works?

This PR uses Clone to get the config.

Check List

Tests

  • Unit test

Release note

@rleungx
fix the data race problem
4011cc3 
Signed-off-by: Ryan Leung <rleungx@gmail.com>
@ti-chi-bot ti-chi-bot requested review from HunDunDM and disksing April 16, 2021 05:38
@rleungx rleungx added the component/statistics Statistics for scheduling. label Apr 16, 2021
@rleungx rleungx requested review from JmPotato and removed request for disksing April 16, 2021 05:39
JmPotato
JmPotato reviewed Apr 16, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@JmPotato JmPotato left a comment

Choose a reason for hiding this comment

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

Rest LGTM

Comment thread server/statistics/store_collection.go Outdated
HunDunDM
HunDunDM reviewed Apr 16, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@HunDunDM HunDunDM left a comment

Choose a reason for hiding this comment

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

rest LGTM

Comment thread server/statistics/store_collection.go Outdated
@rleungx
address the comment
9fe3637 
Signed-off-by: Ryan Leung <rleungx@gmail.com>
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 16, 2021
@ti-chi-bot
Copy link
Copy Markdown
Member

ti-chi-bot commented Apr 16, 2021

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • HunDunDM
  • JmPotato

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.

Details

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 16, 2021
@HunDunDM
Copy link
Copy Markdown
Member

HunDunDM commented Apr 16, 2021

Need cherry-pick?

@HunDunDM
Copy link
Copy Markdown
Member

HunDunDM commented Apr 16, 2021

/merge

@ti-chi-bot
Copy link
Copy Markdown
Member

ti-chi-bot commented Apr 16, 2021

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

/run-all-tests

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@ti-chi-bot
Copy link
Copy Markdown
Member

ti-chi-bot commented Apr 16, 2021

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

DetailsCommit hash: 01f22e1

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 16, 2021
@HunDunDM
Copy link
Copy Markdown
Member

HunDunDM commented Apr 16, 2021

/run-integration-common-test

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 16, 2021 •
edited
Loading

Codecov Report

Merging #3603 (9e9e6a8) into master (9d9c8ee) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

❗ Current head 9e9e6a8 differs from pull request most recent head da084c4. Consider uploading reports for the commit da084c4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3603      +/-   ##
==========================================
- Coverage   74.70%   74.58%   -0.12%     
==========================================
  Files         245      245              
  Lines       24139    24134       -5     
==========================================
- Hits        18033    18001      -32     
- Misses       4499     4516      +17     
- Partials     1607     1617      +10
Flag Coverage Δ
unittests 74.58% <100.00%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
server/statistics/store_collection.go 95.27% <ø> (ø)
server/config/persist_options.go 91.37% <100.00%> (+0.03%) ⬆️
server/server.go 72.65% <100.00%> (-0.56%) ⬇️
server/election/leadership.go 83.52% <0.00%> (-5.89%) ⬇️
server/schedulers/random_merge.go 60.00% <0.00%> (-3.34%) ⬇️
server/tso/allocator_manager.go 74.08% <0.00%> (-2.66%) ⬇️
server/encryptionkm/key_manager.go 71.78% <0.00%> (-1.66%) ⬇️
server/member/member.go 67.20% <0.00%> (-1.62%) ⬇️
server/schedulers/utils.go 93.10% <0.00%> (-1.38%) ⬇️
server/core/storage.go 68.44% <0.00%> (-0.77%) ⬇️
... and 5 more

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 ce22dbb...da084c4. Read the comment docs.

@JmPotato
Copy link
Copy Markdown
Member

JmPotato commented Apr 18, 2021

/run-all-tests

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 19, 2021

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
4.8% 4.8% Duplication

@ti-chi-bot ti-chi-bot merged commit 30b430f into tikv:master Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JmPotato JmPotato JmPotato approved these changes

@HunDunDM HunDunDM HunDunDM approved these changes

Assignees

No one assigned

Labels

component/statistics Statistics for scheduling. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Data race when running the unit tests

4 participants

@rleungx @ti-chi-bot @HunDunDM @JmPotato
⚡