store, executor: support stale read for tikv RPCContext #22176

Yisaer · 2021-01-05T07:17:43Z

What problem does this PR solve?

Issue Number: ref #21094

What is changed and how it works?

Support stale read for tikv rpcContext

Check List

Tests

Unit test

Release note

No release note

This reverts commit 7835b2c.

This reverts commit 6215804.

Signed-off-by: Song Gao <[email protected]>

store/tikv/tikvrpc/tikvrpc.go

djshow832 · 2021-03-04T06:57:37Z

distsql/request_builder.go

@@ -235,6 +236,15 @@ func (builder *RequestBuilder) SetFromSessionVars(sv *variable.SessionVars) *Req
 		builder.Request.SchemaVar = sv.TxnCtx.SchemaVersion
 	}
 	builder.txnScope = sv.TxnCtx.TxnScope
+	builder.IsStaleness = sv.TxnCtx.IsStaleness
+	if builder.IsStaleness && builder.txnScope != oracle.GlobalTxnScope {


Why doesn't global scope support stale read?

global scope also support stale read. If the txnScope is local here, we should pick store which match the label.

So is it possible to also replace build.txnScope with builder.MatchStoreLabels?

I think it is unnecessary, build.txnScope is also responsible for (builder *RequestBuilder) verifyTxnScope().

djshow832 · 2021-03-04T06:59:15Z

store/tikv/client_helper.go

+			if len(opts) > 0 {
+				if !req.StaleRead {
+					panic("req.StaleRead should be true")
+				}
+			}


How about the situation that opts must be empty when StaleRead is false?

I think this assertion is equal to your situation.

Your case is len(opts) > 0 && !req.StaleRead, my case is len(opts) == 0 && req.StaleRead.

I think len(opts) == 0 && req.StaleRead is valid, it indicates a staleness transaction with global txnScope.

Signed-off-by: Song Gao <[email protected]>

Yisaer · 2021-03-05T06:11:59Z

/run-check_dev

ti-chi-bot · 2021-03-05T08:07:10Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

djshow832
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 writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

djshow832 · 2021-03-05T08:07:18Z

/merge

ti-chi-bot · 2021-03-05T08:07:19Z

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

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 · 2021-03-05T08:07:21Z

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

Commit hash: fdc8e68

Signed-off-by: Song Gao <[email protected]>

Yisaer · 2021-03-05T08:49:40Z

/merge

ti-chi-bot · 2021-03-05T08:49:41Z

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

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 · 2021-03-05T08:49:43Z

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

Commit hash: 932dd12

ti-chi-bot · 2021-03-05T09:57:13Z

@Yisaer: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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.

Yisaer added 5 commits January 5, 2021 13:37

support staleness context

5ed556e

enable staleread

864c2f0

use EnableStaleRead

39cc9f1

use match labels

0d1a750

fix fmt

6566224

github-actions bot added the sig/execution SIG execution label Jan 5, 2021

Yisaer added 14 commits January 6, 2021 15:50

revise judge

ac661d3

add test

dfc0842

fix test

28040eb

Update executor_test.go

266765b

Update executor_test.go

a4725ef

add info

66706cc

fix test

3135a99

add debug log

6215804

Update client.go

7835b2c

Revert "Update client.go"

5149542

This reverts commit 7835b2c.

Revert "add debug log"

6ffb15a

This reverts commit 6215804.

Merge remote-tracking branch 'upstream/master' into stale_read

ff83775

add assert case

38a9631

Merge branch 'master' into stale_read

69c62b7

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

Yisaer added 3 commits February 25, 2021 13:27

update kvproto

0e645e6

Merge remote-tracking branch 'upstream/master' into stale_read

e114580

update SendReqCtx

ac7239a

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 25, 2021

remove error code

1760bf2

Signed-off-by: Song Gao <[email protected]>

Yisaer marked this pull request as ready for review February 25, 2021 06:01

Yisaer requested a review from a team as a code owner February 25, 2021 06:01

Yisaer removed the request for review from a team February 25, 2021 06:01

djshow832 reviewed Mar 4, 2021

View reviewed changes

Yisaer added 2 commits March 4, 2021 15:43

address the comment

798ecb6

Signed-off-by: Song Gao <[email protected]>

Merge remote-tracking branch 'upstream/master' into stale_read

a0ab8ac

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

Merge remote-tracking branch 'upstream/master' into stale_read

f03e716

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

make tidy

fdc8e68

djshow832 approved these changes Mar 5, 2021

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 Mar 5, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 5, 2021

ti-chi-bot and others added 4 commits March 5, 2021 16:11

Merge branch 'master' into stale_read

5775100

Merge branch 'master' into stale_read

c985b5c

Merge branch 'master' into stale_read

001a7cb

fix race

932dd12

Signed-off-by: Song Gao <[email protected]>

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 5, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 5, 2021

ti-chi-bot added 2 commits March 5, 2021 17:29

Merge branch 'master' into stale_read

3c869e0

Merge branch 'master' into stale_read

cb14023

ti-chi-bot merged commit 29908c6 into pingcap:master Mar 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

store, executor: support stale read for tikv RPCContext #22176

store, executor: support stale read for tikv RPCContext #22176

Yisaer commented Jan 5, 2021

djshow832 Mar 4, 2021

Yisaer Mar 4, 2021

djshow832 Mar 4, 2021

Yisaer Mar 5, 2021

djshow832 Mar 4, 2021

Yisaer Mar 4, 2021

djshow832 Mar 4, 2021

Yisaer Mar 5, 2021

Yisaer commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

djshow832 commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

Yisaer commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

store, executor: support stale read for tikv RPCContext #22176

store, executor: support stale read for tikv RPCContext #22176

Conversation

Yisaer commented Jan 5, 2021

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yisaer commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

djshow832 commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

Yisaer commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021

ti-chi-bot commented Mar 5, 2021