Skip to content

Commit

Permalink
fix the incorrect untouch used in optimistic transactions and update …
Browse files Browse the repository at this point in the history
…client go
  • Loading branch information
cfzjywxk committed Dec 21, 2021
1 parent fe1aaf2 commit e5125eb
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 10 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ require (
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.7.0
github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2
github.com/tikv/client-go/v2 v2.0.0-rc.0.20211218050306-6165dbaa95d0
github.com/tikv/client-go/v2 v2.0.0-rc.0.20211221041211-e9de5625c45c
github.com/tikv/pd v1.1.0-beta.0.20211118054146-02848d2660ee
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 @@ -712,8 +712,8 @@ github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2/go.mod h1:2PfK
github.com/tidwall/gjson v1.3.5/go.mod h1:P256ACg0Mn+j1RXIDXoss50DeIABTYK1PULOJHhxOls=
github.com/tidwall/match v1.0.1/go.mod h1:LujAq0jyVjBy028G1WhWfIzbpQfMO8bBZ6Tyb0+pL9E=
github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
github.com/tikv/client-go/v2 v2.0.0-rc.0.20211218050306-6165dbaa95d0 h1:38Jst/O36MKXAt7aD1Ipnx4nKwclG66ifkcmi4f0NZ4=
github.com/tikv/client-go/v2 v2.0.0-rc.0.20211218050306-6165dbaa95d0/go.mod h1:wRuh+W35daKTiYBld0oBlT6PSkzEVr+pB/vChzJZk+8=
github.com/tikv/client-go/v2 v2.0.0-rc.0.20211221041211-e9de5625c45c h1:1P6iN1csRSZNHXuaylArmG3/bA5MpYVzc9ZkdHK/L2Y=
github.com/tikv/client-go/v2 v2.0.0-rc.0.20211221041211-e9de5625c45c/go.mod h1:wRuh+W35daKTiYBld0oBlT6PSkzEVr+pB/vChzJZk+8=
github.com/tikv/pd v1.1.0-beta.0.20211029083450-e65f0c55b6ae/go.mod h1:varH0IE0jJ9E9WN2Ei/N6pajMlPkcXdDEf7f5mmsUVQ=
github.com/tikv/pd v1.1.0-beta.0.20211118054146-02848d2660ee h1:rAAdvQ8Hh36syHr92g0VmZEpkH+40RGQBpFL2121xMs=
github.com/tikv/pd v1.1.0-beta.0.20211118054146-02848d2660ee/go.mod h1:lRbwxBAhnTQR5vqbTzeI/Bj62bD2OvYYuFezo2vrmeI=
Expand Down
8 changes: 5 additions & 3 deletions session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"github.com/pingcap/tidb/parser/model"
"github.com/pingcap/tidb/parser/mysql"
"github.com/pingcap/tidb/parser/terror"
"github.com/pingcap/tidb/store/driver/txn"
"github.com/pingcap/tidb/table/tables"
"github.com/pingcap/tidb/table/temptable"
"github.com/pingcap/tidb/util/topsql"
Expand Down Expand Up @@ -753,14 +754,15 @@ func (s *session) commitTxnWithTemporaryData(ctx context.Context, txn kv.Transac

type temporaryTableKVFilter map[int64]tableutil.TempTable

func (m temporaryTableKVFilter) IsUnnecessaryKeyValue(key, value []byte, flags tikvstore.KeyFlags) bool {
func (m temporaryTableKVFilter) IsUnnecessaryKeyValue(key, value []byte, flags tikvstore.KeyFlags) (bool, error) {
tid := tablecodec.DecodeTableID(key)
if _, ok := m[tid]; ok {
return true
return true, nil
}

// This is the default filter for all tables.
return tablecodec.IsUntouchedIndexKValue(key, value)
defaultFilter := txn.TiDBKVFilter{}
return defaultFilter.IsUnnecessaryKeyValue(key, value, flags)
}

// errIsNoisy is used to filter DUPLCATE KEY errors.
Expand Down
15 changes: 13 additions & 2 deletions store/driver/txn/txn_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ import (
derr "github.com/pingcap/tidb/store/driver/error"
"github.com/pingcap/tidb/store/driver/options"
"github.com/pingcap/tidb/tablecodec"
"github.com/pingcap/tidb/util/logutil"
tikverr "github.com/tikv/client-go/v2/error"
tikvstore "github.com/tikv/client-go/v2/kv"
"github.com/tikv/client-go/v2/tikv"
"github.com/tikv/client-go/v2/tikvrpc"
"github.com/tikv/client-go/v2/tikvrpc/interceptor"
"github.com/tikv/client-go/v2/txnkv/txnsnapshot"
"go.uber.org/zap"
)

type tikvTxn struct {
Expand Down Expand Up @@ -292,6 +294,15 @@ func (txn *tikvTxn) extractKeyExistsErr(key kv.Key) error {
type TiDBKVFilter struct{}

// IsUnnecessaryKeyValue defines which kinds of KV pairs from TiDB needn't be committed.
func (f TiDBKVFilter) IsUnnecessaryKeyValue(key, value []byte, flags tikvstore.KeyFlags) bool {
return tablecodec.IsUntouchedIndexKValue(key, value)
func (f TiDBKVFilter) IsUnnecessaryKeyValue(key, value []byte, flags tikvstore.KeyFlags) (bool, error) {
isUntouchedValue := tablecodec.IsUntouchedIndexKValue(key, value)
if isUntouchedValue && flags.HasPresumeKeyNotExists() {
logutil.BgLogger().Error("unexpected path the untouched key value with PresumeKeyNotExists flag",
zap.Stringer("key", kv.Key(key)), zap.Stringer("value", kv.Key(value)),
zap.Uint16("flags", uint16(flags)), zap.Stack("stack"))
return false, errors.Errorf(
"unexpected path the untouched key=%s value=%s contains PresumeKeyNotExists flag keyFlags=%v",
kv.Key(key).String(), kv.Key(value).String(), flags)
}
return isUntouchedValue, nil
}
18 changes: 16 additions & 2 deletions table/tables/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,22 @@ func (c *index) Create(sctx sessionctx.Context, txn kv.Transaction, indexedValue
// should not overwrite the key with un-commit flag.
// So if the key exists, just do nothing and return.
v, err := txn.GetMemBuffer().Get(ctx, key)
if err == nil && len(v) != 0 {
return nil, nil
if err == nil {
if len(v) != 0 {
return nil, nil
}
// The key is marked as deleted in the memory buffer, as the existence check is done lazily
// for optimistic transactions by default. The "untouched" key could still exist in the store,
// it's needed to commit this key to do the existence check so unset the untouched flag.
if !txn.IsPessimistic() {
keyFlags, err := txn.GetMemBuffer().GetFlags(key)
if err != nil {
return nil, err
}
if keyFlags.HasPresumeKeyNotExists() {
opt.Untouched = false
}
}
}
}

Expand Down
26 changes: 26 additions & 0 deletions table/tables/tables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,3 +736,29 @@ func TestViewColumns(t *testing.T) {
"Warning|1356|View 'test.va' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them"))
}
}

func TestConstraintCheckForOptimisticUntouched(t *testing.T) {
t.Parallel()
store, clean := testkit.CreateMockStore(t)
defer clean()

tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("drop table if exists test_optimistic_untouched_flag;")
tk.MustExec(`create table test_optimistic_untouched_flag(c0 int, c1 varchar(20), c2 varchar(20), unique key uk(c0));`)
tk.MustExec(`insert into test_optimistic_untouched_flag(c0, c1, c2) values (1, null, 'green');`)

// Insert a row with duplicated entry on the unique key, the commit should fail.
tk.MustExec("begin optimistic;")
tk.MustExec(`insert into test_optimistic_untouched_flag(c0, c1, c2) values (1, 'red', 'white');`)
tk.MustExec(`delete from test_optimistic_untouched_flag where c1 is null;`)
tk.MustExec("update test_optimistic_untouched_flag set c2 = 'green' where c2 between 'purple' and 'white';")
err := tk.ExecToErr("commit")
require.Error(t, err)

tk.MustExec("begin optimistic;")
tk.MustExec(`insert into test_optimistic_untouched_flag(c0, c1, c2) values (1, 'red', 'white');`)
tk.MustExec("update test_optimistic_untouched_flag set c2 = 'green' where c2 between 'purple' and 'white';")
err = tk.ExecToErr("commit")
require.Error(t, err)
}

0 comments on commit e5125eb

Please sign in to comment.