Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

session: fix data race in the LazyTxn.LockKeys #40350

Merged
merged 22 commits into from
Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions DEPS.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3527,16 +3527,16 @@ def go_deps():
name = "com_github_tiancaiamao_gp",
build_file_proto_mode = "disable",
importpath = "github.com/tiancaiamao/gp",
sum = "h1:4RNtqw1/tW67qP9fFgfQpTVd7DrfkaAWu4vsC18QmBo=",
version = "v0.0.0-20221221095600-1a473d1f9b4b",
sum = "h1:J/YdBZ46WKpXsxsW93SG+q0F8KI+yFrcIDT4c/RNoc4=",
version = "v0.0.0-20221230034425-4025bc8a4d4a",
)

go_repository(
name = "com_github_tikv_client_go_v2",
build_file_proto_mode = "disable_global",
importpath = "github.com/tikv/client-go/v2",
sum = "h1:m6glgBGCIds9QURbk8Mn+8mjLKDcv6nWrNwYh92fydQ=",
version = "v2.0.4-0.20221226080148-018c59dbd837",
sum = "h1:cPtMXTExqjzk8L40qhrgB/mXiBXKP5LRU0vwjtI2Xxo=",
version = "v2.0.4",
)
go_repository(
name = "com_github_tikv_pd_client",
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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.20221226080148-018c59dbd837
github.com/tikv/client-go/v2 v2.0.4
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
Expand Down Expand Up @@ -221,7 +221,7 @@ require (
github.com/shurcooL/vfsgen v0.0.0-20181202132449-6a9ea43bcacd // indirect
github.com/sirupsen/logrus v1.9.0 // indirect
github.com/spaolacci/murmur3 v1.1.0 // indirect
github.com/tiancaiamao/gp v0.0.0-20221221095600-1a473d1f9b4b // indirect
github.com/tiancaiamao/gp v0.0.0-20221230034425-4025bc8a4d4a // indirect
github.com/tklauser/go-sysconf v0.3.10 // indirect
github.com/tklauser/numcpus v0.4.0 // indirect
github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802 // indirect
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -933,10 +933,10 @@ 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/tiancaiamao/gp v0.0.0-20221221095600-1a473d1f9b4b h1:4RNtqw1/tW67qP9fFgfQpTVd7DrfkaAWu4vsC18QmBo=
github.com/tiancaiamao/gp v0.0.0-20221221095600-1a473d1f9b4b/go.mod h1:h4xBhSNtOeEosLJ4P7JyKXX7Cabg7AVkWCK5gV2vOrM=
github.com/tikv/client-go/v2 v2.0.4-0.20221226080148-018c59dbd837 h1:m6glgBGCIds9QURbk8Mn+8mjLKDcv6nWrNwYh92fydQ=
github.com/tikv/client-go/v2 v2.0.4-0.20221226080148-018c59dbd837/go.mod h1:ptS8K+VBrEH2gIS3JxaiFSSLfDDyuS2xcdLozOtBWBw=
github.com/tiancaiamao/gp v0.0.0-20221230034425-4025bc8a4d4a h1:J/YdBZ46WKpXsxsW93SG+q0F8KI+yFrcIDT4c/RNoc4=
github.com/tiancaiamao/gp v0.0.0-20221230034425-4025bc8a4d4a/go.mod h1:h4xBhSNtOeEosLJ4P7JyKXX7Cabg7AVkWCK5gV2vOrM=
github.com/tikv/client-go/v2 v2.0.4 h1:cPtMXTExqjzk8L40qhrgB/mXiBXKP5LRU0vwjtI2Xxo=
github.com/tikv/client-go/v2 v2.0.4/go.mod h1:v52O5zDtv2BBus4lm5yrSQhxGW4Z4RaXWfg0U1Kuyqo=
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=
Expand Down
7 changes: 7 additions & 0 deletions kv/interface_mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ func (t *mockTxn) LockKeys(_ context.Context, _ *LockCtx, _ ...Key) error {
return nil
}

func (t *mockTxn) LockKeysFunc(_ context.Context, _ *LockCtx, fn func(), _ ...Key) error {
if fn != nil {
fn()
}
return nil
}

func (t *mockTxn) SetOption(opt int, val interface{}) {
t.opts[opt] = val
}
Expand Down
5 changes: 5 additions & 0 deletions kv/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,12 @@ type Transaction interface {
String() string
// LockKeys tries to lock the entries with the keys in KV store.
// Will block until all keys are locked successfully or an error occurs.
// fn is called before LockKeys unlocks the keys.
LockKeys(ctx context.Context, lockCtx *LockCtx, keys ...Key) error
// LockKeysFunc tries to lock the entries with the keys in KV store.
// Will block until all keys are locked successfully or an error occurs.
// fn is called before LockKeys unlocks the keys.
LockKeysFunc(ctx context.Context, lockCtx *LockCtx, fn func(), keys ...Key) error
// SetOption sets an option with a value, when val is nil, uses the default
// value of this option.
SetOption(opt int, val interface{})
Expand Down
29 changes: 18 additions & 11 deletions session/txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"fmt"
"runtime/trace"
"strings"
"sync"
"sync/atomic"
"time"

Expand All @@ -36,6 +35,7 @@ import (
"github.com/pingcap/tidb/tablecodec"
"github.com/pingcap/tidb/util/logutil"
"github.com/pingcap/tidb/util/sli"
"github.com/pingcap/tidb/util/syncutil"
"github.com/pingcap/tipb/go-binlog"
"github.com/tikv/client-go/v2/oracle"
"github.com/tikv/client-go/v2/tikv"
Expand Down Expand Up @@ -64,7 +64,7 @@ type LazyTxn struct {
// The data in this session would be query by other sessions, so Mutex is necessary.
// Since read is rare, the reader can copy-on-read to get a data snapshot.
mu struct {
sync.RWMutex
syncutil.RWMutex
txninfo.TxnInfo
}

Expand Down Expand Up @@ -428,6 +428,11 @@ func (txn *LazyTxn) RollbackMemDBToCheckpoint(savepoint *tikv.MemDBCheckpoint) {

// LockKeys Wrap the inner transaction's `LockKeys` to record the status
func (txn *LazyTxn) LockKeys(ctx context.Context, lockCtx *kv.LockCtx, keys ...kv.Key) error {
return txn.LockKeysFunc(ctx, lockCtx, nil, keys...)
}

// LockKeysFunc Wrap the inner transaction's `LockKeys` to record the status
func (txn *LazyTxn) LockKeysFunc(ctx context.Context, lockCtx *kv.LockCtx, fn func(), keys ...kv.Key) error {
failpoint.Inject("beforeLockKeys", func() {})
t := time.Now()

Expand All @@ -438,15 +443,17 @@ func (txn *LazyTxn) LockKeys(ctx context.Context, lockCtx *kv.LockCtx, keys ...k
txn.mu.TxnInfo.BlockStartTime.Valid = true
txn.mu.TxnInfo.BlockStartTime.Time = t
txn.mu.Unlock()

err := txn.Transaction.LockKeys(ctx, lockCtx, keys...)

txn.mu.Lock()
defer txn.mu.Unlock()
txn.updateState(originState)
txn.mu.TxnInfo.BlockStartTime.Valid = false
txn.mu.TxnInfo.EntriesCount = uint64(txn.Transaction.Len())
return err
lockFunc := func() {
if fn != nil {
fn()
}
txn.mu.Lock()
defer txn.mu.Unlock()
txn.updateState(originState)
txn.mu.TxnInfo.BlockStartTime.Valid = false
txn.mu.TxnInfo.EntriesCount = uint64(txn.Transaction.Len())
}
return txn.Transaction.LockKeysFunc(ctx, lockCtx, lockFunc, keys...)
}

func (txn *LazyTxn) reset() {
Expand Down
1 change: 1 addition & 0 deletions sessionctx/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ go_test(
],
embed = [":sessionctx"],
flaky = True,
race = "on",
deps = [
"//testkit/testsetup",
"@com_github_stretchr_testify//require",
Expand Down
2 changes: 2 additions & 0 deletions sessiontxn/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ go_test(
"txn_rc_tso_optimize_test.go",
],
flaky = True,
race = "on",
shard_count = 2,
deps = [
":sessiontxn",
"//domain",
Expand Down
6 changes: 6 additions & 0 deletions store/driver/txn/txn_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ func (txn *tikvTxn) LockKeys(ctx context.Context, lockCtx *kv.LockCtx, keysInput
return txn.extractKeyErr(err)
}

func (txn *tikvTxn) LockKeysFunc(ctx context.Context, lockCtx *kv.LockCtx, fn func(), keysInput ...kv.Key) error {
keys := toTiKVKeys(keysInput)
err := txn.KVTxn.LockKeysFunc(ctx, lockCtx, fn, keys...)
return txn.extractKeyErr(err)
}

func (txn *tikvTxn) Commit(ctx context.Context) error {
err := txn.KVTxn.Commit(ctx)
return txn.extractKeyErr(err)
Expand Down