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) #40389

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ require (
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.7.2-0.20220504104629-106ec21d14df
github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2
github.com/tikv/client-go/v2 v2.0.1-0.20221012074928-624e0ed3cc67
github.com/tikv/client-go/v2 v2.0.1-0.20230117081319-35a262e90d9b
github.com/tikv/pd/client v0.0.0-20220307081149-841fa61e9710
github.com/twmb/murmur3 v1.1.3
github.com/uber/jaeger-client-go v2.22.1+incompatible
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -755,8 +755,8 @@ github.com/stretchr/testify v1.7.2-0.20220504104629-106ec21d14df/go.mod h1:6Fq8o
github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
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.1-0.20221012074928-624e0ed3cc67 h1:ofj1E/98HrrEpXi+RmjBhnBY3+/9ObyorIkloGbvEa0=
github.com/tikv/client-go/v2 v2.0.1-0.20221012074928-624e0ed3cc67/go.mod h1:VTlli8fRRpcpISj9I2IqroQmcAFfaTyBquiRhofOcDs=
github.com/tikv/client-go/v2 v2.0.1-0.20230117081319-35a262e90d9b h1:IUH/4BrP9BJm7to+XUJslcwaZONuIEwwClBnlrO7zJM=
github.com/tikv/client-go/v2 v2.0.1-0.20230117081319-35a262e90d9b/go.mod h1:VTlli8fRRpcpISj9I2IqroQmcAFfaTyBquiRhofOcDs=
github.com/tikv/pd/client v0.0.0-20220307081149-841fa61e9710 h1:jxgmKOscXSjaFEKQGRyY5qOpK8hLqxs2irb/uDJMtwk=
github.com/tikv/pd/client v0.0.0-20220307081149-841fa61e9710/go.mod h1:AtvppPwkiyUgQlR1W9qSqfTB+OsOIu19jDCOxOsPkmU=
github.com/tklauser/go-sysconf v0.3.9 h1:JeUVdAOWhhxVcU6Eqr/ATFHgXk/mmiItdKeJPev3vTo=
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
4 changes: 4 additions & 0 deletions kv/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,10 @@ type Transaction interface {
// LockKeys tries to lock the entries with the keys in KV store.
// Will block until all keys are locked successfully or an error occurs.
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
26 changes: 17 additions & 9 deletions session/txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,11 @@ func (txn *LazyTxn) Rollback() error {

// 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 @@ -389,15 +394,18 @@ func (txn *LazyTxn) LockKeys(ctx context.Context, lockCtx *kv.LockCtx, keys ...k
txn.mu.TxnInfo.BlockStartTime.Time = t
txn.mu.Unlock()

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

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

func (txn *LazyTxn) reset() {
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 @@ -75,6 +75,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