From b4a6acb3e3fdc3daaab192b6690e364439d9e03a Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 19 Oct 2023 17:23:59 +0800 Subject: [PATCH 1/6] add backoff and region error check for GetMvccByEncodedKey API Signed-off-by: crazycs520 --- store/helper/helper.go | 43 +++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/store/helper/helper.go b/store/helper/helper.go index 1b077dcec8843..eafd458097bd7 100644 --- a/store/helper/helper.go +++ b/store/helper/helper.go @@ -96,23 +96,36 @@ func NewHelper(store Storage) *Helper { // GetMvccByEncodedKey get the MVCC value by the specific encoded key. func (h *Helper) GetMvccByEncodedKey(encodedKey kv.Key) (*kvrpcpb.MvccGetByKeyResponse, error) { - keyLocation, err := h.RegionCache.LocateKey(tikv.NewBackofferWithVars(context.Background(), 500, nil), encodedKey) - if err != nil { - return nil, derr.ToTiDBErr(err) - } + bo := tikv.NewBackofferWithVars(context.Background(), 500, nil) + for { + keyLocation, err := h.RegionCache.LocateKey(tikv.NewBackofferWithVars(context.Background(), 500, nil), encodedKey) + if err != nil { + return nil, derr.ToTiDBErr(err) + } - tikvReq := tikvrpc.NewRequest(tikvrpc.CmdMvccGetByKey, &kvrpcpb.MvccGetByKeyRequest{Key: encodedKey}) - kvResp, err := h.Store.SendReq(tikv.NewBackofferWithVars(context.Background(), 500, nil), tikvReq, keyLocation.Region, time.Minute) - if err != nil { - logutil.BgLogger().Info("get MVCC by encoded key failed", - zap.Stringer("encodeKey", encodedKey), - zap.Reflect("region", keyLocation.Region), - zap.Stringer("keyLocation", keyLocation), - zap.Reflect("kvResp", kvResp), - zap.Error(err)) - return nil, errors.Trace(err) + tikvReq := tikvrpc.NewRequest(tikvrpc.CmdMvccGetByKey, &kvrpcpb.MvccGetByKeyRequest{Key: encodedKey}) + kvResp, err := h.Store.SendReq(bo, tikvReq, keyLocation.Region, time.Minute) + if err != nil { + logutil.BgLogger().Info("get MVCC by encoded key failed", + zap.Stringer("encodeKey", encodedKey), + zap.Reflect("region", keyLocation.Region), + zap.Stringer("keyLocation", keyLocation), + zap.Reflect("kvResp", kvResp), + zap.Error(err)) + return nil, errors.Trace(err) + } + regionErr, err := kvResp.GetRegionError() + if err != nil { + return nil, errors.Trace(err) + } + if regionErr != nil { + if err = bo.Backoff(tikv.BoRegionMiss(), errors.New(regionErr.String())); err != nil { + return nil, err + } + continue + } + return kvResp.Resp.(*kvrpcpb.MvccGetByKeyResponse), nil } - return kvResp.Resp.(*kvrpcpb.MvccGetByKeyResponse), nil } // MvccKV wraps the key's mvcc info in tikv. From 184412f3670e921c58eba845d6bf556e67cac1ef Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 19 Oct 2023 19:30:22 +0800 Subject: [PATCH 2/6] add test Signed-off-by: crazycs520 --- executor/executor_failpoint_test.go | 23 +++++++++++++++++++++++ store/mockstore/unistore/rpc.go | 5 +++++ 2 files changed, 28 insertions(+) diff --git a/executor/executor_failpoint_test.go b/executor/executor_failpoint_test.go index c6eebfcdb7845..7c5e93c2b896c 100644 --- a/executor/executor_failpoint_test.go +++ b/executor/executor_failpoint_test.go @@ -28,9 +28,11 @@ import ( "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/executor" + "github.com/pingcap/tidb/meta" "github.com/pingcap/tidb/parser/terror" "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/store/copr" + "github.com/pingcap/tidb/store/helper" "github.com/pingcap/tidb/testkit" "github.com/pingcap/tidb/util/deadlockhistory" "github.com/stretchr/testify/require" @@ -622,3 +624,24 @@ func TestTiKVClientReadTimeout(t *testing.T) { explain = fmt.Sprintf("%v", rows[0]) require.Regexp(t, ".*TableReader.* root time:.*, loops:.* cop_task: {num: 1, .* rpc_num: 2.*", explain) } + +func TestGetMvccByEncodedKeyRegionError(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + h := helper.NewHelper(store.(helper.Storage)) + txn, err := store.Begin() + require.NoError(t, err) + m := meta.NewMeta(txn) + schemaVersion := tk.Session().GetDomainInfoSchema().SchemaMetaVersion() + key := m.EncodeSchemaDiffKey(schemaVersion) + + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/store/mockstore/unistore/epochNotMatch", "1*return(true)")) + defer func() { + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/store/mockstore/unistore/epochNotMatch")) + }() + resp, err := h.GetMvccByEncodedKey(key) + require.NoError(t, err) + require.NotNil(t, resp.Info) + require.Equal(t, 1, len(resp.Info.Writes)) + require.Less(t, uint64(0), resp.Info.Writes[0].CommitTs) +} diff --git a/store/mockstore/unistore/rpc.go b/store/mockstore/unistore/rpc.go index 44a44e5985751..f4dd71f503258 100644 --- a/store/mockstore/unistore/rpc.go +++ b/store/mockstore/unistore/rpc.go @@ -66,6 +66,11 @@ func (c *RPCClient) SendRequest(ctx context.Context, addr string, req *tikvrpc.R failpoint.Return(tikvrpc.GenRegionErrorResp(req, &errorpb.Error{ServerIsBusy: &errorpb.ServerIsBusy{}})) } }) + failpoint.Inject("epochNotMatch", func(val failpoint.Value) { + if val.(bool) { + failpoint.Return(tikvrpc.GenRegionErrorResp(req, &errorpb.Error{EpochNotMatch: &errorpb.EpochNotMatch{}})) + } + }) failpoint.Inject("unistoreRPCClientSendHook", func(val failpoint.Value) { if val.(bool) && UnistoreRPCClientSendHook != nil { From 310b493247ef44d841c9c308ea793e107ddb30a6 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 19 Oct 2023 19:41:53 +0800 Subject: [PATCH 3/6] refine Signed-off-by: crazycs520 --- store/helper/helper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/store/helper/helper.go b/store/helper/helper.go index eafd458097bd7..f731b9d4151af 100644 --- a/store/helper/helper.go +++ b/store/helper/helper.go @@ -96,9 +96,9 @@ func NewHelper(store Storage) *Helper { // GetMvccByEncodedKey get the MVCC value by the specific encoded key. func (h *Helper) GetMvccByEncodedKey(encodedKey kv.Key) (*kvrpcpb.MvccGetByKeyResponse, error) { - bo := tikv.NewBackofferWithVars(context.Background(), 500, nil) + bo := tikv.NewBackofferWithVars(context.Background(), 5000, nil) for { - keyLocation, err := h.RegionCache.LocateKey(tikv.NewBackofferWithVars(context.Background(), 500, nil), encodedKey) + keyLocation, err := h.RegionCache.LocateKey(bo, encodedKey) if err != nil { return nil, derr.ToTiDBErr(err) } From 9845018a998bc1d9fc94fa947284e36ae6825a97 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 20 Oct 2023 11:02:05 +0800 Subject: [PATCH 4/6] refine test Signed-off-by: crazycs520 --- executor/executor_failpoint_test.go | 11 +++++++++-- store/helper/helper.go | 14 ++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/executor/executor_failpoint_test.go b/executor/executor_failpoint_test.go index 7c5e93c2b896c..bc5ee84a000cc 100644 --- a/executor/executor_failpoint_test.go +++ b/executor/executor_failpoint_test.go @@ -635,13 +635,20 @@ func TestGetMvccByEncodedKeyRegionError(t *testing.T) { schemaVersion := tk.Session().GetDomainInfoSchema().SchemaMetaVersion() key := m.EncodeSchemaDiffKey(schemaVersion) + resp, err := h.GetMvccByEncodedKey(key) + require.NoError(t, err) + require.NotNil(t, resp.Info) + require.Equal(t, 1, len(resp.Info.Writes)) + require.Less(t, uint64(0), resp.Info.Writes[0].CommitTs) + commitTs := resp.Info.Writes[0].CommitTs + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/store/mockstore/unistore/epochNotMatch", "1*return(true)")) defer func() { require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/store/mockstore/unistore/epochNotMatch")) }() - resp, err := h.GetMvccByEncodedKey(key) + resp, err = h.GetMvccByEncodedKey(key) require.NoError(t, err) require.NotNil(t, resp.Info) require.Equal(t, 1, len(resp.Info.Writes)) - require.Less(t, uint64(0), resp.Info.Writes[0].CommitTs) + require.Equal(t, commitTs, resp.Info.Writes[0].CommitTs) } diff --git a/store/helper/helper.go b/store/helper/helper.go index f731b9d4151af..28276d050953b 100644 --- a/store/helper/helper.go +++ b/store/helper/helper.go @@ -97,13 +97,13 @@ func NewHelper(store Storage) *Helper { // GetMvccByEncodedKey get the MVCC value by the specific encoded key. func (h *Helper) GetMvccByEncodedKey(encodedKey kv.Key) (*kvrpcpb.MvccGetByKeyResponse, error) { bo := tikv.NewBackofferWithVars(context.Background(), 5000, nil) + tikvReq := tikvrpc.NewRequest(tikvrpc.CmdMvccGetByKey, &kvrpcpb.MvccGetByKeyRequest{Key: encodedKey}) for { keyLocation, err := h.RegionCache.LocateKey(bo, encodedKey) if err != nil { return nil, derr.ToTiDBErr(err) } - tikvReq := tikvrpc.NewRequest(tikvrpc.CmdMvccGetByKey, &kvrpcpb.MvccGetByKeyRequest{Key: encodedKey}) kvResp, err := h.Store.SendReq(bo, tikvReq, keyLocation.Region, time.Minute) if err != nil { logutil.BgLogger().Info("get MVCC by encoded key failed", @@ -124,7 +124,17 @@ func (h *Helper) GetMvccByEncodedKey(encodedKey kv.Key) (*kvrpcpb.MvccGetByKeyRe } continue } - return kvResp.Resp.(*kvrpcpb.MvccGetByKeyResponse), nil + mvccResp := kvResp.Resp.(*kvrpcpb.MvccGetByKeyResponse) + if errMsg := mvccResp.GetError(); errMsg != "" { + logutil.BgLogger().Info("get MVCC by encoded key failed", + zap.Stringer("encodeKey", encodedKey), + zap.Reflect("region", keyLocation.Region), + zap.Stringer("keyLocation", keyLocation), + zap.Reflect("kvResp", kvResp), + zap.String("error", errMsg)) + return nil, errors.New(errMsg) + } + return mvccResp, nil } } From aa282addfa6d027a4ee4b042c76f64b72903a2df Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 20 Oct 2023 11:05:20 +0800 Subject: [PATCH 5/6] update client-go Signed-off-by: crazycs520 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index b9597691ce85e..691ccbdcbe55c 100644 --- a/go.mod +++ b/go.mod @@ -90,7 +90,7 @@ require ( github.com/stretchr/testify v1.8.0 github.com/tdakkota/asciicheck v0.1.1 github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2 - github.com/tikv/client-go/v2 v2.0.4-0.20231012015822-35e4902b28af + github.com/tikv/client-go/v2 v2.0.4-0.20231020030327-4ecf7c282e37 github.com/tikv/pd/client v0.0.0-20221031025758-80f0d8ca4d07 github.com/timakin/bodyclose v0.0.0-20210704033933-f49887972144 github.com/twmb/murmur3 v1.1.3 diff --git a/go.sum b/go.sum index 2e8c3112b9ff5..0ceba09720bdd 100644 --- a/go.sum +++ b/go.sum @@ -935,8 +935,8 @@ github.com/tenntenn/text/transform v0.0.0-20200319021203-7eef512accb3 h1:f+jULpR github.com/tenntenn/text/transform v0.0.0-20200319021203-7eef512accb3/go.mod h1:ON8b8w4BN/kE1EOhwT0o+d62W65a6aPw1nouo9LMgyY= github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2 h1:mbAskLJ0oJfDRtkanvQPiooDH8HvJ2FBh+iKT/OmiQQ= github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2/go.mod h1:2PfKggNGDuadAa0LElHrByyrz4JPZ9fFx6Gs7nx7ZZU= -github.com/tikv/client-go/v2 v2.0.4-0.20231012015822-35e4902b28af h1:LnjpGVXmlHM/isD1m34+gYn0CTynN+BiSt3DkjjtWwo= -github.com/tikv/client-go/v2 v2.0.4-0.20231012015822-35e4902b28af/go.mod h1:mmVCLP2OqWvQJPOIevQPZvGphzh/oq9vv8J5LDfpadQ= +github.com/tikv/client-go/v2 v2.0.4-0.20231020030327-4ecf7c282e37 h1:x4x8yO7tHtxOepMGpPnnLMVw2geDsrYbbbB7k+0zusY= +github.com/tikv/client-go/v2 v2.0.4-0.20231020030327-4ecf7c282e37/go.mod h1:mmVCLP2OqWvQJPOIevQPZvGphzh/oq9vv8J5LDfpadQ= github.com/tikv/pd/client v0.0.0-20221031025758-80f0d8ca4d07 h1:ckPpxKcl75mO2N6a4cJXiZH43hvcHPpqc9dh1TmH1nc= github.com/tikv/pd/client v0.0.0-20221031025758-80f0d8ca4d07/go.mod h1:CipBxPfxPUME+BImx9MUYXCnAVLS3VJUr3mnSJwh40A= github.com/timakin/bodyclose v0.0.0-20210704033933-f49887972144 h1:kl4KhGNsJIbDHS9/4U9yQo1UcPQM0kOMJHn29EoH/Ro= From 671e74eaca0b23abdcde4c346e8dafba94e31def Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 20 Oct 2023 11:23:03 +0800 Subject: [PATCH 6/6] make bazel_prepare Signed-off-by: crazycs520 --- DEPS.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DEPS.bzl b/DEPS.bzl index 467a1b2018ea0..7bd8749dc6d7f 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -3703,8 +3703,8 @@ def go_deps(): name = "com_github_tikv_client_go_v2", build_file_proto_mode = "disable_global", importpath = "github.com/tikv/client-go/v2", - sum = "h1:LnjpGVXmlHM/isD1m34+gYn0CTynN+BiSt3DkjjtWwo=", - version = "v2.0.4-0.20231012015822-35e4902b28af", + sum = "h1:x4x8yO7tHtxOepMGpPnnLMVw2geDsrYbbbB7k+0zusY=", + version = "v2.0.4-0.20231020030327-4ecf7c282e37", ) go_repository( name = "com_github_tikv_pd_client",