From 586265b9a14b8feee75b68b5b18fbddfa3a1e449 Mon Sep 17 00:00:00 2001 From: you06 Date: Sat, 6 May 2023 11:51:13 +0800 Subject: [PATCH 1/7] remove stale-read when key-is-locked Signed-off-by: you06 --- tikvrpc/tikvrpc.go | 10 ++++++++++ txnkv/txnsnapshot/snapshot.go | 2 ++ 2 files changed, 12 insertions(+) diff --git a/tikvrpc/tikvrpc.go b/tikvrpc/tikvrpc.go index 8903bdfcc..78856e86b 100644 --- a/tikvrpc/tikvrpc.go +++ b/tikvrpc/tikvrpc.go @@ -275,6 +275,16 @@ func (req *Request) EnableStaleRead() { req.ReplicaRead = false } +// DisableStaleRead is called when stale-read fallbacks to leader read after meeting key-is-locked error. +func (req *Request) DisableStaleRead() { + // only take effect for stale-read requests. + if !req.StaleRead { + return + } + req.StaleRead = false + req.ReplicaReadType = kv.ReplicaReadLeader +} + // IsGlobalStaleRead checks if the request is a global stale read request. func (req *Request) IsGlobalStaleRead() bool { return req.ReadReplicaScope == oracle.GlobalTxnScope && diff --git a/txnkv/txnsnapshot/snapshot.go b/txnkv/txnsnapshot/snapshot.go index 12a0e544d..b517779e3 100644 --- a/txnkv/txnsnapshot/snapshot.go +++ b/txnkv/txnsnapshot/snapshot.go @@ -504,6 +504,7 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, } else { cli.UpdateResolvingLocks(locks, s.version, *resolvingRecordToken) } + req.DisableStaleRead() resolveLocksOpts := txnlock.ResolveLocksOptions{ CallerStartTS: s.version, Locks: locks, @@ -694,6 +695,7 @@ func (s *KVSnapshot) get(ctx context.Context, bo *retry.Backoffer, k []byte) ([] return nil, err } if firstLock == nil { + req.DisableStaleRead() firstLock = lock } else if s.version == maxTimestamp && firstLock.TxnID != lock.TxnID { // If it is an autocommit point get, it needs to be blocked only From 1c511bf0222b1ddb7c2d907aafe1d12212f3a518 Mon Sep 17 00:00:00 2001 From: you06 Date: Tue, 9 May 2023 22:35:21 +0800 Subject: [PATCH 2/7] disable follower read for batch get Signed-off-by: you06 --- txnkv/txnsnapshot/snapshot.go | 1 + 1 file changed, 1 insertion(+) diff --git a/txnkv/txnsnapshot/snapshot.go b/txnkv/txnsnapshot/snapshot.go index b517779e3..c374c3e81 100644 --- a/txnkv/txnsnapshot/snapshot.go +++ b/txnkv/txnsnapshot/snapshot.go @@ -462,6 +462,7 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, locks []*txnlock.Lock ) if keyErr := batchGetResp.GetError(); keyErr != nil { + req.DisableStaleRead() // If a response-level error happens, skip reading pairs. lock, err := txnlock.ExtractLockFromKeyErr(keyErr) if err != nil { From 0f43d666804f0c580924d27087bfc8606f95269c Mon Sep 17 00:00:00 2001 From: you06 Date: Thu, 25 May 2023 16:34:11 +0800 Subject: [PATCH 3/7] the stale-read flag may already set to false Signed-off-by: you06 --- tikvrpc/tikvrpc.go | 4 ---- txnkv/txnsnapshot/snapshot.go | 12 +++++++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/tikvrpc/tikvrpc.go b/tikvrpc/tikvrpc.go index 78856e86b..8b8e7dd9e 100644 --- a/tikvrpc/tikvrpc.go +++ b/tikvrpc/tikvrpc.go @@ -277,10 +277,6 @@ func (req *Request) EnableStaleRead() { // DisableStaleRead is called when stale-read fallbacks to leader read after meeting key-is-locked error. func (req *Request) DisableStaleRead() { - // only take effect for stale-read requests. - if !req.StaleRead { - return - } req.StaleRead = false req.ReplicaReadType = kv.ReplicaReadLeader } diff --git a/txnkv/txnsnapshot/snapshot.go b/txnkv/txnsnapshot/snapshot.go index c374c3e81..d8c3953d2 100644 --- a/txnkv/txnsnapshot/snapshot.go +++ b/txnkv/txnsnapshot/snapshot.go @@ -462,7 +462,9 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, locks []*txnlock.Lock ) if keyErr := batchGetResp.GetError(); keyErr != nil { - req.DisableStaleRead() + if isStaleness { + req.DisableStaleRead() + } // If a response-level error happens, skip reading pairs. lock, err := txnlock.ExtractLockFromKeyErr(keyErr) if err != nil { @@ -505,7 +507,9 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, } else { cli.UpdateResolvingLocks(locks, s.version, *resolvingRecordToken) } - req.DisableStaleRead() + if isStaleness { + req.DisableStaleRead() + } resolveLocksOpts := txnlock.ResolveLocksOptions{ CallerStartTS: s.version, Locks: locks, @@ -696,7 +700,9 @@ func (s *KVSnapshot) get(ctx context.Context, bo *retry.Backoffer, k []byte) ([] return nil, err } if firstLock == nil { - req.DisableStaleRead() + if isStaleness { + req.DisableStaleRead() + } firstLock = lock } else if s.version == maxTimestamp && firstLock.TxnID != lock.TxnID { // If it is an autocommit point get, it needs to be blocked only From 9ea379ac1301ff5a76eba2d22b31c033b54f9121 Mon Sep 17 00:00:00 2001 From: you06 Date: Thu, 25 May 2023 19:56:14 +0800 Subject: [PATCH 4/7] fix batchget Signed-off-by: you06 --- txnkv/txnsnapshot/snapshot.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/txnkv/txnsnapshot/snapshot.go b/txnkv/txnsnapshot/snapshot.go index d8c3953d2..1ba9039fc 100644 --- a/txnkv/txnsnapshot/snapshot.go +++ b/txnkv/txnsnapshot/snapshot.go @@ -381,6 +381,7 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, s.mergeRegionRequestStats(cli.Stats) }() } + isStaleness := s.mu.isStaleness s.mu.RUnlock() pending := batch.keys @@ -406,7 +407,6 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, s.mu.resourceGroupTagger(req) } scope := s.mu.readReplicaScope - isStaleness := s.mu.isStaleness matchStoreLabels := s.mu.matchStoreLabels replicaAdjuster := s.mu.replicaReadAdjuster s.mu.RUnlock() @@ -462,9 +462,6 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, locks []*txnlock.Lock ) if keyErr := batchGetResp.GetError(); keyErr != nil { - if isStaleness { - req.DisableStaleRead() - } // If a response-level error happens, skip reading pairs. lock, err := txnlock.ExtractLockFromKeyErr(keyErr) if err != nil { @@ -507,9 +504,8 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, } else { cli.UpdateResolvingLocks(locks, s.version, *resolvingRecordToken) } - if isStaleness { - req.DisableStaleRead() - } + // we need to read from leader after resolving the lock. + isStaleness = false resolveLocksOpts := txnlock.ResolveLocksOptions{ CallerStartTS: s.version, Locks: locks, @@ -700,6 +696,7 @@ func (s *KVSnapshot) get(ctx context.Context, bo *retry.Backoffer, k []byte) ([] return nil, err } if firstLock == nil { + // we need to read from leader after resolving the lock. if isStaleness { req.DisableStaleRead() } From 0ba0624db12df06fe2b6ee6cd4828cea0e4b38f4 Mon Sep 17 00:00:00 2001 From: you06 Date: Thu, 25 May 2023 20:39:20 +0800 Subject: [PATCH 5/7] Reset busy-threshold when stale read fallback to leader read Signed-off-by: you06 --- txnkv/txnsnapshot/snapshot.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/txnkv/txnsnapshot/snapshot.go b/txnkv/txnsnapshot/snapshot.go index 1ba9039fc..f4f227dbf 100644 --- a/txnkv/txnsnapshot/snapshot.go +++ b/txnkv/txnsnapshot/snapshot.go @@ -382,6 +382,7 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, }() } isStaleness := s.mu.isStaleness + busyThresholdMs := s.mu.busyThreshold.Milliseconds() s.mu.RUnlock() pending := batch.keys @@ -401,7 +402,7 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, ResourceControlContext: &kvrpcpb.ResourceControlContext{ ResourceGroupName: util.ResourceGroupNameFromCtx(bo.GetCtx()), }, - BusyThresholdMs: uint32(s.mu.busyThreshold.Milliseconds()), + BusyThresholdMs: uint32(busyThresholdMs), }) if s.mu.resourceGroupTag == nil && s.mu.resourceGroupTagger != nil { s.mu.resourceGroupTagger(req) @@ -505,7 +506,10 @@ func (s *KVSnapshot) batchGetSingleRegion(bo *retry.Backoffer, batch batchKeys, cli.UpdateResolvingLocks(locks, s.version, *resolvingRecordToken) } // we need to read from leader after resolving the lock. - isStaleness = false + if isStaleness { + isStaleness = false + busyThresholdMs = 0 + } resolveLocksOpts := txnlock.ResolveLocksOptions{ CallerStartTS: s.version, Locks: locks, @@ -699,6 +703,7 @@ func (s *KVSnapshot) get(ctx context.Context, bo *retry.Backoffer, k []byte) ([] // we need to read from leader after resolving the lock. if isStaleness { req.DisableStaleRead() + req.BusyThresholdMs = 0 } firstLock = lock } else if s.version == maxTimestamp && firstLock.TxnID != lock.TxnID { From 0f3d85aa44c9fd0d409ca6424fdd4d9a26c81ba3 Mon Sep 17 00:00:00 2001 From: you06 Date: Fri, 26 May 2023 11:53:44 +0800 Subject: [PATCH 6/7] address comment Signed-off-by: you06 --- tikvrpc/tikvrpc.go | 4 ++-- txnkv/txnsnapshot/snapshot.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tikvrpc/tikvrpc.go b/tikvrpc/tikvrpc.go index 8b8e7dd9e..4d81e1481 100644 --- a/tikvrpc/tikvrpc.go +++ b/tikvrpc/tikvrpc.go @@ -275,8 +275,8 @@ func (req *Request) EnableStaleRead() { req.ReplicaRead = false } -// DisableStaleRead is called when stale-read fallbacks to leader read after meeting key-is-locked error. -func (req *Request) DisableStaleRead() { +// DisableStaleReadMeetLock is called when stale-read fallbacks to leader read after meeting key-is-locked error. +func (req *Request) DisableStaleReadMeetLock() { req.StaleRead = false req.ReplicaReadType = kv.ReplicaReadLeader } diff --git a/txnkv/txnsnapshot/snapshot.go b/txnkv/txnsnapshot/snapshot.go index f4f227dbf..7508c57fd 100644 --- a/txnkv/txnsnapshot/snapshot.go +++ b/txnkv/txnsnapshot/snapshot.go @@ -702,7 +702,7 @@ func (s *KVSnapshot) get(ctx context.Context, bo *retry.Backoffer, k []byte) ([] if firstLock == nil { // we need to read from leader after resolving the lock. if isStaleness { - req.DisableStaleRead() + req.DisableStaleReadMeetLock() req.BusyThresholdMs = 0 } firstLock = lock From 99587094d9e91c883857640887323a03a59d6387 Mon Sep 17 00:00:00 2001 From: you06 Date: Fri, 26 May 2023 13:47:19 +0800 Subject: [PATCH 7/7] trigger CI Signed-off-by: you06