executor, store/tikv: locks exist keys for point_get & batch_point_get #21229

you06 · 2020-11-23T14:21:34Z

Signed-off-by: you06 you1474600@gmail.com

What problem does this PR solve?

Issue Number: close #21218

Problem Summary:

Currently point_get and batch_point_get only lock keys when row is not exist, this may result in unexpected failure.

What is changed and how it works?

What's Changed:

Lock all keys in point_get and batch_point_get.

How it Works:

When the key is pessimistically locked, other transactions will wait for it.

Related changes

Need to cherry-pick to the release-4.0

Check List

Tests

Unit test

Side effects

Release note

select for update locks exist keys in point_get and batch_point_get

Signed-off-by: you06 <you1474600@gmail.com>

you06 · 2020-11-23T14:48:16Z

/run-all-tests

executor/batch_point_get.go

Signed-off-by: you06 <you1474600@gmail.com>

you06 · 2020-11-24T18:49:39Z

/run-all-tests

you06 · 2020-11-24T18:53:03Z

/run-all-tests

you06 · 2020-11-25T03:16:55Z

/run-integration-ddl-test

you06 · 2020-11-25T03:20:58Z

/run-integration-common-test

store/tikv/2pc.go

Signed-off-by: you06 <you1474600@gmail.com>

executor/batch_point_get.go

executor/batch_point_get_test.go

store/tikv/2pc.go

cfzjywxk · 2020-12-01T12:30:37Z

executor/batch_point_get_test.go

