store/tikv: forward requests by BatchCommands #23243

youjiali1995 · 2021-03-10T11:52:02Z

Signed-off-by: youjiali1995 zlwgx1023@gmail.com

What problem does this PR solve?

Problem Summary:

Support forwarding requests by BatchCommands. Based on #23362.

What is changed and how it works?

What's Changed:

Create a BatchCommands for each forwarded host in a single connection.

How it Works:

Related changes

Need to cherry-pick to the release branch

Check List

Tests

Unit test

Release note

No release note

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

tiancaiamao

If "forwardedHost" is set, the request is sent to that host, instead of the node specified by the "addr"?

So why not simply modify the SendRequest function:

func (c *RPCClient) SendRequest(ctx context.Context, addr string, req *tikvrpc.Request, timeout time.Duration) (*tikvrpc.Response, error) {
     if req.ForwardedHost != nil {
        return originalSendRequest(ctx, req.ForwardedHost, ...)
     }
     originalSendRequest(ctx, addr, ...) 
}

Add forwardedClients map[string]*batchCommandsStream to batchCommandsClient to gain the ability to send requests to other(any) nodes looks weird.

store/tikv/client.go

store/tikv/client_batch.go

youjiali1995 · 2021-03-17T01:46:51Z

If "forwardedHost" is set, the request is sent to that host, instead of the node specified by the "addr"?

No. The request is sent to the node with the address at "addr". If "forwardedHost" is set, the request will be forwarded to the "forwardedHost" by the node as a proxy, i.e. TiDB -> TiKV(addr) -> TiKV(forwardedHost) @tiancaiamao

BTW, I will split this PR into 2. One is to forward requests by unary-calls and the second is to forward requests by BatchCommands. The second one lacks tests and I will add it today.

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

…-redirection-metadata Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

youjiali1995 · 2021-03-17T10:48:48Z

/run-all-tests

youjiali1995 · 2021-03-17T11:15:58Z

/run-unit-test

…-redirection-metadata Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

…-redirection-metadata

youjiali1995 · 2021-03-18T12:40:21Z

/run-all-tests

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

…-redirection-metadata

youjiali1995 · 2021-03-18T13:29:53Z

/run-all-tests

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

youjiali1995 · 2021-03-18T14:14:43Z

/run-all-tests

tiancaiamao · 2021-03-18T14:45:50Z

/LGTM

I have to point out that the complexity of supporting the batch mode maybe not worth its value.
There might be a huge burden for future maintenance work.

MyonKeminta

LGTM

ti-chi-bot · 2021-03-18T15:10:32Z

@MyonKeminta: Please use /LGTM instead of LGTM when you want to approve the pull request by comment.
If you use the GitHub review feature, please approve the PR directly, the comment will not take effect in the GitHub review feature.
If you have any qustions please refer to lgtm command help or lgtm plugin design.

If you have approved this PR, please ignore this reply. This reply is being used as a temporary reply during the migration of the new bot and will be removed on April 1.

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-18T15:10:34Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

MyonKeminta
tiancaiamao

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.

youjiali1995 · 2021-03-18T15:11:10Z

/merge

ti-chi-bot · 2021-03-18T15:11:12Z

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

Commit hash: 94ace66

youjiali1995 · 2021-03-18T15:27:26Z

/run-unit-test

youjiali1995 · 2021-03-18T15:28:50Z

[2021-03-18T15:13:37.242Z] ----------------------------------------------------------------------
[2021-03-18T15:13:37.242Z] FAIL: simple_test.go:606: testSuite3.TestDropPartitionStats
[2021-03-18T15:13:37.242Z] 
[2021-03-18T15:13:37.242Z] simple_test.go:631:
[2021-03-18T15:13:37.242Z]     tk.MustExec("analyze table t")
[2021-03-18T15:13:37.242Z] /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:217:
[2021-03-18T15:13:37.242Z]     tk.c.Assert(err, check.IsNil, check.Commentf("sql:%s, %v, error stack %v", sql, args, errors.ErrorStack(err)))
[2021-03-18T15:13:37.242Z] ... value *errors.withStack = [stats]: some partition level statistics are not in statistics version 2, please set tidb_analyze_version to 2 and analyze the this table ("[stats]: some partition level statistics are not in statistics version 2, please set tidb_analyze_version to 2 and analyze the this table")
[2021-03-18T15:13:37.242Z] ... sql:analyze table t, [], error stack [stats]: some partition level statistics are not in statistics version 2, please set tidb_analyze_version to 2 and analyze the this table
[2021-03-18T15:13:37.242Z] github.com/pingcap/errors.AddStack
[2021-03-18T15:13:37.242Z] 	/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/errors@v0.11.5-0.20201126102027-b0a155152ca3/errors.go:174
[2021-03-18T15:13:37.242Z] github.com/pingcap/errors.Trace
[2021-03-18T15:13:37.242Z] 	/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/errors@v0.11.5-0.20201126102027-b0a155152ca3/juju_adaptor.go:15
[2021-03-18T15:13:37.242Z] github.com/pingcap/tidb/util/testkit.(*TestKit).Exec
[2021-03-18T15:13:37.242Z] 	/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:176
[2021-03-18T15:13:37.242Z] github.com/pingcap/tidb/util/testkit.(*TestKit).MustExec
[2021-03-18T15:13:37.242Z] 	/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:216
[2021-03-18T15:13:37.242Z] github.com/pingcap/tidb/executor_test.(*testSuite3).TestDropPartitionStats
[2021-03-18T15:13:37.242Z] 	/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/simple_test.go:631
[2021-03-18T15:13:37.242Z] reflect.Value.call
[2021-03-18T15:13:37.242Z] 	/usr/local/go/src/reflect/value.go:460
[2021-03-18T15:13:37.242Z] reflect.Value.Call
[2021-03-18T15:13:37.242Z] 	/usr/local/go/src/reflect/value.go:321
[2021-03-18T15:13:37.242Z] github.com/pingcap/check.(*suiteRunner).forkTest.func1
[2021-03-18T15:13:37.242Z] 	/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:850
[2021-03-18T15:13:37.242Z] github.com/pingcap/check.(*suiteRunner).forkCall.func1
[2021-03-18T15:13:37.242Z] 	/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:739
[2021-03-18T15:13:37.242Z] runtime.goexit
[2021-03-18T15:13:37.242Z] 	/usr/local/go/src/runtime/asm_amd64.s:1357
[2021-03-18T15:13:37.242Z]

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

