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

ThreadSafeCache Get method lock bug #1008

Merged
merged 4 commits into from Mar 30, 2018

Conversation

Projects
None yet
6 participants
@blacktear23
Contributor

blacktear23 commented Mar 28, 2018

Make cache.Get use write lock because Get method will rearrange cache's entry.

@sre-bot

This comment has been minimized.

Show comment
Hide comment
@sre-bot

sre-bot Mar 28, 2018

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

sre-bot commented Mar 28, 2018

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Mar 28, 2018

Member

/ok-to-test

Member

disksing commented Mar 28, 2018

/ok-to-test

Show outdated Hide outdated server/cache/cache.go Outdated

Please address comments.

blacktear23 added some commits Mar 30, 2018

@siddontang

LGTM

@siddontang

This comment has been minimized.

Show comment
Hide comment
@siddontang

siddontang Mar 30, 2018

Member

Can this hurt the performance now? @disksing

Member

siddontang commented Mar 30, 2018

Can this hurt the performance now? @disksing

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Mar 30, 2018

Member

It should be ok. It was Lock before #740.

Member

disksing commented Mar 30, 2018

It should be ok. It was Lock before #740.

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Mar 30, 2018

Member

/run-all-tests

Member

disksing commented Mar 30, 2018

/run-all-tests

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Mar 30, 2018

Member

/run-integration-common-test

Member

disksing commented Mar 30, 2018

/run-integration-common-test

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Mar 30, 2018

Member

CI failure not related to PD. I'm going to just merge it.

Member

disksing commented Mar 30, 2018

CI failure not related to PD. I'm going to just merge it.

@disksing disksing merged commit e221ffb into pingcap:master Mar 30, 2018

7 of 8 checks passed

jenkins-ci-pd/integration-common-test Jenkins job failed
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci-pd/build Jenkins job succeeded.
Details
jenkins-ci-pd/integration-compatibility-test Jenkins job succeeded.
Details
jenkins-ci-pd/integration-ddl-test Jenkins job succeeded.
Details
jenkins-ci-pd/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@blacktear23 blacktear23 deleted the blacktear23:thread-safe-cache-bug branch Apr 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment