From 43de9deef13ce3dfb42829f5d40d7bc599d352d2 Mon Sep 17 00:00:00 2001 From: crazycs Date: Mon, 28 Oct 2024 20:57:30 +0800 Subject: [PATCH 1/3] This is an automated cherry-pick of #56847 Signed-off-by: ti-chi-bot --- executor/executor_txn_test.go | 44 +++++++++++++++++++++++ executor/unstabletest/BUILD.bazel | 11 ++++++ sessiontxn/isolation/readcommitted.go | 5 +++ sessiontxn/isolation/repeatable_read.go | 5 +++ store/mockstore/unistore/tikv/BUILD.bazel | 1 + store/mockstore/unistore/tikv/server.go | 13 +++++++ 6 files changed, 79 insertions(+) diff --git a/executor/executor_txn_test.go b/executor/executor_txn_test.go index d2bd8f862830e..27764ca69b851 100644 --- a/executor/executor_txn_test.go +++ b/executor/executor_txn_test.go @@ -22,10 +22,17 @@ import ( "testing" "time" +<<<<<<< HEAD:executor/executor_txn_test.go "github.com/pingcap/tidb/executor" "github.com/pingcap/tidb/sessionctx/binloginfo" "github.com/pingcap/tidb/testkit" "github.com/pingcap/tipb/go-binlog" +======= + "github.com/pingcap/failpoint" + "github.com/pingcap/tidb/pkg/errno" + "github.com/pingcap/tidb/pkg/testkit" + "github.com/pingcap/tidb/pkg/testkit/testfailpoint" +>>>>>>> 38187e5b1d6 (txn: fix issue innodb_lock_wait_timeout doesn't work in some case (#56847)):pkg/executor/test/txn/txn_test.go "github.com/stretchr/testify/require" "google.golang.org/grpc" ) @@ -798,3 +805,40 @@ func (m mockPumpClient) WriteBinlog(ctx context.Context, in *binlog.WriteBinlogR func (m mockPumpClient) PullBinlogs(ctx context.Context, in *binlog.PullBinlogReq, opts ...grpc.CallOption) (binlog.Pump_PullBinlogsClient, error) { return nil, nil } + +func TestInnodbLockWaitTimeout(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t (id int auto_increment, k int,c varchar(255), unique index idx(id))") + tk.MustExec("insert into t (k,c) values (1,'abcdefg');") + for i := 0; i < 8; i++ { + tk.MustExec("insert into t (k,c) select k,c from t;") + } + tk.MustExec("update t set k= id, c = id") + tk.MustExec("split table t by (0), (50), (100);") + tk.MustExec("split table t index idx by (0), (50), (100);") + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/store/mockstore/unistore/tikv/pessimisticLockReturnWriteConflict", `return(true)`)) + defer func() { + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/store/mockstore/unistore/tikv/pessimisticLockReturnWriteConflict")) + }() + tk.MustExec("set @@innodb_lock_wait_timeout=1") + isolations := []string{"REPEATABLE READ", "READ COMMITTED"} + for _, isolation := range isolations { + tk.MustExec("SET SESSION TRANSACTION ISOLATION LEVEL " + isolation) + tk.MustExec("begin") + start := time.Now() + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + res, err := tk.ExecWithContext(ctx, "update t use index (idx) set k=k+1 where id >0;") + cancel() + if res != nil { + require.NoError(t, res.Close()) + } + require.Error(t, err) + msg := fmt.Sprintf("cost: %v", time.Since(start)) + require.Equal(t, "lock wait timeout", err.Error(), msg) + require.Less(t, time.Since(start), time.Second*2) + tk.MustExec("commit") + } +} diff --git a/executor/unstabletest/BUILD.bazel b/executor/unstabletest/BUILD.bazel index 4f91dc7fbb400..5494b8ee4d14c 100644 --- a/executor/unstabletest/BUILD.bazel +++ b/executor/unstabletest/BUILD.bazel @@ -9,6 +9,7 @@ go_test( "unstable_test.go", ], flaky = True, +<<<<<<< HEAD:executor/unstabletest/BUILD.bazel shard_count = 4, deps = [ "//config", @@ -16,6 +17,16 @@ go_test( "//testkit", "//util", "//util/memory", +======= + shard_count = 11, + deps = [ + "//pkg/config", + "//pkg/errno", + "//pkg/meta/autoid", + "//pkg/testkit", + "//pkg/testkit/testfailpoint", + "@com_github_pingcap_failpoint//:failpoint", +>>>>>>> 38187e5b1d6 (txn: fix issue innodb_lock_wait_timeout doesn't work in some case (#56847)):pkg/executor/test/txn/BUILD.bazel "@com_github_stretchr_testify//require", "@com_github_tikv_client_go_v2//tikv", "@io_opencensus_go//stats/view", diff --git a/sessiontxn/isolation/readcommitted.go b/sessiontxn/isolation/readcommitted.go index 510f6b407c9a7..ee8e89df798f2 100644 --- a/sessiontxn/isolation/readcommitted.go +++ b/sessiontxn/isolation/readcommitted.go @@ -230,6 +230,11 @@ func (p *PessimisticRCTxnContextProvider) handleAfterPessimisticLockError(ctx co return sessiontxn.ErrorAction(err) } } else if terror.ErrorEqual(kv.ErrWriteConflict, lockErr) { + sessVars := p.sctx.GetSessionVars() + waitTime := time.Since(sessVars.StmtCtx.GetLockWaitStartTime()) + if waitTime.Milliseconds() >= sessVars.LockWaitTimeout { + return sessiontxn.ErrorAction(tikverr.ErrLockWaitTimeout) + } logutil.Logger(p.ctx).Debug("pessimistic write conflict, retry statement", zap.Uint64("txn", txnCtx.StartTS), zap.Uint64("forUpdateTS", txnCtx.GetForUpdateTS()), diff --git a/sessiontxn/isolation/repeatable_read.go b/sessiontxn/isolation/repeatable_read.go index e64a066d47d89..33003e198e773 100644 --- a/sessiontxn/isolation/repeatable_read.go +++ b/sessiontxn/isolation/repeatable_read.go @@ -257,6 +257,11 @@ func (p *PessimisticRRTxnContextProvider) handleAfterPessimisticLockError(ctx co return sessiontxn.ErrorAction(err) } } else if terror.ErrorEqual(kv.ErrWriteConflict, lockErr) { + waitTime := time.Since(sessVars.StmtCtx.GetLockWaitStartTime()) + if waitTime.Milliseconds() >= sessVars.LockWaitTimeout { + return sessiontxn.ErrorAction(tikverr.ErrLockWaitTimeout) + } + // Always update forUpdateTS by getting a new timestamp from PD. // If we use the conflict commitTS as the new forUpdateTS and async commit // is used, the commitTS of this transaction may exceed the max timestamp diff --git a/store/mockstore/unistore/tikv/BUILD.bazel b/store/mockstore/unistore/tikv/BUILD.bazel index 52578cacbc78a..9ccbe11969142 100644 --- a/store/mockstore/unistore/tikv/BUILD.bazel +++ b/store/mockstore/unistore/tikv/BUILD.bazel @@ -41,6 +41,7 @@ go_library( "@com_github_pingcap_badger//:badger", "@com_github_pingcap_badger//y", "@com_github_pingcap_errors//:errors", + "@com_github_pingcap_failpoint//:failpoint", "@com_github_pingcap_kvproto//pkg/coprocessor", "@com_github_pingcap_kvproto//pkg/deadlock", "@com_github_pingcap_kvproto//pkg/errorpb", diff --git a/store/mockstore/unistore/tikv/server.go b/store/mockstore/unistore/tikv/server.go index bf8eb592dbae8..afe7647784348 100644 --- a/store/mockstore/unistore/tikv/server.go +++ b/store/mockstore/unistore/tikv/server.go @@ -21,6 +21,7 @@ import ( "time" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/coprocessor" deadlockPb "github.com/pingcap/kvproto/pkg/deadlock" "github.com/pingcap/kvproto/pkg/errorpb" @@ -193,6 +194,18 @@ func (svr *Server) KvScan(ctx context.Context, req *kvrpcpb.ScanRequest) (*kvrpc // KvPessimisticLock implements the tikvpb.TikvServer interface. func (svr *Server) KvPessimisticLock(ctx context.Context, req *kvrpcpb.PessimisticLockRequest) (*kvrpcpb.PessimisticLockResponse, error) { + failpoint.Inject("pessimisticLockReturnWriteConflict", func(val failpoint.Value) { + if val.(bool) { + time.Sleep(time.Millisecond * 100) + err := &kverrors.ErrConflict{ + StartTS: req.GetForUpdateTs(), + ConflictTS: req.GetForUpdateTs() + 1, + ConflictCommitTS: req.GetForUpdateTs() + 2, + } + failpoint.Return(&kvrpcpb.PessimisticLockResponse{Errors: []*kvrpcpb.KeyError{convertToKeyError(err)}}, nil) + } + }) + reqCtx, err := newRequestCtx(svr, req.Context, "PessimisticLock") if err != nil { return &kvrpcpb.PessimisticLockResponse{Errors: []*kvrpcpb.KeyError{convertToKeyError(err)}}, nil From f312ccba483b24751a6c1d3761bda9fa5e5ed17e Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 29 Oct 2024 11:09:50 +0800 Subject: [PATCH 2/3] fix conflict Signed-off-by: crazycs520 --- executor/executor_txn_test.go | 8 +------- executor/unstabletest/BUILD.bazel | 11 ----------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/executor/executor_txn_test.go b/executor/executor_txn_test.go index 27764ca69b851..a262dc3f30c7d 100644 --- a/executor/executor_txn_test.go +++ b/executor/executor_txn_test.go @@ -22,17 +22,11 @@ import ( "testing" "time" -<<<<<<< HEAD:executor/executor_txn_test.go + "github.com/pingcap/failpoint" "github.com/pingcap/tidb/executor" "github.com/pingcap/tidb/sessionctx/binloginfo" "github.com/pingcap/tidb/testkit" "github.com/pingcap/tipb/go-binlog" -======= - "github.com/pingcap/failpoint" - "github.com/pingcap/tidb/pkg/errno" - "github.com/pingcap/tidb/pkg/testkit" - "github.com/pingcap/tidb/pkg/testkit/testfailpoint" ->>>>>>> 38187e5b1d6 (txn: fix issue innodb_lock_wait_timeout doesn't work in some case (#56847)):pkg/executor/test/txn/txn_test.go "github.com/stretchr/testify/require" "google.golang.org/grpc" ) diff --git a/executor/unstabletest/BUILD.bazel b/executor/unstabletest/BUILD.bazel index 5494b8ee4d14c..4f91dc7fbb400 100644 --- a/executor/unstabletest/BUILD.bazel +++ b/executor/unstabletest/BUILD.bazel @@ -9,7 +9,6 @@ go_test( "unstable_test.go", ], flaky = True, -<<<<<<< HEAD:executor/unstabletest/BUILD.bazel shard_count = 4, deps = [ "//config", @@ -17,16 +16,6 @@ go_test( "//testkit", "//util", "//util/memory", -======= - shard_count = 11, - deps = [ - "//pkg/config", - "//pkg/errno", - "//pkg/meta/autoid", - "//pkg/testkit", - "//pkg/testkit/testfailpoint", - "@com_github_pingcap_failpoint//:failpoint", ->>>>>>> 38187e5b1d6 (txn: fix issue innodb_lock_wait_timeout doesn't work in some case (#56847)):pkg/executor/test/txn/BUILD.bazel "@com_github_stretchr_testify//require", "@com_github_tikv_client_go_v2//tikv", "@io_opencensus_go//stats/view", From b8aeee086b20355f1f76a9c9be5154e2fa5daf33 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 29 Oct 2024 11:48:33 +0800 Subject: [PATCH 3/3] fix test Signed-off-by: crazycs520 --- executor/executor_txn_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor/executor_txn_test.go b/executor/executor_txn_test.go index a262dc3f30c7d..32bd8d738167a 100644 --- a/executor/executor_txn_test.go +++ b/executor/executor_txn_test.go @@ -813,9 +813,9 @@ func TestInnodbLockWaitTimeout(t *testing.T) { tk.MustExec("update t set k= id, c = id") tk.MustExec("split table t by (0), (50), (100);") tk.MustExec("split table t index idx by (0), (50), (100);") - require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/store/mockstore/unistore/tikv/pessimisticLockReturnWriteConflict", `return(true)`)) + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/store/mockstore/unistore/tikv/pessimisticLockReturnWriteConflict", `return(true)`)) defer func() { - require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/store/mockstore/unistore/tikv/pessimisticLockReturnWriteConflict")) + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/store/mockstore/unistore/tikv/pessimisticLockReturnWriteConflict")) }() tk.MustExec("set @@innodb_lock_wait_timeout=1") isolations := []string{"REPEATABLE READ", "READ COMMITTED"}