ti-srebot · 2021-03-18T15:42:37Z

cherry pick to release-5.0 in PR #23420

youjiali1995 · 2021-03-19T09:39:52Z

/run-cherry-picker

youjiali1995 added component/tikv sig/transaction SIG:Transaction labels Mar 10, 2021

youjiali1995 requested a review from MyonKeminta March 10, 2021 11:52

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 10, 2021

store/tikv: support redirection metadata

d00ce77

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

youjiali1995 force-pushed the support-redirection-metadata branch from ee58a24 to d00ce77 Compare March 10, 2021 12:45

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 16, 2021

youjiali1995 force-pushed the support-redirection-metadata branch from af07828 to b1bb6af Compare March 16, 2021 06:10

store/tikv: forward by batchCommands

e56fed4

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

youjiali1995 force-pushed the support-redirection-metadata branch from b1bb6af to e56fed4 Compare March 16, 2021 06:31

tiancaiamao reviewed Mar 16, 2021

View reviewed changes

store/tikv/client.go Outdated Show resolved Hide resolved

store/tikv/client_batch.go Outdated Show resolved Hide resolved

youjiali1995 added 2 commits March 17, 2021 15:47

store/tikv: add tests for batchCommandsBuilder

264c2df

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

store/tikv: add tests for multiple batchRecvLoop

888f303

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

youjiali1995 changed the title ~~store/tikv: support redirection metadata~~ store/tikv: forward requests by BatchCommands Mar 17, 2021

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

4c0f852

…-redirection-metadata Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

youjiali1995 added the needs-cherry-pick-release-5.0 label Mar 17, 2021

youjiali1995 added 2 commits March 18, 2021 20:17

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

76e51ed

…-redirection-metadata Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

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

56b0085

…-redirection-metadata

youjiali1995 added 2 commits March 18, 2021 21:27

enlarge sleep time

3e68e3e

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

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

4485f4a

…-redirection-metadata

fix race in request forwarding

94ace66

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 18, 2021

MyonKeminta approved these changes Mar 18, 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 18, 2021

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

ti-chi-bot merged commit 3813da0 into pingcap:master Mar 18, 2021

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Mar 18, 2021

cherry pick pingcap#23243 to release-5.0

630a6a2

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

ti-srebot mentioned this pull request Mar 18, 2021

store/tikv: forward requests by BatchCommands (#23243) #23420

Merged

youjiali1995 deleted the support-redirection-metadata branch March 18, 2021 15:44

ti-chi-bot pushed a commit that referenced this pull request Mar 19, 2021

store/tikv: forward requests by BatchCommands (#23243) (#23420)

9ed4abb

SabaPing pushed a commit to SabaPing/tidb that referenced this pull request Mar 25, 2021

store/tikv: forward requests by BatchCommands (pingcap#23243)

41e318a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

store/tikv: forward requests by BatchCommands #23243

store/tikv: forward requests by BatchCommands #23243

youjiali1995 commented Mar 10, 2021 •

edited

Loading

tiancaiamao left a comment

youjiali1995 commented Mar 17, 2021

youjiali1995 commented Mar 17, 2021

youjiali1995 commented Mar 17, 2021

youjiali1995 commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

tiancaiamao commented Mar 18, 2021

MyonKeminta left a comment

ti-chi-bot commented Mar 18, 2021

ti-chi-bot commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

ti-chi-bot commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

ti-srebot commented Mar 18, 2021

youjiali1995 commented Mar 19, 2021

store/tikv: forward requests by BatchCommands #23243

store/tikv: forward requests by BatchCommands #23243

Conversation

youjiali1995 commented Mar 10, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

tiancaiamao left a comment

Choose a reason for hiding this comment

youjiali1995 commented Mar 17, 2021

youjiali1995 commented Mar 17, 2021

youjiali1995 commented Mar 17, 2021

youjiali1995 commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

tiancaiamao commented Mar 18, 2021

MyonKeminta left a comment

Choose a reason for hiding this comment

ti-chi-bot commented Mar 18, 2021

ti-chi-bot commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

ti-chi-bot commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

youjiali1995 commented Mar 18, 2021

ti-srebot commented Mar 18, 2021

youjiali1995 commented Mar 19, 2021

youjiali1995 commented Mar 10, 2021 •

edited

Loading