From 35c28a8613a1fe57ffb95ea63ffedb0480c25b69 Mon Sep 17 00:00:00 2001 From: you06 Date: Tue, 12 Jul 2022 14:41:25 +0800 Subject: [PATCH 1/6] opt point-get by reducing read index Signed-off-by: you06 --- executor/point_get.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/executor/point_get.go b/executor/point_get.go index 04687403c0d4e..3d9ac50d9e6bd 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -237,20 +237,26 @@ func (e *PointGetExecutor) Next(ctx context.Context, req *chunk.Chunk) error { return err } - e.handleVal, err = e.get(ctx, e.idxKey) - if err != nil { - if !kv.ErrNotExist.Equal(err) { - return err - } - } + readFromCache := !e.ctx.GetSessionVars().IsPessimisticReadConsistency() || len(e.handleVal) > 0 - // try lock the index key if isolation level is not read consistency - // also lock key if read consistency read a value - if !e.ctx.GetSessionVars().IsPessimisticReadConsistency() || len(e.handleVal) > 0 { + if !readFromCache { + e.handleVal, err = e.get(ctx, e.idxKey) + if err != nil { + if !kv.ErrNotExist.Equal(err) { + return err + } + } + } else { + // try lock the index key if isolation level is not read consistency + // also lock key if read consistency read a value err = e.lockKeyIfNeeded(ctx, e.idxKey) if err != nil { return err } + e.handleVal, err = e.get(ctx, e.idxKey) + if err != nil { + return err + } // Change the unique index LOCK into PUT record. if e.lock && len(e.handleVal) > 0 { if !e.txn.Valid() { @@ -377,7 +383,7 @@ func (e *PointGetExecutor) lockKeyIfNeeded(ctx context.Context, key []byte) erro return err } lockCtx.IterateValuesNotLocked(func(k, v []byte) { - seVars.TxnCtx.SetPessimisticLockCache(kv.Key(k), v) + seVars.TxnCtx.SetPessimisticLockCache(k, v) }) if len(e.handleVal) > 0 { seVars.TxnCtx.SetPessimisticLockCache(e.idxKey, e.handleVal) From 133938008306c05352406d015a90d2b1a41334f0 Mon Sep 17 00:00:00 2001 From: you06 Date: Tue, 12 Jul 2022 17:12:57 +0800 Subject: [PATCH 2/6] fix bug Signed-off-by: you06 --- executor/point_get.go | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/executor/point_get.go b/executor/point_get.go index 3d9ac50d9e6bd..7360e55154224 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -237,26 +237,30 @@ func (e *PointGetExecutor) Next(ctx context.Context, req *chunk.Chunk) error { return err } - readFromCache := !e.ctx.GetSessionVars().IsPessimisticReadConsistency() || len(e.handleVal) > 0 - - if !readFromCache { - e.handleVal, err = e.get(ctx, e.idxKey) - if err != nil { - if !kv.ErrNotExist.Equal(err) { - return err - } - } - } else { - // try lock the index key if isolation level is not read consistency - // also lock key if read consistency read a value + readFromCache := !e.ctx.GetSessionVars().IsPessimisticReadConsistency() + if readFromCache{ err = e.lockKeyIfNeeded(ctx, e.idxKey) if err != nil { return err } - e.handleVal, err = e.get(ctx, e.idxKey) - if err != nil { + } + + e.handleVal, err = e.get(ctx, e.idxKey) + if err != nil { + if !kv.ErrNotExist.Equal(err) { return err } + } + + // try lock the index key if isolation level is not read consistency + // also lock key if read consistency read a value + if readFromCache || len(e.handleVal) == 0 { + if !readFromCache { + err = e.lockKeyIfNeeded(ctx, e.idxKey) + if err != nil { + return err + } + } // Change the unique index LOCK into PUT record. if e.lock && len(e.handleVal) > 0 { if !e.txn.Valid() { From 0c333c070fd1e947987beec733c4a1b571244478 Mon Sep 17 00:00:00 2001 From: you06 Date: Tue, 12 Jul 2022 20:42:18 +0800 Subject: [PATCH 3/6] fix lint&test Signed-off-by: you06 --- executor/point_get.go | 2 +- server/conn_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/executor/point_get.go b/executor/point_get.go index 7360e55154224..e71e4c0b706a3 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -238,7 +238,7 @@ func (e *PointGetExecutor) Next(ctx context.Context, req *chunk.Chunk) error { } readFromCache := !e.ctx.GetSessionVars().IsPessimisticReadConsistency() - if readFromCache{ + if readFromCache { err = e.lockKeyIfNeeded(ctx, e.idxKey) if err != nil { return err diff --git a/server/conn_test.go b/server/conn_test.go index f9661226ae1c3..df0d5fbef57d1 100644 --- a/server/conn_test.go +++ b/server/conn_test.go @@ -829,13 +829,13 @@ func TestPrefetchPointKeys(t *testing.T) { tk.MustExec("begin pessimistic") tk.MustExec("update prefetch set c = c + 1 where a = 2 and b = 2") - require.Equal(t, 1, tk.Session().GetSessionVars().TxnCtx.PessimisticCacheHit) + require.Equal(t, 2, tk.Session().GetSessionVars().TxnCtx.PessimisticCacheHit) err = cc.handleQuery(ctx, query) require.NoError(t, err) txn, err = tk.Session().Txn(false) require.NoError(t, err) require.True(t, txn.Valid()) - require.Equal(t, 5, tk.Session().GetSessionVars().TxnCtx.PessimisticCacheHit) + require.Equal(t, 6, tk.Session().GetSessionVars().TxnCtx.PessimisticCacheHit) tk.MustExec("commit") tk.MustQuery("select * from prefetch").Check(testkit.Rows("1 1 3", "2 2 6", "3 3 5")) } From fc32324654192922a4725cd6b06c032526876cd9 Mon Sep 17 00:00:00 2001 From: you06 Date: Tue, 12 Jul 2022 22:39:39 +0800 Subject: [PATCH 4/6] fix bug Signed-off-by: you06 --- executor/point_get.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/point_get.go b/executor/point_get.go index e71e4c0b706a3..65f34f15c31b5 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -254,7 +254,7 @@ func (e *PointGetExecutor) Next(ctx context.Context, req *chunk.Chunk) error { // try lock the index key if isolation level is not read consistency // also lock key if read consistency read a value - if readFromCache || len(e.handleVal) == 0 { + if readFromCache || len(e.handleVal) > 0 { if !readFromCache { err = e.lockKeyIfNeeded(ctx, e.idxKey) if err != nil { From f54ef82771bc4fd6285ba7d7ccd87789061a69cd Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 13 Jul 2022 13:59:46 +0800 Subject: [PATCH 5/6] address comment Signed-off-by: you06 --- executor/point_get.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/executor/point_get.go b/executor/point_get.go index 65f34f15c31b5..8baa02111241c 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -237,8 +237,10 @@ func (e *PointGetExecutor) Next(ctx context.Context, req *chunk.Chunk) error { return err } - readFromCache := !e.ctx.GetSessionVars().IsPessimisticReadConsistency() - if readFromCache { + lockNonExistIdxKey := !e.ctx.GetSessionVars().IsPessimisticReadConsistency() + // Non-exist keys are also locked if the isolation level is not read consistency, + // lock it before read here, then it's able to read from pessimistic lock cache. + if lockNonExistIdxKey { err = e.lockKeyIfNeeded(ctx, e.idxKey) if err != nil { return err @@ -252,10 +254,10 @@ func (e *PointGetExecutor) Next(ctx context.Context, req *chunk.Chunk) error { } } - // try lock the index key if isolation level is not read consistency // also lock key if read consistency read a value - if readFromCache || len(e.handleVal) > 0 { - if !readFromCache { + // TODO: pessimistic lock support lock-if-exist. + if lockNonExistIdxKey || len(e.handleVal) > 0 { + if !lockNonExistIdxKey { err = e.lockKeyIfNeeded(ctx, e.idxKey) if err != nil { return err From c2d301b388affcc59064b920f11f8779dae36c8f Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 13 Jul 2022 14:05:43 +0800 Subject: [PATCH 6/6] add comment Signed-off-by: you06 --- executor/point_get.go | 1 + 1 file changed, 1 insertion(+) diff --git a/executor/point_get.go b/executor/point_get.go index 8baa02111241c..e744570cbf370 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -237,6 +237,7 @@ func (e *PointGetExecutor) Next(ctx context.Context, req *chunk.Chunk) error { return err } + // lockNonExistIdxKey indicates the key will be locked regardless of its existence. lockNonExistIdxKey := !e.ctx.GetSessionVars().IsPessimisticReadConsistency() // Non-exist keys are also locked if the isolation level is not read consistency, // lock it before read here, then it's able to read from pessimistic lock cache.