+		}
+		tk2.MustExec(fmt.Sprintf("insert into %s values(5, 5, 5)", tableName))
+		go func() {
+			tk2.MustExec(fmt.Sprintf("insert into %s values(4, 4,40)", tableName))


We could use the lock error returned by the select * from t where (id, v) in (4, 4, 40) for update nowait to verify the lock does exist.
MustExec in anohter goroutine seems will not cause test failure if failed?

select for update nowait statement will lead to fail due to lock on row key, I use an err channel to collect the exec error and assert it in the test function.

cfzjywxk · 2020-12-01T12:33:58Z

executor/point_get_test.go

+		}
+		tk2.MustExec(fmt.Sprintf("insert into %s values(3, 3, 3)", tableName))
+		go func() {
+			tk2.MustExec(fmt.Sprintf("insert into %s values(2, 2, 20)", tableName))


We could check locks like above.

Signed-off-by: you06 <you1474600@gmail.com>

cfzjywxk · 2020-12-03T07:47:24Z

@jackysp @MyonKeminta PTAL

cfzjywxk · 2020-12-03T07:50:26Z

LGTM

jackysp

LGTM

you06 · 2020-12-03T11:43:22Z

The lock behavior's change brings something weird, do not merge this PR.

you06 · 2020-12-03T12:17:58Z

The lock behavior's change brings something weird, do not merge this PR.

The problem is caused by some mistake, not this PR.

cfzjywxk · 2020-12-03T13:14:58Z

/merge

ti-srebot · 2020-12-03T13:17:18Z

/run-all-tests

ti-srebot · 2020-12-03T13:29:31Z

cherry pick to release-4.0 in PR #21483

pingcap#21229) Signed-off-by: you06 <you1474600@gmail.com>

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

#21229) (#21483) Signed-off-by: ti-srebot <ti-srebot@pingcap.com> Signed-off-by: you06 <you1474600@gmail.com>

lock exist keys for point_get & batch_point_get

ed7e252

Signed-off-by: you06 <you1474600@gmail.com>

you06 added the sig/transaction SIG:Transaction label Nov 23, 2020

you06 requested a review from a team as a code owner November 23, 2020 14:21

you06 requested review from XuHuaiyu and removed request for a team November 23, 2020 14:21

Merge branch 'master' into lock-exist-keys

267edd0

github-actions bot added the sig/execution SIG execution label Nov 23, 2020

you06 added 2 commits November 23, 2020 22:22

clean unused code

9d10801

Signed-off-by: you06 <you1474600@gmail.com>

rename test function name

d23b54a

Signed-off-by: you06 <you1474600@gmail.com>

you06 requested review from lysu and cfzjywxk November 23, 2020 14:50

cfzjywxk requested review from coocood and jackysp November 24, 2020 02:23

cfzjywxk added the type/bugfix This PR fixes a bug. label Nov 24, 2020

cfzjywxk reviewed Nov 24, 2020

View reviewed changes

executor/batch_point_get.go Show resolved Hide resolved

you06 added 4 commits November 24, 2020 20:18

commit with untouched lock

8991516

Signed-off-by: you06 <you1474600@gmail.com>

fix check

7d97562

Signed-off-by: you06 <you1474600@gmail.com>

add test for rc, fix path which get handles from builder

522a8e5

Signed-off-by: you06 <you1474600@gmail.com>

Merge branch 'master' into lock-exist-keys

837e302

you06 changed the title ~~txn: locks exist keys for point_get & batch_point_get~~ executor: locks exist keys for point_get & batch_point_get Nov 25, 2020

you06 added the needs-cherry-pick-4.0 label Nov 25, 2020

jackysp reviewed Nov 25, 2020

View reviewed changes

store/tikv/2pc.go Show resolved Hide resolved

you06 added 2 commits November 26, 2020 15:37

remove redundant lock

c1f4b0f

Signed-off-by: you06 <you1474600@gmail.com>

add case for common handle in clustered index

46c74fc

Signed-off-by: you06 <you1474600@gmail.com>

cfzjywxk reviewed Nov 26, 2020

View reviewed changes

cfzjywxk requested a review from MyonKeminta December 1, 2020 12:29

cfzjywxk reviewed Dec 1, 2020

View reviewed changes

you06 added 2 commits December 3, 2020 14:32

use error channel to collect error

4e8eced

Signed-off-by: you06 <you1474600@gmail.com>

Merge branch 'master' into lock-exist-keys

4a23be5

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 3, 2020

jackysp reviewed Dec 3, 2020

View reviewed changes

ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Dec 3, 2020

ti-srebot approved these changes Dec 3, 2020

View reviewed changes

ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Dec 3, 2020

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 3, 2020

Merge branch 'master' into lock-exist-keys

7a2adf7

ti-srebot merged commit 8381461 into pingcap:master Dec 3, 2020

ti-srebot mentioned this pull request Dec 3, 2020

executor, store/tikv: locks exist keys for point_get & batch_point_get (#21229) #21483

Merged

asiafrank pushed a commit to asiafrank/tidb that referenced this pull request Dec 8, 2020

executor, store/tikv: locks exist keys for point_get & batch_point_get (

a19376b

pingcap#21229) Signed-off-by: you06 <you1474600@gmail.com>

you06 added a commit to ti-srebot/tidb that referenced this pull request Jan 26, 2021

cherry pick pingcap#21229 to release-4.0

83476e2

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot added a commit that referenced this pull request Jan 26, 2021

executor, store/tikv: locks exist keys for point_get & batch_point_get (

47beacd

#21229) (#21483) Signed-off-by: ti-srebot <ti-srebot@pingcap.com> Signed-off-by: you06 <you1474600@gmail.com>

This was referenced Jun 23, 2021

Frequently updating non-index column causing too many Lock records in the unique index key #25659

Closed

txn: add http api to change the unique index lock behaviour #25722

Closed

txn: change lock into put record for unique index key lock #25730

Merged

This was referenced Jul 14, 2021

txn: change lock into put record for unique index key lock (#25730) #26223

Merged

txn: change lock into put record for unique index key lock (#25730) #26224

Merged

txn: change lock into put record for unique index key lock (#25730) #26225

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

executor, store/tikv: locks exist keys for point_get & batch_point_get #21229

executor, store/tikv: locks exist keys for point_get & batch_point_get #21229

you06 commented Nov 23, 2020 •

edited

Loading

you06 commented Nov 23, 2020

you06 commented Nov 24, 2020

you06 commented Nov 24, 2020

you06 commented Nov 25, 2020

you06 commented Nov 25, 2020

cfzjywxk Dec 1, 2020

you06 Dec 3, 2020

cfzjywxk Dec 1, 2020

cfzjywxk commented Dec 3, 2020

cfzjywxk commented Dec 3, 2020

jackysp left a comment

you06 commented Dec 3, 2020

you06 commented Dec 3, 2020

cfzjywxk commented Dec 3, 2020

ti-srebot commented Dec 3, 2020

ti-srebot commented Dec 3, 2020

executor, store/tikv: locks exist keys for point_get & batch_point_get #21229

executor, store/tikv: locks exist keys for point_get & batch_point_get #21229

Conversation

you06 commented Nov 23, 2020 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

you06 commented Nov 23, 2020

you06 commented Nov 24, 2020

you06 commented Nov 24, 2020

you06 commented Nov 25, 2020

you06 commented Nov 25, 2020

cfzjywxk Dec 1, 2020

Choose a reason for hiding this comment

you06 Dec 3, 2020

Choose a reason for hiding this comment

cfzjywxk Dec 1, 2020

Choose a reason for hiding this comment

cfzjywxk commented Dec 3, 2020

cfzjywxk commented Dec 3, 2020

jackysp left a comment

Choose a reason for hiding this comment

you06 commented Dec 3, 2020

you06 commented Dec 3, 2020

cfzjywxk commented Dec 3, 2020

ti-srebot commented Dec 3, 2020

ti-srebot commented Dec 3, 2020

you06 commented Nov 23, 2020 •

edited

Loading