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

executor: fix IsPointGet judgment condition #10278

Merged
merged 12 commits into from
Apr 28, 2019

Conversation

tiancaiamao
Copy link
Contributor

@tiancaiamao tiancaiamao commented Apr 26, 2019

What problem does this PR solve?

index lookup should not use the max ts optimization

What is changed and how it works?

fix IsPointGet judgment condition

Check List

Tests

  • Unit test

Related changes

  • Need to cherry-pick to the release branch

@shenli shenli added priority/release-blocker This issue blocks a release. Please solve it ASAP. type/bugfix This PR fixes a bug. labels Apr 27, 2019
@shenli
Copy link
Member

shenli commented Apr 28, 2019

@tiancaiamao Please fix CI.

executor/adapter.go Outdated Show resolved Hide resolved
@tiancaiamao
Copy link
Contributor Author

PTAL @jackysp @winkyao @shenli

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@codecov
Copy link

codecov bot commented Apr 28, 2019

Codecov Report

Merging #10278 into master will increase coverage by 0.007%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##             master     #10278       +/-   ##
===============================================
+ Coverage   77.8185%   77.8256%   +0.007%     
===============================================
  Files           410        410               
  Lines         84724      84733        +9     
===============================================
+ Hits          65931      65944       +13     
+ Misses        13863      13861        -2     
+ Partials       4930       4928        -2

go.mod Outdated
@@ -55,6 +55,7 @@ require (
github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72
github.com/struCoder/pidusage v0.1.2
github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2
github.com/tiancaiamao/debugger v0.0.0-20190426085344-91156e93f38e
Copy link
Member

Choose a reason for hiding this comment

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

Can we put this package into failpoint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

executor/adapter.go Outdated Show resolved Hide resolved
executor/executor_test.go Outdated Show resolved Hide resolved
executor/point_get.go Outdated Show resolved Hide resolved
debugger.Continue("point-get-g1")
debugger.Break(label)
tk2.MustExec("update point_get set b = 2, c = 2 where a = 1")
debugger.Continue("point-get-g1")
Copy link
Contributor

Choose a reason for hiding this comment

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

continue "point-get-g2"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

continue "point-get-g1" is right here... it means notify the other goroutine to continiue, not this one...
Notify the "select c from point_get where b = 1" transaction, to continue the second get(key)

Copy link
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

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

LGTM

@winkyao
Copy link
Contributor

winkyao commented Apr 28, 2019

Please fix ci

@tiancaiamao
Copy link
Contributor Author

I tried failpoint.InjectWithContext but with no luck, so I reset the last commit. @winkyao

@winkyao
Copy link
Contributor

winkyao commented Apr 28, 2019

@tiancaiamao well, let's focus on the bug fixing.

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@tiancaiamao tiancaiamao merged commit 98dbbff into pingcap:master Apr 28, 2019
@tiancaiamao tiancaiamao deleted the fix-point-get branch April 28, 2019 09:47
tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Apr 29, 2019
index lookup should not use the max ts optimization
tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Apr 29, 2019
jackysp added a commit that referenced this pull request Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/release-blocker This issue blocks a release. Please solve it ASAP. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants