txn: txnManager providers GetReadSnapshot and GetForUpdateSnapshot. #35788

SpadeA-Tang · 2022-06-28T08:35:13Z

What problem does this PR solve?

Issue Number: close #35810

Problem Summary:

What is changed and how it works?

Now, the acquisition of snapshot is not managed by transaction manager and is usually done by first fetching appropriate tso and then use that tso to acquire snapshot.

This PR adds new method for TxnManager to transfer the management of snapshot acquisition to TxnManager which shields the tricy optimization when acquiring snapshot:

GetReadSnapshot() (kv.Snapshot, error)
GetForUpdateSnapshot() (kv.Snapshot, error)

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

…shot

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

ti-chi-bot · 2022-06-28T08:35:14Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lcwangchao
xhebox

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.

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

sre-bot · 2022-06-28T09:35:28Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/38cc80ad149db24e5f4d597823cbd26080b89541

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

lcwangchao · 2022-06-29T08:59:46Z

executor/adapter.go

@@ -254,7 +254,7 @@ func (a *ExecStmt) PointGet(ctx context.Context, is infoschema.InfoSchema) (*rec
 		} else {
 			// CachedPlan type is already checked in last step
 			pointGetPlan := a.PsStmt.PreparedAst.CachedPlan.(*plannercore.PointGetPlan)
-			exec.Init(pointGetPlan, startTs)
+			exec.Init(pointGetPlan, startTs, nil)


Why pass a nil snapshot?

It does not have snapshot originally here.

lcwangchao · 2022-06-29T09:07:28Z

executor/batch_point_get.go

-	snapshot.SetOption(kv.TaskID, stmtCtx.TaskID)
-	snapshot.SetOption(kv.ReadReplicaScope, e.readReplicaScope)
-	snapshot.SetOption(kv.IsStalenessReadOnly, e.isStaleness)
+	e.snapshot.SetOption(kv.TaskID, stmtCtx.TaskID)


I think kv.TaskID , kv. ReplicaRead and kv.IsStalenessReadOnly can be set in provider. And kv.IsStalenessReadOnly should and should only be set in StalenessProvider.

Yes, I will refactor it.

executor/point_get.go

sessiontxn/staleread/provider.go

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

xhebox

Overall LGTM

sessiontxn/staleread/provider.go

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

executor/builder.go

sessiontxn/staleread/provider.go

sessiontxn/interface.go

executor/builder.go

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

lcwangchao · 2022-07-01T09:44:55Z

/merge

ti-chi-bot · 2022-07-01T09:44:58Z

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

Commit hash: ffb32d9

SpadeA-Tang · 2022-07-01T10:03:18Z

/run-mysql-test

sre-bot · 2022-07-01T10:44:49Z

TiDB MergeCI notify

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci/integration-cdc-test	🟢 all 35 tests passed	26 min	Existing passed
idc-jenkins-ci-tidb/integration-common-test	🟢 all 11 tests passed	24 min	Existing passed
idc-jenkins-ci-tidb/common-test	🟢 all 12 tests passed	11 min	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	6 min 55 sec	Existing passed
idc-jenkins-ci-tidb/integration-ddl-test	🟢 all 6 tests passed	6 min 16 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	6 min 14 sec	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	6 min 7 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	3 min 16 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	2 min 59 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

SpadeA-Tang added 3 commits June 28, 2022 14:26

first try

350724f

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

Merge branch 'master' of https://github.com/pingcap/tidb into txnSnap…

7fc511b

…shot

update

e867021

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

ti-chi-bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 28, 2022

update

f427321

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

update

250a18b

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. do-not-merge/needs-linked-issue labels Jun 29, 2022

lcwangchao reviewed Jun 29, 2022

View reviewed changes

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2022

resolve conflict

ead24d2

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2022

xhebox reviewed Jun 30, 2022

View reviewed changes

sessiontxn/staleread/provider.go Outdated Show resolved Hide resolved

update

a53e97b

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 30, 2022

update

baffe2a

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2022

SpadeA-Tang added 5 commits June 30, 2022 17:53

update

b439d69

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

update

b7f6b6a

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

update

71f2f9a

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

fmt

1500718

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

Merge branch 'master' into txnSnapshot

2628117

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2022

Merge branch 'master' into txnSnapshot

ad4b406

xhebox approved these changes Jul 1, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 1, 2022

lcwangchao reviewed Jul 1, 2022

View reviewed changes

executor/builder.go Outdated Show resolved Hide resolved

executor/builder.go Outdated Show resolved Hide resolved

sessiontxn/staleread/provider.go Outdated Show resolved Hide resolved

sessiontxn/interface.go Show resolved Hide resolved

lcwangchao reviewed Jul 1, 2022

View reviewed changes

executor/builder.go Show resolved Hide resolved

SpadeA-Tang and others added 5 commits July 1, 2022 15:17

update

f87ede6

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

update method name

ddbdb43

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

Merge branch 'master' into txnSnapshot

6db862a

update

ee44d7e

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

update

ffb32d9

Signed-off-by: SpadeA-Tang <u6748471@anu.edu.au>

lcwangchao approved these changes Jul 1, 2022

View reviewed changes

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 Jul 1, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 1, 2022

Merge branch 'master' into txnSnapshot

38cc80a

ti-chi-bot merged commit 16e5815 into pingcap:master Jul 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

txn: txnManager providers GetReadSnapshot and GetForUpdateSnapshot. #35788

txn: txnManager providers GetReadSnapshot and GetForUpdateSnapshot. #35788

SpadeA-Tang commented Jun 28, 2022 •

edited

Loading

ti-chi-bot commented Jun 28, 2022 •

edited

Loading

sre-bot commented Jun 28, 2022 •

edited

Loading

lcwangchao Jun 29, 2022

SpadeA-Tang Jun 29, 2022

lcwangchao Jun 29, 2022

SpadeA-Tang Jun 29, 2022

xhebox left a comment

lcwangchao commented Jul 1, 2022

ti-chi-bot commented Jul 1, 2022

SpadeA-Tang commented Jul 1, 2022

sre-bot commented Jul 1, 2022

txn: txnManager providers GetReadSnapshot and GetForUpdateSnapshot. #35788

txn: txnManager providers GetReadSnapshot and GetForUpdateSnapshot. #35788

Conversation

SpadeA-Tang commented Jun 28, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jun 28, 2022 • edited Loading

sre-bot commented Jun 28, 2022 • edited Loading

lcwangchao Jun 29, 2022

Choose a reason for hiding this comment

SpadeA-Tang Jun 29, 2022

Choose a reason for hiding this comment

lcwangchao Jun 29, 2022

Choose a reason for hiding this comment

SpadeA-Tang Jun 29, 2022

Choose a reason for hiding this comment

xhebox left a comment

Choose a reason for hiding this comment

lcwangchao commented Jul 1, 2022

ti-chi-bot commented Jul 1, 2022

SpadeA-Tang commented Jul 1, 2022

sre-bot commented Jul 1, 2022

TiDB MergeCI notify

SpadeA-Tang commented Jun 28, 2022 •

edited

Loading

ti-chi-bot commented Jun 28, 2022 •

edited

Loading

sre-bot commented Jun 28, 2022 •

edited

Loading