From 03c6f529df71a666d2dffcfab35511371a450f40 Mon Sep 17 00:00:00 2001 From: lysu Date: Thu, 25 Apr 2019 15:07:53 +0800 Subject: [PATCH 01/27] tikvclient: try follower peers when leader unreachable --- store/tikv/region_cache.go | 185 ++++++++++++++++++++++++-------- store/tikv/region_cache_test.go | 46 +++++--- store/tikv/region_request.go | 2 +- 3 files changed, 173 insertions(+), 60 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index a48207c7b88fb..0b202a8d65f5b 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -29,6 +29,7 @@ import ( "github.com/pingcap/tidb/metrics" "github.com/pingcap/tidb/util/logutil" "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) const ( @@ -63,9 +64,10 @@ type RegionCache struct { pdClient pd.Client mu struct { - sync.RWMutex - regions map[RegionVerID]*CachedRegion - sorted *btree.BTree + sync.RWMutex // mutex protect cached region + regions map[RegionVerID]*CachedRegion // cached regions be organized as regionVerID to region ref mapping + sorted *btree.BTree // cache regions be organized as sorted key to region ref mapping + regionsInStores map[uint64]map[RegionVerID]struct{} // store's cached region info(storeID to regionVerID mapping)` } storeMu struct { sync.RWMutex @@ -79,6 +81,7 @@ func NewRegionCache(pdClient pd.Client) *RegionCache { pdClient: pdClient, } c.mu.regions = make(map[RegionVerID]*CachedRegion) + c.mu.regionsInStores = make(map[uint64]map[RegionVerID]struct{}) c.mu.sorted = btree.New(btreeDegree) c.storeMu.stores = make(map[uint64]*Store) return c @@ -322,6 +325,15 @@ func (c *RegionCache) insertRegionToCache(r *Region) { region: r, lastAccess: time.Now().Unix(), } + // maintain storeID to regionVerIDs mapping. + for _, peer := range r.meta.Peers { + regionsInStore, ok := c.mu.regionsInStores[peer.StoreId] + if !ok { + regionsInStore = make(map[RegionVerID]struct{}) + c.mu.regionsInStores[peer.StoreId] = regionsInStore + } + regionsInStore[r.VerID()] = struct{}{} + } } // getCachedRegion loads a region from cache. It also checks if the region has @@ -381,6 +393,15 @@ func (c *RegionCache) dropRegionFromCache(verID RegionVerID) { tikvRegionCacheCounterWithDropRegionFromCacheOK.Inc() c.mu.sorted.Delete(newBtreeItem(r.region)) delete(c.mu.regions, verID) + // cleanup region info from each store's region mapping. + for _, peer := range r.region.meta.Peers { + if regionsInStore, exists := c.mu.regionsInStores[peer.StoreId]; exists { + delete(regionsInStore, verID) + if len(regionsInStore) == 0 { + delete(c.mu.regionsInStores, peer.StoreId) + } + } + } } // loadRegion loads region from pd client, and picks the first peer as leader. @@ -424,11 +445,13 @@ func (c *RegionCache) loadRegion(bo *Backoffer, key []byte, isEndKey bool) (*Reg continue } region := &Region{ - meta: meta, - peer: meta.Peers[0], + meta: meta, + backupPeers: make(map[uint64]*metapb.Peer, len(meta.Peers)-1), } if leader != nil { region.SwitchPeer(leader.GetStoreId()) + } else { + region.tryRandPeer() } return region, nil } @@ -462,11 +485,13 @@ func (c *RegionCache) loadRegionByID(bo *Backoffer, regionID uint64) (*Region, e return nil, errors.New("receive Region with no peer") } region := &Region{ - meta: meta, - peer: meta.Peers[0], + meta: meta, + backupPeers: make(map[uint64]*metapb.Peer, len(meta.Peers)-1), } if leader != nil { region.SwitchPeer(leader.GetStoreId()) + } else { + region.tryRandPeer() } return region, nil } @@ -532,40 +557,91 @@ func (c *RegionCache) loadStoreAddr(bo *Backoffer, id uint64) (string, error) { } } -// DropStoreOnSendRequestFail is used for clearing cache when a tikv server does not respond. -func (c *RegionCache) DropStoreOnSendRequestFail(ctx *RPCContext, err error) { - // We need to drop the store only when the request is the first one failed on this store. - // Because too many concurrently requests trying to drop the store will be blocked on the lock. - failedRegionID := ctx.Region +// trySwitchNextPeer give up unreachable peer and tries to select another valid peer. +// It returns false if all peers are unreachable. +func (r *Region) trySwitchNextPeer(failedStoreID uint64) (hasReachable bool) { + // try other peers in rand order(set). + if len(r.backupPeers) > 0 { + var ( + id uint64 + peer *metapb.Peer + ) + for id, peer = range r.backupPeers { + break + } + r.peer = peer + delete(r.backupPeers, id) + hasReachable = true + return + } + // return quickly if no other reachable peers. + return +} + +// tryDropBackUpPeers give up unreachable peer in backup peer list. +func (r *Region) tryDropBackupPeers(failStoreID uint64) { + delete(r.backupPeers, failStoreID) +} + +// OnSendRequestFail is used for clearing cache when a tikv server does not respond. +func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { failedStoreID := ctx.Peer.StoreId - c.mu.Lock() - _, ok := c.mu.regions[failedRegionID] - if !ok { - // The failed region is dropped already by another request, we don't need to iterate the regions - // and find regions on the failed store to drop. - c.mu.Unlock() + + // try to mark those kv store as 'start dropping' status by CAS + // to keep just one fail request to handle this store's failure logic + c.storeMu.Lock() + store, exists := c.storeMu.stores[failedStoreID] + if !exists { + // store already be dropped by others(also all region's peer info has be cleanup), so return quickly. + c.storeMu.Unlock() + return + } + if !atomic.CompareAndSwapInt32(&store.Dropping, 0, 1) { + // store already be dropping by others, so return quickly too. + c.storeMu.Unlock() return } - for id, r := range c.mu.regions { - if r.region.peer.GetStoreId() == failedStoreID { - c.dropRegionFromCache(id) + c.storeMu.Unlock() + + // make regions on fail store to try other follower peers. + // drop region if this store is last one for this region. + c.mu.Lock() + if regionInStore, exists := c.mu.regionsInStores[failedStoreID]; exists { + for regionID := range regionInStore { + cacheItem := c.mu.regions[regionID] + if cacheItem.region.peer.GetStoreId() == failedStoreID { + if !cacheItem.region.trySwitchNextPeer(failedStoreID) { + c.dropRegionFromCache(regionID) + } + } else { + cacheItem.region.tryDropBackupPeers(failedStoreID) + } } } c.mu.Unlock() - // Store's meta may be out of date. - var failedStoreAddr string + // do real drop store, it must be handled after drop region logic. + // because `GetRPCContext` will refill store data if region can be get. c.storeMu.Lock() - store, ok := c.storeMu.stores[failedStoreID] - if ok { - failedStoreAddr = store.Addr + store, exists = c.storeMu.stores[failedStoreID] + if !exists { + c.storeMu.Unlock() + return + } + if atomic.LoadInt32(&store.Dropping) == 1 { delete(c.storeMu.stores, failedStoreID) + } else { + logutil.Logger(context.Background()).Error("impossible...") //TODO: remove this after test.. } c.storeMu.Unlock() - logutil.Logger(context.Background()).Info("drop regions that on the store due to send request fail", - zap.Uint64("store", failedStoreID), - zap.String("store addr", failedStoreAddr), - zap.Error(err)) + + // TODO: fix refine log. + logger := logutil.Logger(context.Background()) + if logger.Core().Enabled(zapcore.InfoLevel) { + logger.Info("drop regions that on the store due to send request fail", + zap.Uint64("store", failedStoreID), + zap.Error(err)) + } } // OnRegionEpochNotMatch removes the old region and inserts new regions into the cache. @@ -594,8 +670,8 @@ func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, curr } } region := &Region{ - meta: meta, - peer: meta.Peers[0], + meta: meta, + backupPeers: make(map[uint64]*metapb.Peer, len(meta.Peers)-1), } region.SwitchPeer(ctx.Peer.GetStoreId()) c.insertRegionToCache(region) @@ -633,8 +709,9 @@ func (item *btreeItem) Less(other btree.Item) bool { // Region stores region's meta and its leader peer. type Region struct { - meta *metapb.Region - peer *metapb.Peer + meta *metapb.Region // region meta, immutable after creation + peer *metapb.Peer // peer used by current region + backupPeers map[uint64]*metapb.Peer // reachable peers can be try after `peer` unreachable } // GetID returns id. @@ -685,13 +762,36 @@ func (r *Region) GetContext() *kvrpcpb.Context { // SwitchPeer switches current peer to the one on specific store. It returns // false if no peer matches the storeID. func (r *Region) SwitchPeer(storeID uint64) bool { - for _, p := range r.meta.Peers { - if p.GetStoreId() == storeID { - r.peer = p - return true + r.backupPeers = make(map[uint64]*metapb.Peer, len(r.meta.Peers)-1) + var leaderFound bool + for i := range r.meta.Peers { + v := r.meta.Peers[i] + if v.GetStoreId() == storeID { + leaderFound = true + r.peer = v + } else { + r.backupPeers[v.StoreId] = v } } - return false + if !leaderFound && r.peer == nil { + var ( + id uint64 + peer *metapb.Peer + ) + for id, peer = range r.backupPeers { + break + } + r.peer = peer + delete(r.backupPeers, id) + } + return leaderFound +} + +func (r *Region) tryRandPeer() { + r.peer = r.meta.Peers[0] + for i := 1; i < len(r.meta.Peers); i++ { + r.backupPeers[r.meta.Peers[i].StoreId] = r.meta.Peers[i] + } } // Contains checks whether the key is in the region, for the maximum region endKey is empty. @@ -709,8 +809,9 @@ func (r *Region) ContainsByEnd(key []byte) bool { (bytes.Compare(key, r.meta.GetEndKey()) <= 0 || len(r.meta.GetEndKey()) == 0) } -// Store contains a tikv server's address. +// Store contains a kv server's address. type Store struct { - ID uint64 - Addr string + ID uint64 // store id + Addr string // store address + Dropping int32 // true if on dropping, to control concurrent drop store requests. } diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 5fd5bf62c68fc..33f9b7d36c83a 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -261,14 +261,14 @@ func (s *testRegionCacheSuite) TestReconnect(c *C) { func (s *testRegionCacheSuite) TestRequestFail(c *C) { region := s.getRegion(c, []byte("a")) - + c.Assert(region.backupPeers, HasLen, len(region.meta.Peers)-1) ctx, _ := s.cache.GetRPCContext(s.bo, region.VerID()) - s.cache.DropStoreOnSendRequestFail(ctx, errors.New("test error")) - c.Assert(s.cache.mu.regions, HasLen, 0) + s.cache.OnSendRequestFail(ctx, errors.New("test error")) + c.Assert(region.backupPeers, HasLen, len(region.meta.Peers)-2) region = s.getRegion(c, []byte("a")) c.Assert(s.cache.mu.regions, HasLen, 1) ctx, _ = s.cache.GetRPCContext(s.bo, region.VerID()) - s.cache.DropStoreOnSendRequestFail(ctx, errors.New("test error")) + s.cache.OnSendRequestFail(ctx, errors.New("test error")) c.Assert(len(s.cache.mu.regions), Equals, 0) s.getRegion(c, []byte("a")) c.Assert(s.cache.mu.regions, HasLen, 1) @@ -292,10 +292,13 @@ func (s *testRegionCacheSuite) TestRequestFail2(c *C) { ctx, _ := s.cache.GetRPCContext(s.bo, loc1.Region) c.Assert(s.cache.storeMu.stores, HasLen, 1) s.checkCache(c, 2) - s.cache.DropStoreOnSendRequestFail(ctx, errors.New("test error")) + s.cache.OnSendRequestFail(ctx, errors.New("test error")) // Both region2 and store should be dropped from cache. c.Assert(s.cache.storeMu.stores, HasLen, 0) - c.Assert(s.cache.searchCachedRegion([]byte("x"), true), IsNil) + ctx2, _ := s.cache.GetRPCContext(s.bo, loc2.Region) + s.cache.OnSendRequestFail(ctx2, errors.New("test error")) + r := s.cache.searchCachedRegion([]byte("x"), true) + c.Assert(r, IsNil) s.checkCache(c, 0) } @@ -322,8 +325,8 @@ func (s *testRegionCacheSuite) TestRegionEpochAheadOfTiKV(c *C) { } func (s *testRegionCacheSuite) TestDropStoreOnSendRequestFail(c *C) { - regionCnt := 999 - cluster := createClusterWithStoresAndRegions(regionCnt) + regionCnt, storeCount := 999, 3 + cluster := createClusterWithStoresAndRegions(regionCnt, storeCount) cache := NewRegionCache(mocktikv.NewPDClient(cluster)) loadRegionsToCache(cache, regionCnt) @@ -333,11 +336,20 @@ func (s *testRegionCacheSuite) TestDropStoreOnSendRequestFail(c *C) { loc, err := cache.LocateKey(bo, []byte{}) c.Assert(err, IsNil) - // Drop the regions on one store, should drop only 1/3 of the regions. + // First two drop attempts just switch peer. + for i := 0; i < storeCount-1; i++ { + rpcCtx, err := cache.GetRPCContext(bo, loc.Region) + c.Assert(err, IsNil) + cache.OnSendRequestFail(rpcCtx, errors.New("test error")) + c.Assert(len(cache.mu.regions), Equals, regionCnt+1) + } + + // Drop the regions on one store, all region will be drop because they are in three stores. + // and fail 3 times hit at each stores. rpcCtx, err := cache.GetRPCContext(bo, loc.Region) c.Assert(err, IsNil) - cache.DropStoreOnSendRequestFail(rpcCtx, errors.New("test error")) - c.Assert(len(cache.mu.regions), Equals, regionCnt*2/3) + cache.OnSendRequestFail(rpcCtx, errors.New("test error")) + c.Assert(len(cache.mu.regions), Equals, 0) loadRegionsToCache(cache, regionCnt) c.Assert(len(cache.mu.regions), Equals, regionCnt) @@ -345,9 +357,9 @@ func (s *testRegionCacheSuite) TestDropStoreOnSendRequestFail(c *C) { const regionSplitKeyFormat = "t%08d" -func createClusterWithStoresAndRegions(regionCnt int) *mocktikv.Cluster { +func createClusterWithStoresAndRegions(regionCnt, storeCount int) *mocktikv.Cluster { cluster := mocktikv.NewCluster() - _, _, regionID, _ := mocktikv.BootstrapWithMultiStores(cluster, 3) + _, _, regionID, _ := mocktikv.BootstrapWithMultiStores(cluster, storeCount) for i := 0; i < regionCnt; i++ { rawKey := []byte(fmt.Sprintf(regionSplitKeyFormat, i)) ids := cluster.AllocIDs(4) @@ -449,12 +461,12 @@ func (s *testRegionCacheSuite) TestContainsByEnd(c *C) { func BenchmarkOnRequestFail(b *testing.B) { /* - This benchmark simulate many concurrent requests call DropStoreOnSendRequestFail method + This benchmark simulate many concurrent requests call OnSendRequestFail method after failed on a store, validate that on this scene, requests don't get blocked on the RegionCache lock. */ - regionCnt := 999 - cluster := createClusterWithStoresAndRegions(regionCnt) + regionCnt, storeCount := 998, 3 + cluster := createClusterWithStoresAndRegions(regionCnt, storeCount) cache := NewRegionCache(mocktikv.NewPDClient(cluster)) loadRegionsToCache(cache, regionCnt) bo := NewBackoffer(context.Background(), 1) @@ -471,7 +483,7 @@ func BenchmarkOnRequestFail(b *testing.B) { Meta: region.meta, Peer: region.peer, } - cache.DropStoreOnSendRequestFail(rpcCtx, nil) + cache.OnSendRequestFail(rpcCtx, nil) } }) if len(cache.mu.regions) != regionCnt*2/3 { diff --git a/store/tikv/region_request.go b/store/tikv/region_request.go index a10efc645fcaf..bf0d1c6ea3a70 100644 --- a/store/tikv/region_request.go +++ b/store/tikv/region_request.go @@ -163,7 +163,7 @@ func (s *RegionRequestSender) onSendFail(bo *Backoffer, ctx *RPCContext, err err } } - s.regionCache.DropStoreOnSendRequestFail(ctx, err) + s.regionCache.OnSendRequestFail(ctx, err) // Retry on send request failure when it's not canceled. // When a store is not available, the leader of related region should be elected quickly. From 33cf201caa8d3d3f039603bd94d63cae110ee5e5 Mon Sep 17 00:00:00 2001 From: lysu Date: Tue, 30 Apr 2019 10:26:53 +0800 Subject: [PATCH 02/27] tikv: blackout region store when send request failure --- ddl/table_split_test.go | 2 +- store/tikv/coprocessor_test.go | 2 +- store/tikv/region_cache.go | 551 +++++++++++++++++--------------- store/tikv/region_cache_test.go | 78 ++++- store/tikv/region_request.go | 8 +- store/tikv/split_test.go | 2 +- 6 files changed, 362 insertions(+), 281 deletions(-) diff --git a/ddl/table_split_test.go b/ddl/table_split_test.go index 132255a986dc6..afb33d5b4044f 100644 --- a/ddl/table_split_test.go +++ b/ddl/table_split_test.go @@ -79,7 +79,7 @@ func checkRegionStartWithTableID(c *C, id int64, store kvStore) { c.Assert(err, IsNil) // Region cache may be out of date, so we need to drop this expired region and load it again. - cache.DropRegion(loc.Region) + cache.InvalidateCachedRegion(loc.Region) if bytes.Equal(loc.StartKey, []byte(regionStartKey)) { return } diff --git a/store/tikv/coprocessor_test.go b/store/tikv/coprocessor_test.go index 12a8ed8fd352d..51b22898c9566 100644 --- a/store/tikv/coprocessor_test.go +++ b/store/tikv/coprocessor_test.go @@ -161,7 +161,7 @@ func (s *testCoprocessorSuite) TestRebuild(c *C) { regionIDs = append(regionIDs, cluster.AllocID()) peerIDs = append(peerIDs, cluster.AllocID()) cluster.Split(regionIDs[1], regionIDs[2], []byte("q"), []uint64{peerIDs[2]}, storeID) - cache.DropRegion(tasks[1].region) + cache.InvalidateCachedRegion(tasks[1].region) tasks, err = buildCopTasks(bo, cache, buildCopRanges("a", "z"), true, false) c.Assert(err, IsNil) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 0b202a8d65f5b..c716092dfb4e5 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -20,8 +20,10 @@ import ( "sync" "sync/atomic" "time" + "unsafe" "github.com/google/btree" + "github.com/grpc-ecosystem/go-grpc-middleware/util/backoffutils" "github.com/pingcap/errors" "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/kvproto/pkg/metapb" @@ -29,12 +31,11 @@ import ( "github.com/pingcap/tidb/metrics" "github.com/pingcap/tidb/util/logutil" "go.uber.org/zap" - "go.uber.org/zap/zapcore" ) const ( - btreeDegree = 32 - rcDefaultRegionCacheTTL = time.Minute * 10 + btreeDegree = 32 + rcDefaultRegionCacheTTLSec = 600 ) var ( @@ -53,10 +54,22 @@ type CachedRegion struct { lastAccess int64 } -func (c *CachedRegion) isValid() bool { +const invalidatedLastAccessTime = -1 + +func (c *CachedRegion) checkRegionCacheTTL(ts int64) bool { +retry: lastAccess := atomic.LoadInt64(&c.lastAccess) - lastAccessTime := time.Unix(lastAccess, 0) - return time.Since(lastAccessTime) < rcDefaultRegionCacheTTL + if ts-lastAccess > rcDefaultRegionCacheTTLSec { + return false + } + if !atomic.CompareAndSwapInt64(&c.lastAccess, lastAccess, ts) { + goto retry + } + return true +} + +func (c *CachedRegion) invalidate() { + atomic.StoreInt64(&c.lastAccess, invalidatedLastAccessTime) } // RegionCache caches Regions loaded from PD. @@ -64,10 +77,9 @@ type RegionCache struct { pdClient pd.Client mu struct { - sync.RWMutex // mutex protect cached region - regions map[RegionVerID]*CachedRegion // cached regions be organized as regionVerID to region ref mapping - sorted *btree.BTree // cache regions be organized as sorted key to region ref mapping - regionsInStores map[uint64]map[RegionVerID]struct{} // store's cached region info(storeID to regionVerID mapping)` + sync.RWMutex // mutex protect cached region + regions map[RegionVerID]*CachedRegion // cached regions be organized as regionVerID to region ref mapping + sorted *btree.BTree // cache regions be organized as sorted key to region ref mapping } storeMu struct { sync.RWMutex @@ -81,7 +93,6 @@ func NewRegionCache(pdClient pd.Client) *RegionCache { pdClient: pdClient, } c.mu.regions = make(map[RegionVerID]*CachedRegion) - c.mu.regionsInStores = make(map[uint64]map[RegionVerID]struct{}) c.mu.sorted = btree.New(btreeDegree) c.storeMu.stores = make(map[uint64]*Store) return c @@ -111,31 +122,35 @@ func (c *RPCContext) String() string { // GetRPCContext returns RPCContext for a region. If it returns nil, the region // must be out of date and already dropped from cache. func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, error) { - c.mu.RLock() - region := c.getCachedRegion(id) - if region == nil { - c.mu.RUnlock() + ts := time.Now().Unix() + + cachedRegion := c.getRegionCacheItemWithRLock(id) + if cachedRegion == nil { + return nil, nil + } + + if !cachedRegion.checkRegionCacheTTL(ts) { + return nil, nil + } + + store, peer := c.ensureRegionWorkPeer(cachedRegion.region, ts) + if store == nil { return nil, nil } - // Note: it is safe to use region.meta and region.peer without clone after - // unlock, because region cache will never update the content of region's meta - // or peer. On the contrary, if we want to use `region` after unlock, then we - // need to clone it to avoid data race. - meta, peer := region.meta, region.peer - c.mu.RUnlock() - addr, err := c.GetStoreAddr(bo, peer.GetStoreId()) + addr, err := store.ResolveAddr(bo, ts) if err != nil { - return nil, errors.Trace(err) + return nil, err } - if addr == "" { - // Store not found, region must be out of date. - c.DropRegion(id) + if len(addr) == 0 { + // Store not found, region must be out of date. TODO: check me? + cachedRegion.invalidate() return nil, nil } + return &RPCContext{ Region: id, - Meta: meta, + Meta: cachedRegion.region.meta, Peer: peer, Addr: addr, }, nil @@ -175,8 +190,8 @@ func (c *RegionCache) LocateKey(bo *Backoffer, key []byte) (*KeyLocation, error) } c.mu.Lock() - defer c.mu.Unlock() c.insertRegionToCache(r) + c.mu.Unlock() return &KeyLocation{ Region: r.VerID(), @@ -207,8 +222,8 @@ func (c *RegionCache) LocateEndKey(bo *Backoffer, key []byte) (*KeyLocation, err } c.mu.Lock() - defer c.mu.Unlock() c.insertRegionToCache(r) + c.mu.Unlock() return &KeyLocation{ Region: r.VerID(), @@ -238,8 +253,8 @@ func (c *RegionCache) LocateRegionByID(bo *Backoffer, regionID uint64) (*KeyLoca } c.mu.Lock() - defer c.mu.Unlock() c.insertRegionToCache(r) + c.mu.Unlock() return &KeyLocation{ Region: r.VerID(), StartKey: r.StartKey(), @@ -287,70 +302,45 @@ func (c *RegionCache) ListRegionIDsInKeyRange(bo *Backoffer, startKey, endKey [] return regionIDs, nil } -// DropRegion removes a cached Region. -func (c *RegionCache) DropRegion(id RegionVerID) { - c.mu.Lock() - defer c.mu.Unlock() - c.dropRegionFromCache(id) +// InvalidateCachedRegion removes a cached Region. +func (c *RegionCache) InvalidateCachedRegion(id RegionVerID) { + cachedRegion := c.getRegionCacheItemWithRLock(id) + if cachedRegion == nil { + return + } + tikvRegionCacheCounterWithDropRegionFromCacheOK.Inc() + cachedRegion.invalidate() } // UpdateLeader update some region cache with newer leader info. func (c *RegionCache) UpdateLeader(regionID RegionVerID, leaderStoreID uint64) { - c.mu.Lock() - defer c.mu.Unlock() - - r := c.getCachedRegion(regionID) - if r == nil { + cachedRegion := c.getRegionCacheItemWithRLock(regionID) + if cachedRegion == nil { logutil.Logger(context.Background()).Debug("regionCache: cannot find region when updating leader", zap.Uint64("regionID", regionID.GetID()), zap.Uint64("leaderStoreID", leaderStoreID)) return } - - if !r.SwitchPeer(leaderStoreID) { + if !cachedRegion.region.SwitchWorkPeer(leaderStoreID) { logutil.Logger(context.Background()).Debug("regionCache: cannot find peer when updating leader", zap.Uint64("regionID", regionID.GetID()), zap.Uint64("leaderStoreID", leaderStoreID)) - c.dropRegionFromCache(r.VerID()) + cachedRegion.invalidate() } + } // insertRegionToCache tries to insert the Region to cache. func (c *RegionCache) insertRegionToCache(r *Region) { - old := c.mu.sorted.ReplaceOrInsert(newBtreeItem(r)) - if old != nil { - delete(c.mu.regions, old.(*btreeItem).region.VerID()) - } - c.mu.regions[r.VerID()] = &CachedRegion{ + cachedRegion := &CachedRegion{ region: r, lastAccess: time.Now().Unix(), } - // maintain storeID to regionVerIDs mapping. - for _, peer := range r.meta.Peers { - regionsInStore, ok := c.mu.regionsInStores[peer.StoreId] - if !ok { - regionsInStore = make(map[RegionVerID]struct{}) - c.mu.regionsInStores[peer.StoreId] = regionsInStore - } - regionsInStore[r.VerID()] = struct{}{} - } -} - -// getCachedRegion loads a region from cache. It also checks if the region has -// not been accessed for a long time (maybe out of date). In this case, it -// returns nil so the region will be loaded from PD again. -// Note that it should be called with c.mu.RLock(), and the returned Region -// should not be used after c.mu is RUnlock(). -func (c *RegionCache) getCachedRegion(id RegionVerID) *Region { - cachedRegion, ok := c.mu.regions[id] - if !ok { - return nil - } - if cachedRegion.isValid() { - atomic.StoreInt64(&cachedRegion.lastAccess, time.Now().Unix()) - return cachedRegion.region + old := c.mu.sorted.ReplaceOrInsert(newBtreeItem(cachedRegion)) + if old != nil { + delete(c.mu.regions, old.(*btreeItem).cachedRegion.region.VerID()) } - return nil + c.mu.regions[r.VerID()] = cachedRegion } // searchCachedRegion finds a region from cache by key. Like `getCachedRegion`, @@ -358,17 +348,26 @@ func (c *RegionCache) getCachedRegion(id RegionVerID) *Region { // used after c.mu is RUnlock(). // If the given key is the end key of the region that you want, you may set the second argument to true. This is useful when processing in reverse order. func (c *RegionCache) searchCachedRegion(key []byte, isEndKey bool) *Region { - var r *Region + ts := time.Now().Unix() + var cr *CachedRegion c.mu.sorted.DescendLessOrEqual(newBtreeSearchItem(key), func(item btree.Item) bool { - r = item.(*btreeItem).region - if isEndKey && bytes.Equal(r.StartKey(), key) { - r = nil // clear result + cr = item.(*btreeItem).cachedRegion + if isEndKey && bytes.Equal(cr.region.StartKey(), key) { + cr = nil // clear result return true // iterate next item } + if !cr.checkRegionCacheTTL(ts) { + cr = nil + return true + } return false }) - if r != nil && (!isEndKey && r.Contains(key) || isEndKey && r.ContainsByEnd(key)) { - return c.getCachedRegion(r.VerID()) + if cr != nil && (!isEndKey && cr.region.Contains(key) || isEndKey && cr.region.ContainsByEnd(key)) { + workStore, _ := c.ensureRegionWorkPeer(cr.region, ts) + if workStore == nil { + return nil + } + return cr.region } return nil } @@ -385,25 +384,6 @@ func (c *RegionCache) getRegionByIDFromCache(regionID uint64) *Region { return nil } -func (c *RegionCache) dropRegionFromCache(verID RegionVerID) { - r, ok := c.mu.regions[verID] - if !ok { - return - } - tikvRegionCacheCounterWithDropRegionFromCacheOK.Inc() - c.mu.sorted.Delete(newBtreeItem(r.region)) - delete(c.mu.regions, verID) - // cleanup region info from each store's region mapping. - for _, peer := range r.region.meta.Peers { - if regionsInStore, exists := c.mu.regionsInStores[peer.StoreId]; exists { - delete(regionsInStore, verID) - if len(regionsInStore) == 0 { - delete(c.mu.regionsInStores, peer.StoreId) - } - } - } -} - // loadRegion loads region from pd client, and picks the first peer as leader. // If the given key is the end key of the region that you want, you may set the second argument to true. This is useful when processing in reverse order. func (c *RegionCache) loadRegion(bo *Backoffer, key []byte, isEndKey bool) (*Region, error) { @@ -445,13 +425,11 @@ func (c *RegionCache) loadRegion(bo *Backoffer, key []byte, isEndKey bool) (*Reg continue } region := &Region{ - meta: meta, - backupPeers: make(map[uint64]*metapb.Peer, len(meta.Peers)-1), + meta: meta, + _workPeer: unsafe.Pointer(meta.Peers[0]), } if leader != nil { - region.SwitchPeer(leader.GetStoreId()) - } else { - region.tryRandPeer() + region.SwitchWorkPeer(leader.GetStoreId()) } return region, nil } @@ -485,162 +463,126 @@ func (c *RegionCache) loadRegionByID(bo *Backoffer, regionID uint64) (*Region, e return nil, errors.New("receive Region with no peer") } region := &Region{ - meta: meta, - backupPeers: make(map[uint64]*metapb.Peer, len(meta.Peers)-1), + meta: meta, + _workPeer: unsafe.Pointer(meta.Peers[0]), } if leader != nil { - region.SwitchPeer(leader.GetStoreId()) - } else { - region.tryRandPeer() + region.SwitchWorkPeer(leader.GetStoreId()) } return region, nil } } -// GetStoreAddr returns a tikv server's address by its storeID. It checks cache -// first, sends request to pd server when necessary. -func (c *RegionCache) GetStoreAddr(bo *Backoffer, id uint64) (string, error) { - c.storeMu.RLock() - if store, ok := c.storeMu.stores[id]; ok { - c.storeMu.RUnlock() - return store.Addr, nil - } - c.storeMu.RUnlock() - return c.ReloadStoreAddr(bo, id) +func (c *RegionCache) getRegionCacheItemWithRLock(regionID RegionVerID) (cacheItem *CachedRegion) { + c.mu.RLock() + cacheItem = c.mu.regions[regionID] + c.mu.RUnlock() + return } -// ReloadStoreAddr reloads store's address. -func (c *RegionCache) ReloadStoreAddr(bo *Backoffer, id uint64) (string, error) { - addr, err := c.loadStoreAddr(bo, id) - if err != nil || addr == "" { - return "", errors.Trace(err) - } - - c.storeMu.Lock() - defer c.storeMu.Unlock() - c.storeMu.stores[id] = &Store{ - ID: id, - Addr: addr, - } - return addr, nil -} +func (c *RegionCache) ensureRegionWorkPeer(region *Region, ts int64) (workStore *Store, workPeer *metapb.Peer) { +retry: + regionPeer := region.WorkPeer() -// ClearStoreByID clears store from cache with storeID. -func (c *RegionCache) ClearStoreByID(id uint64) { - c.storeMu.Lock() - defer c.storeMu.Unlock() - delete(c.storeMu.stores, id) -} + var ( + cachedStore *Store + exists bool + ) + if regionPeer != nil { + c.storeMu.RLock() + cachedStore, exists = c.storeMu.stores[regionPeer.GetStoreId()] + c.storeMu.RUnlock() + if !exists { + cachedStore = c.getStoreByStoreID(regionPeer.GetStoreId()) + } -func (c *RegionCache) loadStoreAddr(bo *Backoffer, id uint64) (string, error) { - for { - store, err := c.pdClient.GetStore(bo.ctx, id) - if err != nil { - tikvRegionCacheCounterWithGetStoreError.Inc() - } else { - tikvRegionCacheCounterWithGetStoreOK.Inc() + if cachedStore.Reachable(ts) { + workStore = cachedStore + workPeer = regionPeer + return } - if err != nil { - if errors.Cause(err) == context.Canceled { - return "", errors.Trace(err) - } - err = errors.Errorf("loadStore from PD failed, id: %d, err: %v", id, err) - if err = bo.Backoff(BoPDRPC, err); err != nil { - return "", errors.Trace(err) - } + } + + cachedStore = nil + var newPeer *metapb.Peer + for i := range region.meta.Peers { + otherPeer := region.meta.Peers[i] + if regionPeer != nil && otherPeer.StoreId == regionPeer.GetStoreId() { continue } - if store == nil { - return "", nil + + c.storeMu.RLock() + peerStore, exists := c.storeMu.stores[otherPeer.GetStoreId()] + c.storeMu.RUnlock() + if !exists { + peerStore = c.getStoreByStoreID(otherPeer.GetStoreId()) } - return store.GetAddress(), nil - } -} -// trySwitchNextPeer give up unreachable peer and tries to select another valid peer. -// It returns false if all peers are unreachable. -func (r *Region) trySwitchNextPeer(failedStoreID uint64) (hasReachable bool) { - // try other peers in rand order(set). - if len(r.backupPeers) > 0 { - var ( - id uint64 - peer *metapb.Peer - ) - for id, peer = range r.backupPeers { + if peerStore.Reachable(ts) { + cachedStore = peerStore + newPeer = otherPeer break } - r.peer = peer - delete(r.backupPeers, id) - hasReachable = true + } + if !atomic.CompareAndSwapPointer(®ion._workPeer, unsafe.Pointer(regionPeer), unsafe.Pointer(newPeer)) { + goto retry + } + workStore = cachedStore + workPeer = newPeer + return +} + +func (c *RegionCache) getStoreByStoreID(storeID uint64) (store *Store) { + var ok bool + c.storeMu.Lock() + store, ok = c.storeMu.stores[storeID] + if ok { + c.storeMu.Unlock() return } - // return quickly if no other reachable peers. + access := &storeAccess{} + store = &Store{ + ID: storeID, + StoreLoader: c.pdClient.GetStore, + accessibility: unsafe.Pointer(access), + } + c.storeMu.stores[storeID] = store + c.storeMu.Unlock() return } -// tryDropBackUpPeers give up unreachable peer in backup peer list. -func (r *Region) tryDropBackupPeers(failStoreID uint64) { - delete(r.backupPeers, failStoreID) +// ClearStoreByID clears store from cache with storeID. +func (c *RegionCache) ClearStoreByID(id uint64) { + c.storeMu.Lock() + defer c.storeMu.Unlock() + delete(c.storeMu.stores, id) } // OnSendRequestFail is used for clearing cache when a tikv server does not respond. func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { failedStoreID := ctx.Peer.StoreId - // try to mark those kv store as 'start dropping' status by CAS - // to keep just one fail request to handle this store's failure logic - c.storeMu.Lock() + c.storeMu.RLock() store, exists := c.storeMu.stores[failedStoreID] if !exists { - // store already be dropped by others(also all region's peer info has be cleanup), so return quickly. - c.storeMu.Unlock() - return - } - if !atomic.CompareAndSwapInt32(&store.Dropping, 0, 1) { - // store already be dropping by others, so return quickly too. - c.storeMu.Unlock() + c.storeMu.RUnlock() return } - c.storeMu.Unlock() + c.storeMu.RUnlock() - // make regions on fail store to try other follower peers. - // drop region if this store is last one for this region. - c.mu.Lock() - if regionInStore, exists := c.mu.regionsInStores[failedStoreID]; exists { - for regionID := range regionInStore { - cacheItem := c.mu.regions[regionID] - if cacheItem.region.peer.GetStoreId() == failedStoreID { - if !cacheItem.region.trySwitchNextPeer(failedStoreID) { - c.dropRegionFromCache(regionID) - } - } else { - cacheItem.region.tryDropBackupPeers(failedStoreID) - } - } - } - c.mu.Unlock() + store.Mark(false) - // do real drop store, it must be handled after drop region logic. - // because `GetRPCContext` will refill store data if region can be get. - c.storeMu.Lock() - store, exists = c.storeMu.stores[failedStoreID] - if !exists { - c.storeMu.Unlock() + cr := c.getRegionCacheItemWithRLock(ctx.Region) + if cr == nil { return } - if atomic.LoadInt32(&store.Dropping) == 1 { - delete(c.storeMu.stores, failedStoreID) - } else { - logutil.Logger(context.Background()).Error("impossible...") //TODO: remove this after test.. + lastAccess := atomic.LoadInt64(&cr.lastAccess) + if lastAccess == invalidatedLastAccessTime { + return } - c.storeMu.Unlock() - - // TODO: fix refine log. - logger := logutil.Logger(context.Background()) - if logger.Core().Enabled(zapcore.InfoLevel) { - logger.Info("drop regions that on the store due to send request fail", - zap.Uint64("store", failedStoreID), - zap.Error(err)) + store, _ = c.ensureRegionWorkPeer(cr.region, time.Now().Unix()) + if store == nil { + cr.invalidate() } } @@ -649,7 +591,11 @@ func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, curr c.mu.Lock() defer c.mu.Unlock() - c.dropRegionFromCache(ctx.Region) + cachedRegion, ok := c.mu.regions[ctx.Region] + if ok { + tikvRegionCacheCounterWithDropRegionFromCacheOK.Inc() + cachedRegion.invalidate() + } // Find whether the region epoch in `ctx` is ahead of TiKV's. If so, backoff. for _, meta := range currentRegions { @@ -670,10 +616,9 @@ func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, curr } } region := &Region{ - meta: meta, - backupPeers: make(map[uint64]*metapb.Peer, len(meta.Peers)-1), + meta: meta, } - region.SwitchPeer(ctx.Peer.GetStoreId()) + region.SwitchWorkPeer(ctx.Peer.GetStoreId()) c.insertRegionToCache(region) } return nil @@ -686,14 +631,14 @@ func (c *RegionCache) PDClient() pd.Client { // btreeItem is BTree's Item that uses []byte to compare. type btreeItem struct { - key []byte - region *Region + key []byte + cachedRegion *CachedRegion } -func newBtreeItem(r *Region) *btreeItem { +func newBtreeItem(cr *CachedRegion) *btreeItem { return &btreeItem{ - key: r.StartKey(), - region: r, + key: cr.region.StartKey(), + cachedRegion: cr, } } @@ -709,9 +654,8 @@ func (item *btreeItem) Less(other btree.Item) bool { // Region stores region's meta and its leader peer. type Region struct { - meta *metapb.Region // region meta, immutable after creation - peer *metapb.Peer // peer used by current region - backupPeers map[uint64]*metapb.Peer // reachable peers can be try after `peer` unreachable + meta *metapb.Region // region meta, immutable after creation + _workPeer unsafe.Pointer // peer used by current region(*metapb.Peer) } // GetID returns id. @@ -719,6 +663,11 @@ func (r *Region) GetID() uint64 { return r.meta.GetId() } +// WorkPeer returns current work peer. +func (r *Region) WorkPeer() *metapb.Peer { + return (*metapb.Peer)(atomic.LoadPointer(&r._workPeer)) +} + // RegionVerID is a unique ID that can identify a Region at a specific version. type RegionVerID struct { id uint64 @@ -755,45 +704,24 @@ func (r *Region) GetContext() *kvrpcpb.Context { return &kvrpcpb.Context{ RegionId: r.meta.Id, RegionEpoch: r.meta.RegionEpoch, - Peer: r.peer, + Peer: r.WorkPeer(), } } -// SwitchPeer switches current peer to the one on specific store. It returns +// SwitchWorkPeer switches current peer to the one on specific store. It returns // false if no peer matches the storeID. -func (r *Region) SwitchPeer(storeID uint64) bool { - r.backupPeers = make(map[uint64]*metapb.Peer, len(r.meta.Peers)-1) +func (r *Region) SwitchWorkPeer(storeID uint64) bool { var leaderFound bool for i := range r.meta.Peers { v := r.meta.Peers[i] if v.GetStoreId() == storeID { leaderFound = true - r.peer = v - } else { - r.backupPeers[v.StoreId] = v + atomic.StorePointer(&r._workPeer, unsafe.Pointer(v)) } } - if !leaderFound && r.peer == nil { - var ( - id uint64 - peer *metapb.Peer - ) - for id, peer = range r.backupPeers { - break - } - r.peer = peer - delete(r.backupPeers, id) - } return leaderFound } -func (r *Region) tryRandPeer() { - r.peer = r.meta.Peers[0] - for i := 1; i < len(r.meta.Peers); i++ { - r.backupPeers[r.meta.Peers[i].StoreId] = r.meta.Peers[i] - } -} - // Contains checks whether the key is in the region, for the maximum region endKey is empty. // startKey <= key < endKey. func (r *Region) Contains(key []byte) bool { @@ -809,9 +737,110 @@ func (r *Region) ContainsByEnd(key []byte) bool { (bytes.Compare(key, r.meta.GetEndKey()) <= 0 || len(r.meta.GetEndKey()) == 0) } +// storeLoader loads the Store info given by storeId. +type storeLoader func(ctx context.Context, id uint64) (*metapb.Store, error) + // Store contains a kv server's address. type Store struct { - ID uint64 // store id - Addr string // store address - Dropping int32 // true if on dropping, to control concurrent drop store requests. + sync.Mutex // mutex to avoid duplicate load requests + StoreLoader storeLoader // loader to get store address from PD + lastLoadTS int64 // last load success timestamp(sec) + addr string // loaded store address + + ID uint64 // store id + + accessibility unsafe.Pointer // store's accessibility, point to *storeAccess +} + +// storeAccess contains store's access info. +type storeAccess struct { + failAttempt uint // continue fail attempt count + lastFailedTime int64 // last fail attempt time. +} + +// reResolveUnreachableStoreIntervalSec after it will trigger re-resolve if store becomes unreachable. +const reResolveUnreachableStoreIntervalSec = 3600 + +// ResolveAddr resolves the address of store. +// following up resolve request will reuse previous result until +// store become unreachable and after reResolveUnreachableStoreIntervalSec +func (s *Store) ResolveAddr(bo *Backoffer, ts int64) (addr string, err error) { + var store *metapb.Store + s.Lock() // hold store-level mutex to protect duplicate resolve request. + if len(s.addr) > 0 { + if s.Reachable(ts) || ts-s.lastLoadTS > reResolveUnreachableStoreIntervalSec { + addr = s.addr + s.Unlock() + return + } + } + // re-resolve if unreachable and long time from last load. + for { + store, err = s.StoreLoader(bo.ctx, s.ID) + if err != nil { + tikvRegionCacheCounterWithGetStoreError.Inc() + } else { + tikvRegionCacheCounterWithGetStoreOK.Inc() + } + if err != nil { + // TODO: more refine PD error status handle. + if errors.Cause(err) == context.Canceled { + s.Unlock() + return + } + err = errors.Errorf("loadStore from PD failed, id: %d, err: %v", s.ID, err) + if err = bo.Backoff(BoPDRPC, err); err != nil { + s.Unlock() + return + } + continue + } + if store == nil { + s.Unlock() + return + } + addr = store.GetAddress() + s.addr = addr + s.lastLoadTS = ts + s.Unlock() + return + } +} + +// maxExponentAttempt before this blackout time is exponent increment. +const maxExponentAttempt = 10 + +// Reachable returns whether store can be reachable. +func (s *Store) Reachable(ts int64) bool { + status := (*storeAccess)(atomic.LoadPointer(&s.accessibility)) + if status.failAttempt == 0 || status.lastFailedTime == 0 { + // return quickly if it's continue success. + return true + } + // check blackout time window to determine store's reachable. + attempt := status.failAttempt + if attempt > maxExponentAttempt { + attempt = maxExponentAttempt + } + blackoutDeadline := status.lastFailedTime + int64(backoffutils.ExponentBase2(attempt)) + return blackoutDeadline < ts +} + +// Mark marks the processing result. +func (s *Store) Mark(success bool) { +retry: + old := (*storeAccess)(atomic.LoadPointer(&s.accessibility)) + if (old.failAttempt == 0 && success) || (!success && old.failAttempt >= maxExponentAttempt) { + // return quickly if continue success, and no more mark when attempt meet max bound. + return + } + // mark store be success or fail + var newAccess storeAccess + if !success { + newAccess.lastFailedTime = time.Now().Unix() + newAccess.failAttempt = old.failAttempt + 1 + } + if !atomic.CompareAndSwapPointer(&s.accessibility, unsafe.Pointer(old), unsafe.Pointer(&newAccess)) { + goto retry + } } diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 33f9b7d36c83a..e05d42c12c165 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -17,6 +17,7 @@ import ( "context" "errors" "fmt" + "github.com/google/btree" "github.com/pingcap/kvproto/pkg/metapb" "testing" "time" @@ -57,13 +58,55 @@ func (s *testRegionCacheSuite) storeAddr(id uint64) string { } func (s *testRegionCacheSuite) checkCache(c *C, len int) { - c.Assert(s.cache.mu.regions, HasLen, len) - c.Assert(s.cache.mu.sorted.Len(), Equals, len) + ts := time.Now().Unix() + c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, len) + c.Assert(workableRegionsInBtree(s.cache, s.cache.mu.sorted, ts), Equals, len) for _, r := range s.cache.mu.regions { - c.Assert(r.region, DeepEquals, s.cache.searchCachedRegion(r.region.StartKey(), false)) + if r.checkRegionCacheTTL(ts) { + if store, _ := s.cache.ensureRegionWorkPeer(r.region, ts); store != nil { + c.Assert(r.region, DeepEquals, s.cache.searchCachedRegion(r.region.StartKey(), false)) + } + } } } +func workableRegions(cache *RegionCache, regions map[RegionVerID]*CachedRegion, ts int64) (len int) { + for _, region := range regions { + if !region.checkRegionCacheTTL(ts) { + continue + } + store, _ := cache.ensureRegionWorkPeer(region.region, ts) + if store != nil { + len++ + } + } + return +} + +func workableRegionsInBtree(cache *RegionCache, t *btree.BTree, ts int64) (len int) { + t.Descend(func(item btree.Item) bool { + r := item.(*btreeItem).cachedRegion + if !r.checkRegionCacheTTL(ts) { + return true + } + store, _ := cache.ensureRegionWorkPeer(r.region, ts) + if store != nil { + len++ + } + return true + }) + return +} + +func reachableStore(stores map[uint64]*Store, ts int64) (cnt int) { + for _, store := range stores { + if store.Reachable(ts) { + cnt++ + } + } + return +} + func (s *testRegionCacheSuite) getRegion(c *C, key []byte) *Region { _, err := s.cache.LocateKey(s.bo, key) c.Assert(err, IsNil) @@ -209,7 +252,7 @@ func (s *testRegionCacheSuite) TestSplit(c *C) { s.cluster.Split(s.region1, region2, []byte("m"), newPeers, newPeers[0]) // tikv-server reports `NotInRegion` - s.cache.DropRegion(r.VerID()) + s.cache.InvalidateCachedRegion(r.VerID()) s.checkCache(c, 0) r = s.getRegion(c, []byte("x")) @@ -236,7 +279,7 @@ func (s *testRegionCacheSuite) TestMerge(c *C) { s.cluster.Merge(s.region1, region2) // tikv-server reports `NotInRegion` - s.cache.DropRegion(loc.Region) + s.cache.InvalidateCachedRegion(loc.Region) s.checkCache(c, 0) loc, err = s.cache.LocateKey(s.bo, []byte("x")) @@ -250,7 +293,7 @@ func (s *testRegionCacheSuite) TestReconnect(c *C) { c.Assert(err, IsNil) // connect tikv-server failed, cause drop cache - s.cache.DropRegion(loc.Region) + s.cache.InvalidateCachedRegion(loc.Region) r := s.getRegion(c, []byte("a")) c.Assert(r, NotNil) @@ -260,21 +303,22 @@ func (s *testRegionCacheSuite) TestReconnect(c *C) { } func (s *testRegionCacheSuite) TestRequestFail(c *C) { + ts := time.Now().Unix() region := s.getRegion(c, []byte("a")) - c.Assert(region.backupPeers, HasLen, len(region.meta.Peers)-1) ctx, _ := s.cache.GetRPCContext(s.bo, region.VerID()) s.cache.OnSendRequestFail(ctx, errors.New("test error")) - c.Assert(region.backupPeers, HasLen, len(region.meta.Peers)-2) region = s.getRegion(c, []byte("a")) c.Assert(s.cache.mu.regions, HasLen, 1) ctx, _ = s.cache.GetRPCContext(s.bo, region.VerID()) s.cache.OnSendRequestFail(ctx, errors.New("test error")) - c.Assert(len(s.cache.mu.regions), Equals, 0) + c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 0) + time.Sleep(2 * time.Second) s.getRegion(c, []byte("a")) - c.Assert(s.cache.mu.regions, HasLen, 1) + c.Assert(workableRegions(s.cache, s.cache.mu.regions, time.Now().Unix()), Equals, 1) } func (s *testRegionCacheSuite) TestRequestFail2(c *C) { + ts := time.Now().Unix() // key range: ['' - 'm' - 'z'] region2 := s.cluster.AllocID() newPeers := s.cluster.AllocIDs(2) @@ -294,11 +338,12 @@ func (s *testRegionCacheSuite) TestRequestFail2(c *C) { s.checkCache(c, 2) s.cache.OnSendRequestFail(ctx, errors.New("test error")) // Both region2 and store should be dropped from cache. - c.Assert(s.cache.storeMu.stores, HasLen, 0) + c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 1) ctx2, _ := s.cache.GetRPCContext(s.bo, loc2.Region) s.cache.OnSendRequestFail(ctx2, errors.New("test error")) r := s.cache.searchCachedRegion([]byte("x"), true) c.Assert(r, IsNil) + c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 0) s.checkCache(c, 0) } @@ -325,12 +370,13 @@ func (s *testRegionCacheSuite) TestRegionEpochAheadOfTiKV(c *C) { } func (s *testRegionCacheSuite) TestDropStoreOnSendRequestFail(c *C) { + ts := time.Now().Unix() regionCnt, storeCount := 999, 3 cluster := createClusterWithStoresAndRegions(regionCnt, storeCount) cache := NewRegionCache(mocktikv.NewPDClient(cluster)) loadRegionsToCache(cache, regionCnt) - c.Assert(len(cache.mu.regions), Equals, regionCnt) + c.Assert(workableRegions(cache, cache.mu.regions, ts), Equals, regionCnt) bo := NewBackoffer(context.Background(), 1) loc, err := cache.LocateKey(bo, []byte{}) @@ -341,7 +387,7 @@ func (s *testRegionCacheSuite) TestDropStoreOnSendRequestFail(c *C) { rpcCtx, err := cache.GetRPCContext(bo, loc.Region) c.Assert(err, IsNil) cache.OnSendRequestFail(rpcCtx, errors.New("test error")) - c.Assert(len(cache.mu.regions), Equals, regionCnt+1) + c.Assert(workableRegions(cache, cache.mu.regions, ts), Equals, regionCnt+1) } // Drop the regions on one store, all region will be drop because they are in three stores. @@ -349,10 +395,10 @@ func (s *testRegionCacheSuite) TestDropStoreOnSendRequestFail(c *C) { rpcCtx, err := cache.GetRPCContext(bo, loc.Region) c.Assert(err, IsNil) cache.OnSendRequestFail(rpcCtx, errors.New("test error")) - c.Assert(len(cache.mu.regions), Equals, 0) + c.Assert(workableRegions(cache, cache.mu.regions, ts), Equals, 0) loadRegionsToCache(cache, regionCnt) - c.Assert(len(cache.mu.regions), Equals, regionCnt) + c.Assert(len(cache.mu.regions), Equals, regionCnt+1) } const regionSplitKeyFormat = "t%08d" @@ -481,7 +527,7 @@ func BenchmarkOnRequestFail(b *testing.B) { rpcCtx := &RPCContext{ Region: loc.Region, Meta: region.meta, - Peer: region.peer, + Peer: region.WorkPeer(), } cache.OnSendRequestFail(rpcCtx, nil) } diff --git a/store/tikv/region_request.go b/store/tikv/region_request.go index bf0d1c6ea3a70..14da0c3fa5903 100644 --- a/store/tikv/region_request.go +++ b/store/tikv/region_request.go @@ -143,9 +143,15 @@ func (s *RegionRequestSender) sendReqToRegion(bo *Backoffer, ctx *RPCContext, re } return nil, true, nil } + s.onSendSuccess(ctx) return } +func (s *RegionRequestSender) onSendSuccess(ctx *RPCContext) { + store := s.regionCache.getStoreByStoreID(ctx.Peer.StoreId) + store.Mark(true) +} + func (s *RegionRequestSender) onSendFail(bo *Backoffer, ctx *RPCContext, err error) error { // If it failed because the context is cancelled by ourself, don't retry. if errors.Cause(err) == context.Canceled { @@ -254,7 +260,7 @@ func (s *RegionRequestSender) onRegionError(bo *Backoffer, ctx *RPCContext, regi logutil.Logger(context.Background()).Debug("tikv reports region error", zap.Stringer("regionErr", regionErr), zap.Stringer("ctx", ctx)) - s.regionCache.DropRegion(ctx.Region) + s.regionCache.InvalidateCachedRegion(ctx.Region) return false, nil } diff --git a/store/tikv/split_test.go b/store/tikv/split_test.go index 06cbd732cdac8..3a40c844b14e4 100644 --- a/store/tikv/split_test.go +++ b/store/tikv/split_test.go @@ -69,7 +69,7 @@ func (s *testSplitSuite) TestSplitBatchGet(c *C) { } s.split(c, loc.Region.id, []byte("b")) - s.store.regionCache.DropRegion(loc.Region) + s.store.regionCache.InvalidateCachedRegion(loc.Region) // mocktikv will panic if it meets a not-in-region key. err = snapshot.batchGetSingleRegion(s.bo, batch, func([]byte, []byte) {}) From 4d71d79a62cd58552236a3b6bd28e5de6d1f7a50 Mon Sep 17 00:00:00 2001 From: lysu Date: Tue, 30 Apr 2019 21:43:08 +0800 Subject: [PATCH 03/27] address comment and refine --- store/tikv/region_cache.go | 416 ++++++++++++++++++-------------- store/tikv/region_cache_test.go | 14 +- store/tikv/region_request.go | 8 +- 3 files changed, 242 insertions(+), 196 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index c716092dfb4e5..62a45745cb4d0 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -36,6 +36,7 @@ import ( const ( btreeDegree = 32 rcDefaultRegionCacheTTLSec = 600 + invalidatedLastAccessTime = -1 ) var ( @@ -48,28 +49,27 @@ var ( tikvRegionCacheCounterWithGetStoreError = metrics.TiKVRegionCacheCounter.WithLabelValues("get_store", "err") ) -// CachedRegion encapsulates {Region, TTL} -type CachedRegion struct { - region *Region - lastAccess int64 +// Region presents kv region +type Region struct { + meta *metapb.Region // immutable after fetched from pd + _workStore unsafe.Pointer // point to current work store(*Store) + lastAccess int64 // last region access time, see checkRegionCacheTTL } -const invalidatedLastAccessTime = -1 - -func (c *CachedRegion) checkRegionCacheTTL(ts int64) bool { +func (r *Region) checkRegionCacheTTL(ts int64) bool { retry: - lastAccess := atomic.LoadInt64(&c.lastAccess) + lastAccess := atomic.LoadInt64(&r.lastAccess) if ts-lastAccess > rcDefaultRegionCacheTTLSec { return false } - if !atomic.CompareAndSwapInt64(&c.lastAccess, lastAccess, ts) { + if !atomic.CompareAndSwapInt64(&r.lastAccess, lastAccess, ts) { goto retry } return true } -func (c *CachedRegion) invalidate() { - atomic.StoreInt64(&c.lastAccess, invalidatedLastAccessTime) +func (r *Region) invalidate() { + atomic.StoreInt64(&r.lastAccess, invalidatedLastAccessTime) } // RegionCache caches Regions loaded from PD. @@ -77,9 +77,9 @@ type RegionCache struct { pdClient pd.Client mu struct { - sync.RWMutex // mutex protect cached region - regions map[RegionVerID]*CachedRegion // cached regions be organized as regionVerID to region ref mapping - sorted *btree.BTree // cache regions be organized as sorted key to region ref mapping + sync.RWMutex // mutex protect cached region + regions map[RegionVerID]*Region // cached regions be organized as regionVerID to region ref mapping + sorted *btree.BTree // cache regions be organized as sorted key to region ref mapping } storeMu struct { sync.RWMutex @@ -92,7 +92,7 @@ func NewRegionCache(pdClient pd.Client) *RegionCache { c := &RegionCache{ pdClient: pdClient, } - c.mu.regions = make(map[RegionVerID]*CachedRegion) + c.mu.regions = make(map[RegionVerID]*Region) c.mu.sorted = btree.New(btreeDegree) c.storeMu.stores = make(map[uint64]*Store) return c @@ -102,21 +102,21 @@ func NewRegionCache(pdClient pd.Client) *RegionCache { type RPCContext struct { Region RegionVerID Meta *metapb.Region - Peer *metapb.Peer + Store *Store Addr string } // GetStoreID returns StoreID. func (c *RPCContext) GetStoreID() uint64 { - if c.Peer != nil { - return c.Peer.StoreId + if c.Store != nil { + return c.Store.peer.StoreId } return 0 } func (c *RPCContext) String() string { return fmt.Sprintf("region ID: %d, meta: %s, peer: %s, addr: %s", - c.Region.GetID(), c.Meta, c.Peer, c.Addr) + c.Region.GetID(), c.Meta, c.Store.peer, c.Addr) } // GetRPCContext returns RPCContext for a region. If it returns nil, the region @@ -124,7 +124,7 @@ func (c *RPCContext) String() string { func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, error) { ts := time.Now().Unix() - cachedRegion := c.getRegionCacheItemWithRLock(id) + cachedRegion := c.getCachedRegionWithRLock(id) if cachedRegion == nil { return nil, nil } @@ -133,12 +133,12 @@ func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, return nil, nil } - store, peer := c.ensureRegionWorkPeer(cachedRegion.region, ts) + store := c.ensureRegionWorkStore(cachedRegion, ts) if store == nil { return nil, nil } - addr, err := store.ResolveAddr(bo, ts) + addr, err := store.resolveAddr(bo, ts) if err != nil { return nil, err } @@ -150,8 +150,8 @@ func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, return &RPCContext{ Region: id, - Meta: cachedRegion.region.meta, - Peer: peer, + Meta: cachedRegion.meta, + Store: store, Addr: addr, }, nil } @@ -171,7 +171,6 @@ func (l *KeyLocation) Contains(key []byte) bool { // LocateKey searches for the region and range that the key is located. func (c *RegionCache) LocateKey(bo *Backoffer, key []byte) (*KeyLocation, error) { - c.mu.RLock() r := c.searchCachedRegion(key, false) if r != nil { loc := &KeyLocation{ @@ -179,10 +178,8 @@ func (c *RegionCache) LocateKey(bo *Backoffer, key []byte) (*KeyLocation, error) StartKey: r.StartKey(), EndKey: r.EndKey(), } - c.mu.RUnlock() return loc, nil } - c.mu.RUnlock() r, err := c.loadRegion(bo, key, false) if err != nil { @@ -203,7 +200,6 @@ func (c *RegionCache) LocateKey(bo *Backoffer, key []byte) (*KeyLocation, error) // LocateEndKey searches for the region and range that the key is located. // Unlike LocateKey, start key of a region is exclusive and end key is inclusive. func (c *RegionCache) LocateEndKey(bo *Backoffer, key []byte) (*KeyLocation, error) { - c.mu.RLock() r := c.searchCachedRegion(key, true) if r != nil { loc := &KeyLocation{ @@ -211,10 +207,8 @@ func (c *RegionCache) LocateEndKey(bo *Backoffer, key []byte) (*KeyLocation, err StartKey: r.StartKey(), EndKey: r.EndKey(), } - c.mu.RUnlock() return loc, nil } - c.mu.RUnlock() r, err := c.loadRegion(bo, key, true) if err != nil { @@ -304,7 +298,7 @@ func (c *RegionCache) ListRegionIDsInKeyRange(bo *Backoffer, startKey, endKey [] // InvalidateCachedRegion removes a cached Region. func (c *RegionCache) InvalidateCachedRegion(id RegionVerID) { - cachedRegion := c.getRegionCacheItemWithRLock(id) + cachedRegion := c.getCachedRegionWithRLock(id) if cachedRegion == nil { return } @@ -314,33 +308,28 @@ func (c *RegionCache) InvalidateCachedRegion(id RegionVerID) { // UpdateLeader update some region cache with newer leader info. func (c *RegionCache) UpdateLeader(regionID RegionVerID, leaderStoreID uint64) { - cachedRegion := c.getRegionCacheItemWithRLock(regionID) - if cachedRegion == nil { + r := c.getCachedRegionWithRLock(regionID) + if r == nil { logutil.Logger(context.Background()).Debug("regionCache: cannot find region when updating leader", zap.Uint64("regionID", regionID.GetID()), zap.Uint64("leaderStoreID", leaderStoreID)) return } - if !cachedRegion.region.SwitchWorkPeer(leaderStoreID) { + if !c.switchWorkStore(r, leaderStoreID) { logutil.Logger(context.Background()).Debug("regionCache: cannot find peer when updating leader", zap.Uint64("regionID", regionID.GetID()), zap.Uint64("leaderStoreID", leaderStoreID)) - cachedRegion.invalidate() + r.invalidate() } - } // insertRegionToCache tries to insert the Region to cache. -func (c *RegionCache) insertRegionToCache(r *Region) { - cachedRegion := &CachedRegion{ - region: r, - lastAccess: time.Now().Unix(), - } +func (c *RegionCache) insertRegionToCache(cachedRegion *Region) { old := c.mu.sorted.ReplaceOrInsert(newBtreeItem(cachedRegion)) if old != nil { - delete(c.mu.regions, old.(*btreeItem).cachedRegion.region.VerID()) + delete(c.mu.regions, old.(*btreeItem).cachedRegion.VerID()) } - c.mu.regions[r.VerID()] = cachedRegion + c.mu.regions[cachedRegion.VerID()] = cachedRegion } // searchCachedRegion finds a region from cache by key. Like `getCachedRegion`, @@ -349,25 +338,27 @@ func (c *RegionCache) insertRegionToCache(r *Region) { // If the given key is the end key of the region that you want, you may set the second argument to true. This is useful when processing in reverse order. func (c *RegionCache) searchCachedRegion(key []byte, isEndKey bool) *Region { ts := time.Now().Unix() - var cr *CachedRegion + var r *Region + c.mu.RLock() c.mu.sorted.DescendLessOrEqual(newBtreeSearchItem(key), func(item btree.Item) bool { - cr = item.(*btreeItem).cachedRegion - if isEndKey && bytes.Equal(cr.region.StartKey(), key) { - cr = nil // clear result + r = item.(*btreeItem).cachedRegion + if isEndKey && bytes.Equal(r.StartKey(), key) { + r = nil // clear result return true // iterate next item } - if !cr.checkRegionCacheTTL(ts) { - cr = nil + if !r.checkRegionCacheTTL(ts) { + r = nil return true } return false }) - if cr != nil && (!isEndKey && cr.region.Contains(key) || isEndKey && cr.region.ContainsByEnd(key)) { - workStore, _ := c.ensureRegionWorkPeer(cr.region, ts) + c.mu.RUnlock() + if r != nil && (!isEndKey && r.Contains(key) || isEndKey && r.ContainsByEnd(key)) { + workStore := c.ensureRegionWorkStore(r, ts) if workStore == nil { return nil } - return cr.region + return r } return nil } @@ -378,7 +369,7 @@ func (c *RegionCache) searchCachedRegion(key []byte, isEndKey bool) *Region { func (c *RegionCache) getRegionByIDFromCache(regionID uint64) *Region { for v, r := range c.mu.regions { if v.id == regionID { - return r.region + return r } } return nil @@ -425,11 +416,13 @@ func (c *RegionCache) loadRegion(bo *Backoffer, key []byte, isEndKey bool) (*Reg continue } region := &Region{ - meta: meta, - _workPeer: unsafe.Pointer(meta.Peers[0]), + meta: meta, + lastAccess: time.Now().Unix(), } if leader != nil { - region.SwitchWorkPeer(leader.GetStoreId()) + c.switchWorkStore(region, leader.StoreId) + } else { + c.switchFirstStore(region) } return region, nil } @@ -463,104 +456,86 @@ func (c *RegionCache) loadRegionByID(bo *Backoffer, regionID uint64) (*Region, e return nil, errors.New("receive Region with no peer") } region := &Region{ - meta: meta, - _workPeer: unsafe.Pointer(meta.Peers[0]), + meta: meta, + lastAccess: time.Now().Unix(), } if leader != nil { - region.SwitchWorkPeer(leader.GetStoreId()) + c.switchWorkStore(region, leader.GetStoreId()) + } else { + c.switchFirstStore(region) } return region, nil } } -func (c *RegionCache) getRegionCacheItemWithRLock(regionID RegionVerID) (cacheItem *CachedRegion) { +func (c *RegionCache) getCachedRegionWithRLock(regionID RegionVerID) (r *Region) { c.mu.RLock() - cacheItem = c.mu.regions[regionID] + r = c.mu.regions[regionID] c.mu.RUnlock() return } -func (c *RegionCache) ensureRegionWorkPeer(region *Region, ts int64) (workStore *Store, workPeer *metapb.Peer) { +// ensureRegionWorkStore ensures region have workable store and return it. +func (c *RegionCache) ensureRegionWorkStore(region *Region, ts int64) (workStore *Store) { retry: - regionPeer := region.WorkPeer() - - var ( - cachedStore *Store - exists bool - ) - if regionPeer != nil { - c.storeMu.RLock() - cachedStore, exists = c.storeMu.stores[regionPeer.GetStoreId()] - c.storeMu.RUnlock() - if !exists { - cachedStore = c.getStoreByStoreID(regionPeer.GetStoreId()) - } - - if cachedStore.Reachable(ts) { - workStore = cachedStore - workPeer = regionPeer - return - } + cachedStore := region.WorkStore() + if cachedStore != nil && cachedStore.Available(ts) { + workStore = cachedStore + return } - cachedStore = nil - var newPeer *metapb.Peer + var newStore *Store for i := range region.meta.Peers { otherPeer := region.meta.Peers[i] - if regionPeer != nil && otherPeer.StoreId == regionPeer.GetStoreId() { + if cachedStore != nil && otherPeer.StoreId == cachedStore.peer.StoreId { continue } c.storeMu.RLock() - peerStore, exists := c.storeMu.stores[otherPeer.GetStoreId()] + peerStore, exists := c.storeMu.stores[otherPeer.StoreId] c.storeMu.RUnlock() if !exists { - peerStore = c.getStoreByStoreID(otherPeer.GetStoreId()) + peerStore = c.getStoreByStoreID(otherPeer) } - if peerStore.Reachable(ts) { - cachedStore = peerStore - newPeer = otherPeer + if peerStore.Available(ts) { + newStore = peerStore break } } - if !atomic.CompareAndSwapPointer(®ion._workPeer, unsafe.Pointer(regionPeer), unsafe.Pointer(newPeer)) { + if newStore == nil { + return + } + if !atomic.CompareAndSwapPointer(®ion._workStore, unsafe.Pointer(cachedStore), unsafe.Pointer(newStore)) { goto retry } - workStore = cachedStore - workPeer = newPeer + workStore = newStore return } -func (c *RegionCache) getStoreByStoreID(storeID uint64) (store *Store) { - var ok bool +func (c *RegionCache) getStoreByStoreID(peer *metapb.Peer) (store *Store) { + var ( + ok bool + storeID = peer.GetStoreId() + ) c.storeMu.Lock() store, ok = c.storeMu.stores[storeID] if ok { c.storeMu.Unlock() return } - access := &storeAccess{} store = &Store{ - ID: storeID, - StoreLoader: c.pdClient.GetStore, - accessibility: unsafe.Pointer(access), + peer: peer, } + store.resolve.fn = c.pdClient.GetStore c.storeMu.stores[storeID] = store c.storeMu.Unlock() return } -// ClearStoreByID clears store from cache with storeID. -func (c *RegionCache) ClearStoreByID(id uint64) { - c.storeMu.Lock() - defer c.storeMu.Unlock() - delete(c.storeMu.stores, id) -} - // OnSendRequestFail is used for clearing cache when a tikv server does not respond. func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { - failedStoreID := ctx.Peer.StoreId + failedStoreID := ctx.Store.peer.StoreId c.storeMu.RLock() store, exists := c.storeMu.stores[failedStoreID] @@ -570,19 +545,19 @@ func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { } c.storeMu.RUnlock() - store.Mark(false) + store.markAccess(false) - cr := c.getRegionCacheItemWithRLock(ctx.Region) - if cr == nil { + r := c.getCachedRegionWithRLock(ctx.Region) + if r == nil { return } - lastAccess := atomic.LoadInt64(&cr.lastAccess) + lastAccess := atomic.LoadInt64(&r.lastAccess) if lastAccess == invalidatedLastAccessTime { return } - store, _ = c.ensureRegionWorkPeer(cr.region, time.Now().Unix()) + store = c.ensureRegionWorkStore(r, time.Now().Unix()) if store == nil { - cr.invalidate() + r.invalidate() } } @@ -616,9 +591,10 @@ func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, curr } } region := &Region{ - meta: meta, + meta: meta, + lastAccess: time.Now().Unix(), } - region.SwitchWorkPeer(ctx.Peer.GetStoreId()) + c.switchWorkStore(region, ctx.Store.peer.GetStoreId()) c.insertRegionToCache(region) } return nil @@ -632,12 +608,12 @@ func (c *RegionCache) PDClient() pd.Client { // btreeItem is BTree's Item that uses []byte to compare. type btreeItem struct { key []byte - cachedRegion *CachedRegion + cachedRegion *Region } -func newBtreeItem(cr *CachedRegion) *btreeItem { +func newBtreeItem(cr *Region) *btreeItem { return &btreeItem{ - key: cr.region.StartKey(), + key: cr.StartKey(), cachedRegion: cr, } } @@ -652,20 +628,14 @@ func (item *btreeItem) Less(other btree.Item) bool { return bytes.Compare(item.key, other.(*btreeItem).key) < 0 } -// Region stores region's meta and its leader peer. -type Region struct { - meta *metapb.Region // region meta, immutable after creation - _workPeer unsafe.Pointer // peer used by current region(*metapb.Peer) -} - // GetID returns id. func (r *Region) GetID() uint64 { return r.meta.GetId() } -// WorkPeer returns current work peer. -func (r *Region) WorkPeer() *metapb.Peer { - return (*metapb.Peer)(atomic.LoadPointer(&r._workPeer)) +// WorkStore returns current work store. +func (r *Region) WorkStore() *Store { + return (*Store)(atomic.LoadPointer(&r._workStore)) } // RegionVerID is a unique ID that can identify a Region at a specific version. @@ -704,22 +674,53 @@ func (r *Region) GetContext() *kvrpcpb.Context { return &kvrpcpb.Context{ RegionId: r.meta.Id, RegionEpoch: r.meta.RegionEpoch, - Peer: r.WorkPeer(), + Peer: r.WorkStore().peer, } } -// SwitchWorkPeer switches current peer to the one on specific store. It returns +// switchWorkStore switches current store to the one on specific store. It returns // false if no peer matches the storeID. -func (r *Region) SwitchWorkPeer(storeID uint64) bool { - var leaderFound bool +func (c *RegionCache) switchWorkStore(r *Region, storeID uint64) (foundLeader bool) { + if len(r.meta.Peers) == 0 { + return + } + var leaderPeer *metapb.Peer for i := range r.meta.Peers { - v := r.meta.Peers[i] - if v.GetStoreId() == storeID { - leaderFound = true - atomic.StorePointer(&r._workPeer, unsafe.Pointer(v)) + p := r.meta.Peers[i] + if p.GetStoreId() == storeID { + leaderPeer = p } } - return leaderFound + if leaderPeer != nil { + foundLeader = true + } else { + leaderPeer = r.meta.Peers[0] + } + + c.storeMu.RLock() + peerStore, exists := c.storeMu.stores[leaderPeer.StoreId] + c.storeMu.RUnlock() + if !exists { + peerStore = c.getStoreByStoreID(leaderPeer) + } + atomic.StorePointer(&r._workStore, unsafe.Pointer(peerStore)) + return +} + +// switchFirstStore switches current store to first peer +// it will be used in init region but no leader info +func (c *RegionCache) switchFirstStore(r *Region) { + if len(r.meta.Peers) == 0 { + return + } + leaderPeer := r.meta.Peers[0] + c.storeMu.RLock() + peerStore, exists := c.storeMu.stores[leaderPeer.StoreId] + c.storeMu.RUnlock() + if !exists { + peerStore = c.getStoreByStoreID(leaderPeer) + } + atomic.StorePointer(&r._workStore, unsafe.Pointer(peerStore)) } // Contains checks whether the key is in the region, for the maximum region endKey is empty. @@ -737,46 +738,61 @@ func (r *Region) ContainsByEnd(key []byte) bool { (bytes.Compare(key, r.meta.GetEndKey()) <= 0 || len(r.meta.GetEndKey()) == 0) } -// storeLoader loads the Store info given by storeId. -type storeLoader func(ctx context.Context, id uint64) (*metapb.Store, error) +// fn loads the Store info given by storeId. +type resolveFunc func(ctx context.Context, id uint64) (*metapb.Store, error) -// Store contains a kv server's address. +// Store contains a kv process's address. type Store struct { - sync.Mutex // mutex to avoid duplicate load requests - StoreLoader storeLoader // loader to get store address from PD - lastLoadTS int64 // last load success timestamp(sec) - addr string // loaded store address + addr atomic.Value // loaded store address(*string) + peer *metapb.Peer // store's peer + flag storeFlag // store flag - ID uint64 // store id - - accessibility unsafe.Pointer // store's accessibility, point to *storeAccess + resolve struct { + sync.Mutex + fn resolveFunc // func to get store address from PD + } } -// storeAccess contains store's access info. -type storeAccess struct { - failAttempt uint // continue fail attempt count - lastFailedTime int64 // last fail attempt time. -} +// storeFlag contains store's access info. +// it will contain info like this: +// | <- need reload(1bit) -> | <- attempt(4bit) -> | <- lastFailedTime(59bit) -> | +type storeFlag uint64 -// reResolveUnreachableStoreIntervalSec after it will trigger re-resolve if store becomes unreachable. -const reResolveUnreachableStoreIntervalSec = 3600 +const ( + lastFailTimeMask = (1 << 59) - 1 + attemptMask = ((1 << 4) - 1) << 59 + accessCleanMask = ^((uint64(1) << 63) - 1) + needResolveMask = 1 << 63 +) -// ResolveAddr resolves the address of store. +// resolveAddr resolves the address of store. // following up resolve request will reuse previous result until // store become unreachable and after reResolveUnreachableStoreIntervalSec -func (s *Store) ResolveAddr(bo *Backoffer, ts int64) (addr string, err error) { - var store *metapb.Store - s.Lock() // hold store-level mutex to protect duplicate resolve request. - if len(s.addr) > 0 { - if s.Reachable(ts) || ts-s.lastLoadTS > reResolveUnreachableStoreIntervalSec { - addr = s.addr - s.Unlock() +func (s *Store) resolveAddr(bo *Backoffer, ts int64) (addr string, err error) { + if !s.needResolve() { + v := s.addr.Load() + if v == nil { + addr = "" return } + addr = v.(string) + return + } + s.resolve.Lock() + if !s.needResolve() { + s.resolve.Unlock() + v := s.addr.Load() + if v == nil { + addr = "" + return + } + addr = v.(string) + return } + var store *metapb.Store // re-resolve if unreachable and long time from last load. for { - store, err = s.StoreLoader(bo.ctx, s.ID) + store, err = s.resolve.fn(bo.ctx, s.peer.StoreId) if err != nil { tikvRegionCacheCounterWithGetStoreError.Inc() } else { @@ -785,24 +801,24 @@ func (s *Store) ResolveAddr(bo *Backoffer, ts int64) (addr string, err error) { if err != nil { // TODO: more refine PD error status handle. if errors.Cause(err) == context.Canceled { - s.Unlock() + s.resolve.Unlock() return } - err = errors.Errorf("loadStore from PD failed, id: %d, err: %v", s.ID, err) + err = errors.Errorf("loadStore from PD failed, id: %d, err: %v", s.peer.StoreId, err) if err = bo.Backoff(BoPDRPC, err); err != nil { - s.Unlock() + s.resolve.Unlock() return } continue } if store == nil { - s.Unlock() + s.resolve.Unlock() return } addr = store.GetAddress() - s.addr = addr - s.lastLoadTS = ts - s.Unlock() + s.addr.Store(addr) + s.markResolved(true) + s.resolve.Unlock() return } } @@ -810,37 +826,67 @@ func (s *Store) ResolveAddr(bo *Backoffer, ts int64) (addr string, err error) { // maxExponentAttempt before this blackout time is exponent increment. const maxExponentAttempt = 10 -// Reachable returns whether store can be reachable. -func (s *Store) Reachable(ts int64) bool { - status := (*storeAccess)(atomic.LoadPointer(&s.accessibility)) - if status.failAttempt == 0 || status.lastFailedTime == 0 { +// Available returns whether store be available for current. +func (s *Store) Available(ts int64) bool { + flag := atomic.LoadUint64((*uint64)(&s.flag)) + lastFailTime := int64(flag & lastFailTimeMask) + failAttempt := uint(flag&attemptMask) >> 59 + if failAttempt == 0 || lastFailTime == 0 { // return quickly if it's continue success. return true } // check blackout time window to determine store's reachable. - attempt := status.failAttempt - if attempt > maxExponentAttempt { - attempt = maxExponentAttempt + if failAttempt > maxExponentAttempt { + failAttempt = maxExponentAttempt } - blackoutDeadline := status.lastFailedTime + int64(backoffutils.ExponentBase2(attempt)) + blackoutDeadline := lastFailTime + int64(backoffutils.ExponentBase2(failAttempt)) return blackoutDeadline < ts } -// Mark marks the processing result. -func (s *Store) Mark(success bool) { +// needResolve checks whether store need resolve. +func (s *Store) needResolve() bool { + flag := storeFlag(atomic.LoadUint64((*uint64)(&s.flag))) + return flag&needResolveMask == 0 +} + +// markAccess marks the processing result. +func (s *Store) markAccess(success bool) { retry: - old := (*storeAccess)(atomic.LoadPointer(&s.accessibility)) - if (old.failAttempt == 0 && success) || (!success && old.failAttempt >= maxExponentAttempt) { + oldFlag := storeFlag(atomic.LoadUint64((*uint64)(&s.flag))) + lastFailTime := int64(oldFlag & lastFailTimeMask) + failAttempt := uint(oldFlag&attemptMask) >> 59 + if (failAttempt == 0 && success) || (!success && failAttempt >= maxExponentAttempt) { // return quickly if continue success, and no more mark when attempt meet max bound. return } - // mark store be success or fail - var newAccess storeAccess if !success { - newAccess.lastFailedTime = time.Now().Unix() - newAccess.failAttempt = old.failAttempt + 1 + if lastFailTime == 0 { + lastFailTime = time.Now().Unix() + } + failAttempt = failAttempt + 1 + } else { + lastFailTime = 0 + failAttempt = 0 } - if !atomic.CompareAndSwapPointer(&s.accessibility, unsafe.Pointer(old), unsafe.Pointer(&newAccess)) { + newFlag := uint64(oldFlag)&accessCleanMask | uint64(failAttempt<<59) | uint64(lastFailTime) + if !atomic.CompareAndSwapUint64((*uint64)(&s.flag), uint64(oldFlag), uint64(newFlag)) { + goto retry + } +} + +// markResolved marks the resolved status. +// `resolved = true` will let following requests use resolved addr +// `resolved = false` will let next request resolve store address +func (s *Store) markResolved(resolved bool) { +retry: + oldFlag := storeFlag(atomic.LoadUint64((*uint64)(&s.flag))) + newFlag := uint64(oldFlag) + if resolved { + newFlag = newFlag | needResolveMask + } else { + newFlag = newFlag & (^uint64(needResolveMask)) + } + if !atomic.CompareAndSwapUint64((*uint64)(&s.flag), uint64(oldFlag), uint64(newFlag)) { goto retry } } diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index e05d42c12c165..45a8a5eecae9d 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -63,19 +63,19 @@ func (s *testRegionCacheSuite) checkCache(c *C, len int) { c.Assert(workableRegionsInBtree(s.cache, s.cache.mu.sorted, ts), Equals, len) for _, r := range s.cache.mu.regions { if r.checkRegionCacheTTL(ts) { - if store, _ := s.cache.ensureRegionWorkPeer(r.region, ts); store != nil { - c.Assert(r.region, DeepEquals, s.cache.searchCachedRegion(r.region.StartKey(), false)) + if store := s.cache.ensureRegionWorkStore(r, ts); store != nil { + c.Assert(r, DeepEquals, s.cache.searchCachedRegion(r.StartKey(), false)) } } } } -func workableRegions(cache *RegionCache, regions map[RegionVerID]*CachedRegion, ts int64) (len int) { +func workableRegions(cache *RegionCache, regions map[RegionVerID]*Region, ts int64) (len int) { for _, region := range regions { if !region.checkRegionCacheTTL(ts) { continue } - store, _ := cache.ensureRegionWorkPeer(region.region, ts) + store := cache.ensureRegionWorkStore(region, ts) if store != nil { len++ } @@ -89,7 +89,7 @@ func workableRegionsInBtree(cache *RegionCache, t *btree.BTree, ts int64) (len i if !r.checkRegionCacheTTL(ts) { return true } - store, _ := cache.ensureRegionWorkPeer(r.region, ts) + store := cache.ensureRegionWorkStore(r, ts) if store != nil { len++ } @@ -100,7 +100,7 @@ func workableRegionsInBtree(cache *RegionCache, t *btree.BTree, ts int64) (len i func reachableStore(stores map[uint64]*Store, ts int64) (cnt int) { for _, store := range stores { - if store.Reachable(ts) { + if store.Available(ts) { cnt++ } } @@ -527,7 +527,7 @@ func BenchmarkOnRequestFail(b *testing.B) { rpcCtx := &RPCContext{ Region: loc.Region, Meta: region.meta, - Peer: region.WorkPeer(), + Store: region.WorkStore(), } cache.OnSendRequestFail(rpcCtx, nil) } diff --git a/store/tikv/region_request.go b/store/tikv/region_request.go index 14da0c3fa5903..51a4f2c5613c3 100644 --- a/store/tikv/region_request.go +++ b/store/tikv/region_request.go @@ -132,7 +132,7 @@ func (s *RegionRequestSender) SendReqCtx(bo *Backoffer, req *tikvrpc.Request, re } func (s *RegionRequestSender) sendReqToRegion(bo *Backoffer, ctx *RPCContext, req *tikvrpc.Request, timeout time.Duration) (resp *tikvrpc.Response, retry bool, err error) { - if e := tikvrpc.SetContext(req, ctx.Meta, ctx.Peer); e != nil { + if e := tikvrpc.SetContext(req, ctx.Meta, ctx.Store.peer); e != nil { return nil, false, errors.Trace(e) } resp, err = s.client.SendRequest(bo.ctx, ctx.Addr, req, timeout) @@ -148,8 +148,8 @@ func (s *RegionRequestSender) sendReqToRegion(bo *Backoffer, ctx *RPCContext, re } func (s *RegionRequestSender) onSendSuccess(ctx *RPCContext) { - store := s.regionCache.getStoreByStoreID(ctx.Peer.StoreId) - store.Mark(true) + store := s.regionCache.getStoreByStoreID(ctx.Store.peer) + store.markAccess(true) } func (s *RegionRequestSender) onSendFail(bo *Backoffer, ctx *RPCContext, err error) error { @@ -226,7 +226,7 @@ func (s *RegionRequestSender) onRegionError(bo *Backoffer, ctx *RPCContext, regi logutil.Logger(context.Background()).Warn("tikv reports `StoreNotMatch` retry later", zap.Stringer("storeNotMatch", storeNotMatch), zap.Stringer("ctx", ctx)) - s.regionCache.ClearStoreByID(ctx.GetStoreID()) + ctx.Store.markResolved(false) return true, nil } From f8c9e6cea98472b620e5597380ef0a40573bc512 Mon Sep 17 00:00:00 2001 From: lysu Date: Sun, 5 May 2019 21:19:10 +0800 Subject: [PATCH 04/27] address comment - remove _workStore --- store/tikv/region_cache.go | 105 ++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 59 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 62a45745cb4d0..7d79bd685e6fe 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -20,7 +20,6 @@ import ( "sync" "sync/atomic" "time" - "unsafe" "github.com/google/btree" "github.com/grpc-ecosystem/go-grpc-middleware/util/backoffutils" @@ -51,9 +50,22 @@ var ( // Region presents kv region type Region struct { - meta *metapb.Region // immutable after fetched from pd - _workStore unsafe.Pointer // point to current work store(*Store) - lastAccess int64 // last region access time, see checkRegionCacheTTL + meta *metapb.Region // immutable after fetched from pd + lastAccess int64 // last region access time, see checkRegionCacheTTL + stores []*Store // stores in this region + workStoreIdx int32 // point to current work +} + +func (r *Region) initStores(c *RegionCache) { + for _, p := range r.meta.Peers { + c.storeMu.RLock() + store, exists := c.storeMu.stores[p.StoreId] + c.storeMu.RUnlock() + if !exists { + store = c.getStoreByStoreID(p) + } + r.stores = append(r.stores, store) + } } func (r *Region) checkRegionCacheTTL(ts int64) bool { @@ -418,11 +430,11 @@ func (c *RegionCache) loadRegion(bo *Backoffer, key []byte, isEndKey bool) (*Reg region := &Region{ meta: meta, lastAccess: time.Now().Unix(), + stores: make([]*Store, 0, len(meta.Peers)), } + region.initStores(c) if leader != nil { c.switchWorkStore(region, leader.StoreId) - } else { - c.switchFirstStore(region) } return region, nil } @@ -458,11 +470,11 @@ func (c *RegionCache) loadRegionByID(bo *Backoffer, regionID uint64) (*Region, e region := &Region{ meta: meta, lastAccess: time.Now().Unix(), + stores: make([]*Store, 0, len(meta.Peers)), } + region.initStores(c) if leader != nil { c.switchWorkStore(region, leader.GetStoreId()) - } else { - c.switchFirstStore(region) } return region, nil } @@ -477,39 +489,33 @@ func (c *RegionCache) getCachedRegionWithRLock(regionID RegionVerID) (r *Region) // ensureRegionWorkStore ensures region have workable store and return it. func (c *RegionCache) ensureRegionWorkStore(region *Region, ts int64) (workStore *Store) { + if len(region.stores) == 0 { + return + } retry: - cachedStore := region.WorkStore() + cachedStore, cachedIdx := region.WorkStore() if cachedStore != nil && cachedStore.Available(ts) { workStore = cachedStore return } - var newStore *Store - for i := range region.meta.Peers { - otherPeer := region.meta.Peers[i] - if cachedStore != nil && otherPeer.StoreId == cachedStore.peer.StoreId { + newIdx := cachedIdx + for i, store := range region.stores { + if i == cachedIdx { continue } - - c.storeMu.RLock() - peerStore, exists := c.storeMu.stores[otherPeer.StoreId] - c.storeMu.RUnlock() - if !exists { - peerStore = c.getStoreByStoreID(otherPeer) - } - - if peerStore.Available(ts) { - newStore = peerStore + if store.Available(ts) { + newIdx = i break } } - if newStore == nil { + if newIdx == cachedIdx { return } - if !atomic.CompareAndSwapPointer(®ion._workStore, unsafe.Pointer(cachedStore), unsafe.Pointer(newStore)) { + if !atomic.CompareAndSwapInt32(®ion.workStoreIdx, int32(cachedIdx), int32(newIdx)) { goto retry } - workStore = newStore + workStore = region.stores[newIdx] return } @@ -593,7 +599,9 @@ func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, curr region := &Region{ meta: meta, lastAccess: time.Now().Unix(), + stores: make([]*Store, 0, len(meta.Peers)), } + region.initStores(c) c.switchWorkStore(region, ctx.Store.peer.GetStoreId()) c.insertRegionToCache(region) } @@ -634,8 +642,10 @@ func (r *Region) GetID() uint64 { } // WorkStore returns current work store. -func (r *Region) WorkStore() *Store { - return (*Store)(atomic.LoadPointer(&r._workStore)) +func (r *Region) WorkStore() (store *Store, idx int) { + idx = int(atomic.LoadInt32(&r.workStoreIdx)) + store = r.stores[idx] + return } // RegionVerID is a unique ID that can identify a Region at a specific version. @@ -671,10 +681,11 @@ func (r *Region) EndKey() []byte { // GetContext constructs kvprotopb.Context from region info. func (r *Region) GetContext() *kvrpcpb.Context { + store, _ := r.WorkStore() return &kvrpcpb.Context{ RegionId: r.meta.Id, RegionEpoch: r.meta.RegionEpoch, - Peer: r.WorkStore().peer, + Peer: store.peer, } } @@ -684,45 +695,21 @@ func (c *RegionCache) switchWorkStore(r *Region, storeID uint64) (foundLeader bo if len(r.meta.Peers) == 0 { return } - var leaderPeer *metapb.Peer - for i := range r.meta.Peers { - p := r.meta.Peers[i] + leaderIdx := -1 + for i, p := range r.meta.Peers { if p.GetStoreId() == storeID { - leaderPeer = p + leaderIdx = i } } - if leaderPeer != nil { + if leaderIdx >= 0 { foundLeader = true } else { - leaderPeer = r.meta.Peers[0] - } - - c.storeMu.RLock() - peerStore, exists := c.storeMu.stores[leaderPeer.StoreId] - c.storeMu.RUnlock() - if !exists { - peerStore = c.getStoreByStoreID(leaderPeer) + leaderIdx = 0 } - atomic.StorePointer(&r._workStore, unsafe.Pointer(peerStore)) + atomic.StoreInt32(&r.workStoreIdx, int32(leaderIdx)) return } -// switchFirstStore switches current store to first peer -// it will be used in init region but no leader info -func (c *RegionCache) switchFirstStore(r *Region) { - if len(r.meta.Peers) == 0 { - return - } - leaderPeer := r.meta.Peers[0] - c.storeMu.RLock() - peerStore, exists := c.storeMu.stores[leaderPeer.StoreId] - c.storeMu.RUnlock() - if !exists { - peerStore = c.getStoreByStoreID(leaderPeer) - } - atomic.StorePointer(&r._workStore, unsafe.Pointer(peerStore)) -} - // Contains checks whether the key is in the region, for the maximum region endKey is empty. // startKey <= key < endKey. func (r *Region) Contains(key []byte) bool { From 72befe2f8edf07478588ecf5dc49d4ce3955b20e Mon Sep 17 00:00:00 2001 From: lysu Date: Sun, 5 May 2019 21:54:58 +0800 Subject: [PATCH 05/27] address comment - remove bit operation --- store/tikv/region_cache.go | 76 ++++++++++++++++----------------- store/tikv/region_cache_test.go | 5 ++- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 7d79bd685e6fe..a96931bd8b7e4 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -20,6 +20,7 @@ import ( "sync" "sync/atomic" "time" + "unsafe" "github.com/google/btree" "github.com/grpc-ecosystem/go-grpc-middleware/util/backoffutils" @@ -730,9 +731,9 @@ type resolveFunc func(ctx context.Context, id uint64) (*metapb.Store, error) // Store contains a kv process's address. type Store struct { - addr atomic.Value // loaded store address(*string) - peer *metapb.Peer // store's peer - flag storeFlag // store flag + addr atomic.Value // loaded store address(*string) + peer *metapb.Peer // store's peer + state uint64 // unsafe store storeState resolve struct { sync.Mutex @@ -740,17 +741,12 @@ type Store struct { } } -// storeFlag contains store's access info. -// it will contain info like this: -// | <- need reload(1bit) -> | <- attempt(4bit) -> | <- lastFailedTime(59bit) -> | -type storeFlag uint64 - -const ( - lastFailTimeMask = (1 << 59) - 1 - attemptMask = ((1 << 4) - 1) << 59 - accessCleanMask = ^((uint64(1) << 63) - 1) - needResolveMask = 1 << 63 -) +// storeState contains store's access info. +type storeState struct { + lastFailedTime uint32 + failedAttempt uint16 + resolved bool +} // resolveAddr resolves the address of store. // following up resolve request will reuse previous result until @@ -815,48 +811,48 @@ const maxExponentAttempt = 10 // Available returns whether store be available for current. func (s *Store) Available(ts int64) bool { - flag := atomic.LoadUint64((*uint64)(&s.flag)) - lastFailTime := int64(flag & lastFailTimeMask) - failAttempt := uint(flag&attemptMask) >> 59 - if failAttempt == 0 || lastFailTime == 0 { + var state storeState + *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) + if state.failedAttempt == 0 || state.lastFailedTime == 0 { // return quickly if it's continue success. return true } // check blackout time window to determine store's reachable. - if failAttempt > maxExponentAttempt { - failAttempt = maxExponentAttempt + if state.failedAttempt > maxExponentAttempt { + state.failedAttempt = maxExponentAttempt } - blackoutDeadline := lastFailTime + int64(backoffutils.ExponentBase2(failAttempt)) + blackoutDeadline := int64(state.lastFailedTime) + int64(backoffutils.ExponentBase2(uint(state.failedAttempt))) return blackoutDeadline < ts } // needResolve checks whether store need resolve. func (s *Store) needResolve() bool { - flag := storeFlag(atomic.LoadUint64((*uint64)(&s.flag))) - return flag&needResolveMask == 0 + var state storeState + *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) + return state.resolved == false } // markAccess marks the processing result. func (s *Store) markAccess(success bool) { retry: - oldFlag := storeFlag(atomic.LoadUint64((*uint64)(&s.flag))) - lastFailTime := int64(oldFlag & lastFailTimeMask) - failAttempt := uint(oldFlag&attemptMask) >> 59 - if (failAttempt == 0 && success) || (!success && failAttempt >= maxExponentAttempt) { + oldValue := atomic.LoadUint64(&s.state) + var state storeState + *(*uint64)(unsafe.Pointer(&state)) = oldValue + if (state.failedAttempt == 0 && success) || (!success && state.failedAttempt >= maxExponentAttempt) { // return quickly if continue success, and no more mark when attempt meet max bound. return } if !success { - if lastFailTime == 0 { - lastFailTime = time.Now().Unix() + if state.lastFailedTime == 0 { + state.lastFailedTime = uint32(time.Now().Unix()) } - failAttempt = failAttempt + 1 + state.failedAttempt = state.failedAttempt + 1 } else { - lastFailTime = 0 - failAttempt = 0 + state.lastFailedTime = 0 + state.failedAttempt = 0 } - newFlag := uint64(oldFlag)&accessCleanMask | uint64(failAttempt<<59) | uint64(lastFailTime) - if !atomic.CompareAndSwapUint64((*uint64)(&s.flag), uint64(oldFlag), uint64(newFlag)) { + newValue := *(*uint64)(unsafe.Pointer(&state)) + if !atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) { goto retry } } @@ -866,14 +862,16 @@ retry: // `resolved = false` will let next request resolve store address func (s *Store) markResolved(resolved bool) { retry: - oldFlag := storeFlag(atomic.LoadUint64((*uint64)(&s.flag))) - newFlag := uint64(oldFlag) + oldValue := atomic.LoadUint64(&s.state) + var state storeState + *(*uint64)(unsafe.Pointer(&state)) = oldValue if resolved { - newFlag = newFlag | needResolveMask + state.resolved = true } else { - newFlag = newFlag & (^uint64(needResolveMask)) + state.resolved = false } - if !atomic.CompareAndSwapUint64((*uint64)(&s.flag), uint64(oldFlag), uint64(newFlag)) { + newValue := *(*uint64)(unsafe.Pointer(&state)) + if !atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) { goto retry } } diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 45a8a5eecae9d..5275ab14d6e3e 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -334,7 +334,7 @@ func (s *testRegionCacheSuite) TestRequestFail2(c *C) { // Request should fail on region1. ctx, _ := s.cache.GetRPCContext(s.bo, loc1.Region) - c.Assert(s.cache.storeMu.stores, HasLen, 1) + c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 2) s.checkCache(c, 2) s.cache.OnSendRequestFail(ctx, errors.New("test error")) // Both region2 and store should be dropped from cache. @@ -522,12 +522,13 @@ func BenchmarkOnRequestFail(b *testing.B) { } region := cache.getRegionByIDFromCache(loc.Region.id) b.ResetTimer() + store, _ := region.WorkStore() b.RunParallel(func(pb *testing.PB) { for pb.Next() { rpcCtx := &RPCContext{ Region: loc.Region, Meta: region.meta, - Store: region.WorkStore(), + Store: store, } cache.OnSendRequestFail(rpcCtx, nil) } From 2247229dd3e8b65e68a485b6ae8cb32c315a6bba Mon Sep 17 00:00:00 2001 From: lysu Date: Mon, 6 May 2019 17:04:23 +0800 Subject: [PATCH 06/27] ac: resolve fail store's addr in another goroutine --- store/tikv/coprocessor_test.go | 3 + store/tikv/kv.go | 1 + store/tikv/rawkv.go | 1 + store/tikv/region_cache.go | 158 ++++++++++++++++++++++++++---- store/tikv/region_cache_test.go | 3 + store/tikv/region_request.go | 2 +- store/tikv/region_request_test.go | 4 + 7 files changed, 150 insertions(+), 22 deletions(-) diff --git a/store/tikv/coprocessor_test.go b/store/tikv/coprocessor_test.go index 51b22898c9566..6daec69cf5c0c 100644 --- a/store/tikv/coprocessor_test.go +++ b/store/tikv/coprocessor_test.go @@ -34,6 +34,7 @@ func (s *testCoprocessorSuite) TestBuildTasks(c *C) { _, regionIDs, _ := mocktikv.BootstrapWithMultiRegions(cluster, []byte("g"), []byte("n"), []byte("t")) pdCli := &codecPDClient{mocktikv.NewPDClient(cluster)} cache := NewRegionCache(pdCli) + defer cache.Close() bo := NewBackoffer(context.Background(), 3000) @@ -96,6 +97,7 @@ func (s *testCoprocessorSuite) TestSplitRegionRanges(c *C) { mocktikv.BootstrapWithMultiRegions(cluster, []byte("g"), []byte("n"), []byte("t")) pdCli := &codecPDClient{mocktikv.NewPDClient(cluster)} cache := NewRegionCache(pdCli) + defer cache.Close() bo := NewBackoffer(context.Background(), 3000) @@ -148,6 +150,7 @@ func (s *testCoprocessorSuite) TestRebuild(c *C) { storeID, regionIDs, peerIDs := mocktikv.BootstrapWithMultiRegions(cluster, []byte("m")) pdCli := &codecPDClient{mocktikv.NewPDClient(cluster)} cache := NewRegionCache(pdCli) + defer cache.Close() bo := NewBackoffer(context.Background(), 3000) tasks, err := buildCopTasks(bo, cache, buildCopRanges("a", "z"), false, false) diff --git a/store/tikv/kv.go b/store/tikv/kv.go index 5d278f7e686bc..245af37af1c5b 100644 --- a/store/tikv/kv.go +++ b/store/tikv/kv.go @@ -296,6 +296,7 @@ func (s *tikvStore) Close() error { if s.txnLatches != nil { s.txnLatches.Close() } + s.regionCache.Close() return nil } diff --git a/store/tikv/rawkv.go b/store/tikv/rawkv.go index 692cf77807d10..3286f02c8891d 100644 --- a/store/tikv/rawkv.go +++ b/store/tikv/rawkv.go @@ -83,6 +83,7 @@ func NewRawKVClient(pdAddrs []string, security config.Security) (*RawKVClient, e // Close closes the client. func (c *RawKVClient) Close() error { c.pdClient.Close() + c.regionCache.Close() return c.rpcClient.Close() } diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index a96931bd8b7e4..cdde29aa2760d 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -98,6 +98,7 @@ type RegionCache struct { sync.RWMutex stores map[uint64]*Store } + closeCh chan struct{} } // NewRegionCache creates a RegionCache. @@ -108,9 +109,59 @@ func NewRegionCache(pdClient pd.Client) *RegionCache { c.mu.regions = make(map[RegionVerID]*Region) c.mu.sorted = btree.New(btreeDegree) c.storeMu.stores = make(map[uint64]*Store) + c.closeCh = make(chan struct{}) + go c.asyncCheckAndResolveLoop() return c } +// Close releases region cache's resource. +func (c *RegionCache) Close() { + close(c.closeCh) +} + +const checkStoreInterval = 1 * time.Second + +// asyncCheckAndResolveLoop with +func (c *RegionCache) asyncCheckAndResolveLoop() { + tick := time.NewTicker(checkStoreInterval) + defer tick.Stop() + var needCheckStores []*Store + for { + select { + case <-tick.C: + c.checkAndResolve(&needCheckStores) + case <-c.closeCh: + return + } + } +} + +// checkAndResolve checks and resolve addr of failed stores. +// this method isn't thread-safe and only be used by one goroutine. +func (c *RegionCache) checkAndResolve(needCheckStores *[]*Store) { + defer func() { + r := recover() + if r != nil { + logutil.Logger(context.Background()).Error("panic in the checkAndResolve goroutine", + zap.Reflect("r", r), + zap.Stack("stack trace")) + } + }() + + *needCheckStores = (*needCheckStores)[0:] + c.storeMu.RLock() + for i := range c.storeMu.stores { + if c.storeMu.stores[i].needCheck() { + *needCheckStores = append(*needCheckStores, c.storeMu.stores[i]) + } + } + c.storeMu.RUnlock() + + for _, store := range *needCheckStores { + store.reResolve() + } +} + // RPCContext contains data that is needed to send RPC to a region. type RPCContext struct { Region RegionVerID @@ -151,7 +202,7 @@ func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, return nil, nil } - addr, err := store.resolveAddr(bo, ts) + addr, err := store.getAddr(bo) if err != nil { return nil, err } @@ -736,8 +787,8 @@ type Store struct { state uint64 // unsafe store storeState resolve struct { - sync.Mutex - fn resolveFunc // func to get store address from PD + sync.Mutex // protect pd from concurrent init requests + fn resolveFunc // func to get store address from PD } } @@ -745,14 +796,23 @@ type Store struct { type storeState struct { lastFailedTime uint32 failedAttempt uint16 - resolved bool + resolveState resolveState } -// resolveAddr resolves the address of store. +type resolveState uint8 + +const ( + unresolved resolveState = iota + resolved + needCheck +) + +// getAddr resolves the address of store. // following up resolve request will reuse previous result until // store become unreachable and after reResolveUnreachableStoreIntervalSec -func (s *Store) resolveAddr(bo *Backoffer, ts int64) (addr string, err error) { - if !s.needResolve() { +func (s *Store) getAddr(bo *Backoffer) (addr string, err error) { + // always use current addr event if it maybe staled. + if !s.needInitResolve() { v := s.addr.Load() if v == nil { addr = "" @@ -761,8 +821,15 @@ func (s *Store) resolveAddr(bo *Backoffer, ts int64) (addr string, err error) { addr = v.(string) return } + // only resolve store addr from init status at first time. + addr, err = s.initResolve(bo) + return +} + +// initResolve resolves addr for store that never resolved. +func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { s.resolve.Lock() - if !s.needResolve() { + if !s.needInitResolve() { s.resolve.Unlock() v := s.addr.Load() if v == nil { @@ -773,7 +840,6 @@ func (s *Store) resolveAddr(bo *Backoffer, ts int64) (addr string, err error) { return } var store *metapb.Store - // re-resolve if unreachable and long time from last load. for { store, err = s.resolve.fn(bo.ctx, s.peer.StoreId) if err != nil { @@ -800,12 +866,39 @@ func (s *Store) resolveAddr(bo *Backoffer, ts int64) (addr string, err error) { } addr = store.GetAddress() s.addr.Store(addr) - s.markResolved(true) + s.markResolved() s.resolve.Unlock() return } } +// reResolve try to resolve addr for store that need check. +func (s *Store) reResolve() { + var addr string + store, err := s.resolve.fn(context.Background(), s.peer.StoreId) + if err != nil { + tikvRegionCacheCounterWithGetStoreError.Inc() + } else { + tikvRegionCacheCounterWithGetStoreOK.Inc() + } + if err != nil { + // TODO: more refine PD error status handle. + if errors.Cause(err) == context.Canceled { + return + } + logutil.Logger(context.Background()).Error("loadStore from PD failed", zap.Uint64("id", s.peer.StoreId), zap.Error(err)) + // we cannot do backoff in reResolve loop but try check other store and wait tick. + return + } + if store == nil { + return + } + addr = store.GetAddress() + s.addr.Store(addr) + s.markResolved() + return +} + // maxExponentAttempt before this blackout time is exponent increment. const maxExponentAttempt = 10 @@ -825,11 +918,18 @@ func (s *Store) Available(ts int64) bool { return blackoutDeadline < ts } -// needResolve checks whether store need resolve. -func (s *Store) needResolve() bool { +// needInitResolve checks whether store need to do init block resolve. +func (s *Store) needInitResolve() bool { var state storeState *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) - return state.resolved == false + return state.resolveState == unresolved +} + +// needCheck checks whether store need to do async resolve check. +func (s *Store) needCheck() bool { + var state storeState + *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) + return state.resolveState == needCheck } // markAccess marks the processing result. @@ -847,6 +947,9 @@ retry: state.lastFailedTime = uint32(time.Now().Unix()) } state.failedAttempt = state.failedAttempt + 1 + if state.resolveState == resolved { + state.resolveState = needCheck + } } else { state.lastFailedTime = 0 state.failedAttempt = 0 @@ -857,19 +960,32 @@ retry: } } -// markResolved marks the resolved status. -// `resolved = true` will let following requests use resolved addr -// `resolved = false` will let next request resolve store address -func (s *Store) markResolved(resolved bool) { +// markResolved marks store has be resolved. +func (s *Store) markResolved() { retry: oldValue := atomic.LoadUint64(&s.state) var state storeState *(*uint64)(unsafe.Pointer(&state)) = oldValue - if resolved { - state.resolved = true - } else { - state.resolved = false + if state.resolveState == resolved { + return + } + state.resolveState = resolved + newValue := *(*uint64)(unsafe.Pointer(&state)) + if !atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) { + goto retry + } +} + +// markNeedCheck marks resolved store to be async resolve to check store addr change. +func (s *Store) markNeedCheck() { +retry: + oldValue := atomic.LoadUint64(&s.state) + var state storeState + *(*uint64)(unsafe.Pointer(&state)) = oldValue + if state.resolveState != resolved { + return } + state.resolveState = needCheck newValue := *(*uint64)(unsafe.Pointer(&state)) if !atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) { goto retry diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 5275ab14d6e3e..5469d0c339665 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -351,6 +351,7 @@ func (s *testRegionCacheSuite) TestRegionEpochAheadOfTiKV(c *C) { // Create a separated region cache to do this test. pdCli := &codecPDClient{mocktikv.NewPDClient(s.cluster)} cache := NewRegionCache(pdCli) + defer cache.Close() region := createSampleRegion([]byte("k1"), []byte("k2")) region.meta.Id = 1 @@ -375,6 +376,7 @@ func (s *testRegionCacheSuite) TestDropStoreOnSendRequestFail(c *C) { cluster := createClusterWithStoresAndRegions(regionCnt, storeCount) cache := NewRegionCache(mocktikv.NewPDClient(cluster)) + defer cache.Close() loadRegionsToCache(cache, regionCnt) c.Assert(workableRegions(cache, cache.mu.regions, ts), Equals, regionCnt) @@ -514,6 +516,7 @@ func BenchmarkOnRequestFail(b *testing.B) { regionCnt, storeCount := 998, 3 cluster := createClusterWithStoresAndRegions(regionCnt, storeCount) cache := NewRegionCache(mocktikv.NewPDClient(cluster)) + defer cache.Close() loadRegionsToCache(cache, regionCnt) bo := NewBackoffer(context.Background(), 1) loc, err := cache.LocateKey(bo, []byte{}) diff --git a/store/tikv/region_request.go b/store/tikv/region_request.go index 51a4f2c5613c3..927cd20226fa1 100644 --- a/store/tikv/region_request.go +++ b/store/tikv/region_request.go @@ -226,7 +226,7 @@ func (s *RegionRequestSender) onRegionError(bo *Backoffer, ctx *RPCContext, regi logutil.Logger(context.Background()).Warn("tikv reports `StoreNotMatch` retry later", zap.Stringer("storeNotMatch", storeNotMatch), zap.Stringer("ctx", ctx)) - ctx.Store.markResolved(false) + ctx.Store.markNeedCheck() return true, nil } diff --git a/store/tikv/region_request_test.go b/store/tikv/region_request_test.go index a6d1a6bc2c72a..949af3f9eef7f 100644 --- a/store/tikv/region_request_test.go +++ b/store/tikv/region_request_test.go @@ -54,6 +54,10 @@ func (s *testRegionRequestSuite) SetUpTest(c *C) { s.regionRequestSender = NewRegionRequestSender(s.cache, client) } +func (s *testRegionRequestSuite) TearDownTest(c *C) { + s.cache.Close() +} + func (s *testRegionRequestSuite) TestOnSendFailedWithStoreRestart(c *C) { req := &tikvrpc.Request{ Type: tikvrpc.CmdRawPut, From da530fb46b7cfdc7a52b4fb30e7ab497ab16b7c2 Mon Sep 17 00:00:00 2001 From: lysu Date: Tue, 7 May 2019 17:31:56 +0800 Subject: [PATCH 07/27] ac: fix store/peer relationship bug --- store/tikv/region_cache.go | 72 ++++++++++++++++++--------------- store/tikv/region_cache_test.go | 11 ++--- store/tikv/region_request.go | 4 +- 3 files changed, 47 insertions(+), 40 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index cdde29aa2760d..7cb1e47b728fc 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -54,7 +54,7 @@ type Region struct { meta *metapb.Region // immutable after fetched from pd lastAccess int64 // last region access time, see checkRegionCacheTTL stores []*Store // stores in this region - workStoreIdx int32 // point to current work + workStoreIdx int32 // point to current work peer in meta.Peers and work store in stores(same idx) } func (r *Region) initStores(c *RegionCache) { @@ -63,7 +63,7 @@ func (r *Region) initStores(c *RegionCache) { store, exists := c.storeMu.stores[p.StoreId] c.storeMu.RUnlock() if !exists { - store = c.getStoreByStoreID(p) + store = c.getStoreByStoreID(p.StoreId) } r.stores = append(r.stores, store) } @@ -150,9 +150,9 @@ func (c *RegionCache) checkAndResolve(needCheckStores *[]*Store) { *needCheckStores = (*needCheckStores)[0:] c.storeMu.RLock() - for i := range c.storeMu.stores { - if c.storeMu.stores[i].needCheck() { - *needCheckStores = append(*needCheckStores, c.storeMu.stores[i]) + for _, store := range c.storeMu.stores { + if store.needCheck() { + *needCheckStores = append(*needCheckStores, store) } } c.storeMu.RUnlock() @@ -166,6 +166,7 @@ func (c *RegionCache) checkAndResolve(needCheckStores *[]*Store) { type RPCContext struct { Region RegionVerID Meta *metapb.Region + Peer *metapb.Peer Store *Store Addr string } @@ -173,14 +174,14 @@ type RPCContext struct { // GetStoreID returns StoreID. func (c *RPCContext) GetStoreID() uint64 { if c.Store != nil { - return c.Store.peer.StoreId + return c.Store.storeID } return 0 } func (c *RPCContext) String() string { return fmt.Sprintf("region ID: %d, meta: %s, peer: %s, addr: %s", - c.Region.GetID(), c.Meta, c.Store.peer, c.Addr) + c.Region.GetID(), c.Meta, c.Peer, c.Addr) } // GetRPCContext returns RPCContext for a region. If it returns nil, the region @@ -197,7 +198,7 @@ func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, return nil, nil } - store := c.ensureRegionWorkStore(cachedRegion, ts) + store, peer := c.ensureRegionWorkStore(cachedRegion, ts) if store == nil { return nil, nil } @@ -215,6 +216,7 @@ func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, return &RPCContext{ Region: id, Meta: cachedRegion.meta, + Peer: peer, Store: store, Addr: addr, }, nil @@ -418,7 +420,7 @@ func (c *RegionCache) searchCachedRegion(key []byte, isEndKey bool) *Region { }) c.mu.RUnlock() if r != nil && (!isEndKey && r.Contains(key) || isEndKey && r.ContainsByEnd(key)) { - workStore := c.ensureRegionWorkStore(r, ts) + workStore, _ := c.ensureRegionWorkStore(r, ts) if workStore == nil { return nil } @@ -540,14 +542,15 @@ func (c *RegionCache) getCachedRegionWithRLock(regionID RegionVerID) (r *Region) } // ensureRegionWorkStore ensures region have workable store and return it. -func (c *RegionCache) ensureRegionWorkStore(region *Region, ts int64) (workStore *Store) { +func (c *RegionCache) ensureRegionWorkStore(region *Region, ts int64) (workStore *Store, workPeer *metapb.Peer) { if len(region.stores) == 0 { return } retry: - cachedStore, cachedIdx := region.WorkStore() + cachedStore, cachedPeer, cachedIdx := region.WorkStorePeer() if cachedStore != nil && cachedStore.Available(ts) { workStore = cachedStore + workPeer = cachedPeer return } @@ -568,23 +571,19 @@ retry: goto retry } workStore = region.stores[newIdx] + workPeer = region.meta.Peers[newIdx] return } -func (c *RegionCache) getStoreByStoreID(peer *metapb.Peer) (store *Store) { - var ( - ok bool - storeID = peer.GetStoreId() - ) +func (c *RegionCache) getStoreByStoreID(storeID uint64) (store *Store) { + var ok bool c.storeMu.Lock() store, ok = c.storeMu.stores[storeID] if ok { c.storeMu.Unlock() return } - store = &Store{ - peer: peer, - } + store = &Store{storeID: storeID} store.resolve.fn = c.pdClient.GetStore c.storeMu.stores[storeID] = store c.storeMu.Unlock() @@ -593,7 +592,7 @@ func (c *RegionCache) getStoreByStoreID(peer *metapb.Peer) (store *Store) { // OnSendRequestFail is used for clearing cache when a tikv server does not respond. func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { - failedStoreID := ctx.Store.peer.StoreId + failedStoreID := ctx.Store.storeID c.storeMu.RLock() store, exists := c.storeMu.stores[failedStoreID] @@ -613,7 +612,7 @@ func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { if lastAccess == invalidatedLastAccessTime { return } - store = c.ensureRegionWorkStore(r, time.Now().Unix()) + store, _ = c.ensureRegionWorkStore(r, time.Now().Unix()) if store == nil { r.invalidate() } @@ -654,7 +653,7 @@ func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, curr stores: make([]*Store, 0, len(meta.Peers)), } region.initStores(c) - c.switchWorkStore(region, ctx.Store.peer.GetStoreId()) + c.switchWorkStore(region, ctx.Store.storeID) c.insertRegionToCache(region) } return nil @@ -693,10 +692,18 @@ func (r *Region) GetID() uint64 { return r.meta.GetId() } -// WorkStore returns current work store. -func (r *Region) WorkStore() (store *Store, idx int) { +// WorkStorePeer returns current work store with work peer. +func (r *Region) WorkStorePeer() (store *Store, peer *metapb.Peer, idx int) { idx = int(atomic.LoadInt32(&r.workStoreIdx)) store = r.stores[idx] + peer = r.meta.Peers[idx] + return +} + +// workPeer returns current work peer. +func (r *Region) workPeer() (peer *metapb.Peer) { + idx := int(atomic.LoadInt32(&r.workStoreIdx)) + peer = r.meta.Peers[idx] return } @@ -733,11 +740,10 @@ func (r *Region) EndKey() []byte { // GetContext constructs kvprotopb.Context from region info. func (r *Region) GetContext() *kvrpcpb.Context { - store, _ := r.WorkStore() return &kvrpcpb.Context{ RegionId: r.meta.Id, RegionEpoch: r.meta.RegionEpoch, - Peer: store.peer, + Peer: r.workPeer(), } } @@ -782,9 +788,9 @@ type resolveFunc func(ctx context.Context, id uint64) (*metapb.Store, error) // Store contains a kv process's address. type Store struct { - addr atomic.Value // loaded store address(*string) - peer *metapb.Peer // store's peer - state uint64 // unsafe store storeState + addr atomic.Value // loaded store address(*string) + storeID uint64 // store's id + state uint64 // unsafe store storeState resolve struct { sync.Mutex // protect pd from concurrent init requests @@ -841,7 +847,7 @@ func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { } var store *metapb.Store for { - store, err = s.resolve.fn(bo.ctx, s.peer.StoreId) + store, err = s.resolve.fn(bo.ctx, s.storeID) if err != nil { tikvRegionCacheCounterWithGetStoreError.Inc() } else { @@ -853,7 +859,7 @@ func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { s.resolve.Unlock() return } - err = errors.Errorf("loadStore from PD failed, id: %d, err: %v", s.peer.StoreId, err) + err = errors.Errorf("loadStore from PD failed, id: %d, err: %v", s.storeID, err) if err = bo.Backoff(BoPDRPC, err); err != nil { s.resolve.Unlock() return @@ -875,7 +881,7 @@ func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { // reResolve try to resolve addr for store that need check. func (s *Store) reResolve() { var addr string - store, err := s.resolve.fn(context.Background(), s.peer.StoreId) + store, err := s.resolve.fn(context.Background(), s.storeID) if err != nil { tikvRegionCacheCounterWithGetStoreError.Inc() } else { @@ -886,7 +892,7 @@ func (s *Store) reResolve() { if errors.Cause(err) == context.Canceled { return } - logutil.Logger(context.Background()).Error("loadStore from PD failed", zap.Uint64("id", s.peer.StoreId), zap.Error(err)) + logutil.Logger(context.Background()).Error("loadStore from PD failed", zap.Uint64("id", s.storeID), zap.Error(err)) // we cannot do backoff in reResolve loop but try check other store and wait tick. return } diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 5469d0c339665..5e62290258de2 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -18,11 +18,11 @@ import ( "errors" "fmt" "github.com/google/btree" - "github.com/pingcap/kvproto/pkg/metapb" "testing" "time" . "github.com/pingcap/check" + "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/tidb/store/mockstore/mocktikv" ) @@ -63,7 +63,7 @@ func (s *testRegionCacheSuite) checkCache(c *C, len int) { c.Assert(workableRegionsInBtree(s.cache, s.cache.mu.sorted, ts), Equals, len) for _, r := range s.cache.mu.regions { if r.checkRegionCacheTTL(ts) { - if store := s.cache.ensureRegionWorkStore(r, ts); store != nil { + if store, _ := s.cache.ensureRegionWorkStore(r, ts); store != nil { c.Assert(r, DeepEquals, s.cache.searchCachedRegion(r.StartKey(), false)) } } @@ -75,7 +75,7 @@ func workableRegions(cache *RegionCache, regions map[RegionVerID]*Region, ts int if !region.checkRegionCacheTTL(ts) { continue } - store := cache.ensureRegionWorkStore(region, ts) + store, _ := cache.ensureRegionWorkStore(region, ts) if store != nil { len++ } @@ -89,7 +89,7 @@ func workableRegionsInBtree(cache *RegionCache, t *btree.BTree, ts int64) (len i if !r.checkRegionCacheTTL(ts) { return true } - store := cache.ensureRegionWorkStore(r, ts) + store, _ := cache.ensureRegionWorkStore(r, ts) if store != nil { len++ } @@ -525,12 +525,13 @@ func BenchmarkOnRequestFail(b *testing.B) { } region := cache.getRegionByIDFromCache(loc.Region.id) b.ResetTimer() - store, _ := region.WorkStore() + store, peer, _ := region.WorkStorePeer() b.RunParallel(func(pb *testing.PB) { for pb.Next() { rpcCtx := &RPCContext{ Region: loc.Region, Meta: region.meta, + Peer: peer, Store: store, } cache.OnSendRequestFail(rpcCtx, nil) diff --git a/store/tikv/region_request.go b/store/tikv/region_request.go index 927cd20226fa1..17ad3e85ee0c3 100644 --- a/store/tikv/region_request.go +++ b/store/tikv/region_request.go @@ -132,7 +132,7 @@ func (s *RegionRequestSender) SendReqCtx(bo *Backoffer, req *tikvrpc.Request, re } func (s *RegionRequestSender) sendReqToRegion(bo *Backoffer, ctx *RPCContext, req *tikvrpc.Request, timeout time.Duration) (resp *tikvrpc.Response, retry bool, err error) { - if e := tikvrpc.SetContext(req, ctx.Meta, ctx.Store.peer); e != nil { + if e := tikvrpc.SetContext(req, ctx.Meta, ctx.Peer); e != nil { return nil, false, errors.Trace(e) } resp, err = s.client.SendRequest(bo.ctx, ctx.Addr, req, timeout) @@ -148,7 +148,7 @@ func (s *RegionRequestSender) sendReqToRegion(bo *Backoffer, ctx *RPCContext, re } func (s *RegionRequestSender) onSendSuccess(ctx *RPCContext) { - store := s.regionCache.getStoreByStoreID(ctx.Store.peer) + store := s.regionCache.getStoreByStoreID(ctx.Store.storeID) store.markAccess(true) } From 1b915d12bc6a4be58bea46c2f5e7ef5b4a64187a Mon Sep 17 00:00:00 2001 From: lysu Date: Tue, 7 May 2019 17:35:38 +0800 Subject: [PATCH 08/27] remove unused code. --- store/tikv/region_cache.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 7cb1e47b728fc..3db4d4bcc3d3b 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -25,7 +25,6 @@ import ( "github.com/google/btree" "github.com/grpc-ecosystem/go-grpc-middleware/util/backoffutils" "github.com/pingcap/errors" - "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/pd/client" "github.com/pingcap/tidb/metrics" @@ -700,13 +699,6 @@ func (r *Region) WorkStorePeer() (store *Store, peer *metapb.Peer, idx int) { return } -// workPeer returns current work peer. -func (r *Region) workPeer() (peer *metapb.Peer) { - idx := int(atomic.LoadInt32(&r.workStoreIdx)) - peer = r.meta.Peers[idx] - return -} - // RegionVerID is a unique ID that can identify a Region at a specific version. type RegionVerID struct { id uint64 @@ -738,15 +730,6 @@ func (r *Region) EndKey() []byte { return r.meta.EndKey } -// GetContext constructs kvprotopb.Context from region info. -func (r *Region) GetContext() *kvrpcpb.Context { - return &kvrpcpb.Context{ - RegionId: r.meta.Id, - RegionEpoch: r.meta.RegionEpoch, - Peer: r.workPeer(), - } -} - // switchWorkStore switches current store to the one on specific store. It returns // false if no peer matches the storeID. func (c *RegionCache) switchWorkStore(r *Region, storeID uint64) (foundLeader bool) { From 966f35078c218d3f6a6b120d9914d8595633921c Mon Sep 17 00:00:00 2001 From: lysu Date: Tue, 7 May 2019 22:24:09 +0800 Subject: [PATCH 09/27] refine rr balance logic and refine test case --- store/tikv/region_cache.go | 84 ++++++++++++++++++------- store/tikv/region_cache_test.go | 106 ++++++++++++++++++++------------ 2 files changed, 129 insertions(+), 61 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 3db4d4bcc3d3b..1d1a90feb55a7 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -197,8 +197,10 @@ func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, return nil, nil } - store, peer := c.ensureRegionWorkStore(cachedRegion, ts) + store, peer := c.routeStoreInRegion(cachedRegion, ts) if store == nil { + // Store not found, region must be out of date. + cachedRegion.invalidate() return nil, nil } @@ -207,7 +209,7 @@ func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, return nil, err } if len(addr) == 0 { - // Store not found, region must be out of date. TODO: check me? + // Store not found, region must be out of date. cachedRegion.invalidate() return nil, nil } @@ -419,8 +421,7 @@ func (c *RegionCache) searchCachedRegion(key []byte, isEndKey bool) *Region { }) c.mu.RUnlock() if r != nil && (!isEndKey && r.Contains(key) || isEndKey && r.ContainsByEnd(key)) { - workStore, _ := c.ensureRegionWorkStore(r, ts) - if workStore == nil { + if !c.hasAvailableStore(r, ts) { return nil } return r @@ -540,32 +541,38 @@ func (c *RegionCache) getCachedRegionWithRLock(regionID RegionVerID) (r *Region) return } -// ensureRegionWorkStore ensures region have workable store and return it. -func (c *RegionCache) ensureRegionWorkStore(region *Region, ts int64) (workStore *Store, workPeer *metapb.Peer) { +// routeStoreInRegion ensures region have workable store and return it. +func (c *RegionCache) routeStoreInRegion(region *Region, ts int64) (workStore *Store, workPeer *metapb.Peer) { if len(region.stores) == 0 { return } retry: cachedStore, cachedPeer, cachedIdx := region.WorkStorePeer() - if cachedStore != nil && cachedStore.Available(ts) { + if cachedStore != nil && cachedStore.Operational() { workStore = cachedStore workPeer = cachedPeer return } - newIdx := cachedIdx - for i, store := range region.stores { - if i == cachedIdx { - continue - } + newIdx := -1 + storeNum := len(region.stores) + i := (cachedIdx + 1) % storeNum + start := i + for { + store := region.stores[i] if store.Available(ts) { newIdx = i break } + i = (i + 1) % storeNum + if i == start { + break + } } - if newIdx == cachedIdx { + if newIdx < 0 { return } + //fmt.Println("change " , cachedIdx, "to", newIdx, string(debug.Stack())) if !atomic.CompareAndSwapInt32(®ion.workStoreIdx, int32(cachedIdx), int32(newIdx)) { goto retry } @@ -574,6 +581,20 @@ retry: return } +// hasAvailableStore checks whether region has available store. +// different to `routeStoreInRegion` just check and never change work store or peer. +func (c *RegionCache) hasAvailableStore(region *Region, ts int64) bool { + if len(region.stores) == 0 { + return false + } + for _, store := range region.stores { + if store.Available(ts) { + return true + } + } + return false +} + func (c *RegionCache) getStoreByStoreID(storeID uint64) (store *Store) { var ok bool c.storeMu.Lock() @@ -611,8 +632,7 @@ func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { if lastAccess == invalidatedLastAccessTime { return } - store, _ = c.ensureRegionWorkStore(r, time.Now().Unix()) - if store == nil { + if !c.hasAvailableStore(r, time.Now().Unix()) { r.invalidate() } } @@ -888,8 +908,23 @@ func (s *Store) reResolve() { return } -// maxExponentAttempt before this blackout time is exponent increment. -const maxExponentAttempt = 10 +const ( + // maxExponentAttempt before this blackout time is exponent increment. + maxExponentAttempt = 10 + // startBlackoutAfterAttempt after continue fail attempts start blackout store. + startBlackoutAfterAttempt = 20 +) + +// Operational returns whether store is operational and no need retry other node. +func (s *Store) Operational() bool { + var state storeState + *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) + if state.failedAttempt == 0 || state.lastFailedTime == 0 { + // return quickly if it's continue success. + return true + } + return false +} // Available returns whether store be available for current. func (s *Store) Available(ts int64) bool { @@ -899,12 +934,17 @@ func (s *Store) Available(ts int64) bool { // return quickly if it's continue success. return true } + // first `startBlackoutAfterAttempt` can retry immediately. + if state.failedAttempt < startBlackoutAfterAttempt { + return true + } + // continue fail over than `startBlackoutAfterAttempt` start blackout store logic. // check blackout time window to determine store's reachable. - if state.failedAttempt > maxExponentAttempt { - state.failedAttempt = maxExponentAttempt + if state.failedAttempt > startBlackoutAfterAttempt+maxExponentAttempt { + state.failedAttempt = startBlackoutAfterAttempt + maxExponentAttempt } - blackoutDeadline := int64(state.lastFailedTime) + int64(backoffutils.ExponentBase2(uint(state.failedAttempt))) - return blackoutDeadline < ts + blackoutDeadline := int64(state.lastFailedTime) + 1*int64(backoffutils.ExponentBase2(uint(state.failedAttempt-startBlackoutAfterAttempt+1))) + return blackoutDeadline <= ts } // needInitResolve checks whether store need to do init block resolve. @@ -927,7 +967,7 @@ retry: oldValue := atomic.LoadUint64(&s.state) var state storeState *(*uint64)(unsafe.Pointer(&state)) = oldValue - if (state.failedAttempt == 0 && success) || (!success && state.failedAttempt >= maxExponentAttempt) { + if (state.failedAttempt == 0 && success) || (!success && state.failedAttempt >= (startBlackoutAfterAttempt+maxExponentAttempt)) { // return quickly if continue success, and no more mark when attempt meet max bound. return } diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 5e62290258de2..baa90dc2132aa 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -63,7 +63,7 @@ func (s *testRegionCacheSuite) checkCache(c *C, len int) { c.Assert(workableRegionsInBtree(s.cache, s.cache.mu.sorted, ts), Equals, len) for _, r := range s.cache.mu.regions { if r.checkRegionCacheTTL(ts) { - if store, _ := s.cache.ensureRegionWorkStore(r, ts); store != nil { + if store, _ := s.cache.routeStoreInRegion(r, ts); store != nil { c.Assert(r, DeepEquals, s.cache.searchCachedRegion(r.StartKey(), false)) } } @@ -75,7 +75,7 @@ func workableRegions(cache *RegionCache, regions map[RegionVerID]*Region, ts int if !region.checkRegionCacheTTL(ts) { continue } - store, _ := cache.ensureRegionWorkStore(region, ts) + store, _ := cache.routeStoreInRegion(region, ts) if store != nil { len++ } @@ -89,7 +89,7 @@ func workableRegionsInBtree(cache *RegionCache, t *btree.BTree, ts int64) (len i if !r.checkRegionCacheTTL(ts) { return true } - store, _ := cache.ensureRegionWorkStore(r, ts) + store, _ := cache.routeStoreInRegion(r, ts) if store != nil { len++ } @@ -114,6 +114,7 @@ func (s *testRegionCacheSuite) getRegion(c *C, key []byte) *Region { c.Assert(r, NotNil) return r } + func (s *testRegionCacheSuite) getRegionWithEndKey(c *C, key []byte) *Region { _, err := s.cache.LocateEndKey(s.bo, key) c.Assert(err, IsNil) @@ -302,22 +303,45 @@ func (s *testRegionCacheSuite) TestReconnect(c *C) { s.checkCache(c, 1) } -func (s *testRegionCacheSuite) TestRequestFail(c *C) { +func (s *testRegionCacheSuite) TestSendFailBlackout(c *C) { ts := time.Now().Unix() region := s.getRegion(c, []byte("a")) - ctx, _ := s.cache.GetRPCContext(s.bo, region.VerID()) - s.cache.OnSendRequestFail(ctx, errors.New("test error")) - region = s.getRegion(c, []byte("a")) - c.Assert(s.cache.mu.regions, HasLen, 1) - ctx, _ = s.cache.GetRPCContext(s.bo, region.VerID()) - s.cache.OnSendRequestFail(ctx, errors.New("test error")) + + // init with 1 region 2 stores + c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 1) + c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 2) + + // for each stores has 20 chance to retry, and still have chance to access store for 21 + for i := 0; i < 38; i++ { + ctx, _ := s.cache.GetRPCContext(s.bo, region.VerID()) + if ctx == nil { + fmt.Println() + } + s.cache.OnSendRequestFail(ctx, errors.New("test error")) + + } + ts = time.Now().Unix() + c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 1) + c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 2) + + // 21 fail attempts will start blackout store in 1 second + for i := 0; i < 2; i++ { + // first fail request make 1st store' failAttempt + 1 + ctx, _ := s.cache.GetRPCContext(s.bo, region.VerID()) + s.cache.OnSendRequestFail(ctx, errors.New("test error")) + } c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 0) - time.Sleep(2 * time.Second) + c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 0) + + // after 1 second blackout, 2 store can be access again. + time.Sleep(1 * time.Second) + ts = time.Now().Unix() s.getRegion(c, []byte("a")) - c.Assert(workableRegions(s.cache, s.cache.mu.regions, time.Now().Unix()), Equals, 1) + c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 1) + c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 2) } -func (s *testRegionCacheSuite) TestRequestFail2(c *C) { +func (s *testRegionCacheSuite) TestSendFailBlackTwoRegion(c *C) { ts := time.Now().Unix() // key range: ['' - 'm' - 'z'] region2 := s.cluster.AllocID() @@ -332,19 +356,31 @@ func (s *testRegionCacheSuite) TestRequestFail2(c *C) { c.Assert(err, IsNil) c.Assert(loc2.Region.id, Equals, region2) - // Request should fail on region1. - ctx, _ := s.cache.GetRPCContext(s.bo, loc1.Region) c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 2) s.checkCache(c, 2) - s.cache.OnSendRequestFail(ctx, errors.New("test error")) - // Both region2 and store should be dropped from cache. - c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 1) - ctx2, _ := s.cache.GetRPCContext(s.bo, loc2.Region) - s.cache.OnSendRequestFail(ctx2, errors.New("test error")) - r := s.cache.searchCachedRegion([]byte("x"), true) - c.Assert(r, IsNil) + + // send request fail in 2 regions backed by same 2 stores. + for i := 0; i < 20; i++ { + ctx, _ := s.cache.GetRPCContext(s.bo, loc1.Region) + s.cache.OnSendRequestFail(ctx, errors.New("test error")) + } + for i := 0; i < 20; i++ { + ctx, _ := s.cache.GetRPCContext(s.bo, loc2.Region) + s.cache.OnSendRequestFail(ctx, errors.New("test error")) + } + + // both 2 region are invalidate and both 2 store are available. c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 0) - s.checkCache(c, 0) + c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 0) + + // after sleep 1 second, region recover + time.Sleep(1 * time.Second) + ts = time.Now().Unix() + c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 2) + s.getRegion(c, []byte("a")) + c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 1) + s.getRegion(c, []byte("x")) + c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 2) } func (s *testRegionCacheSuite) TestRegionEpochAheadOfTiKV(c *C) { @@ -372,7 +408,7 @@ func (s *testRegionCacheSuite) TestRegionEpochAheadOfTiKV(c *C) { func (s *testRegionCacheSuite) TestDropStoreOnSendRequestFail(c *C) { ts := time.Now().Unix() - regionCnt, storeCount := 999, 3 + regionCnt, storeCount := 8, 3 cluster := createClusterWithStoresAndRegions(regionCnt, storeCount) cache := NewRegionCache(mocktikv.NewPDClient(cluster)) @@ -384,23 +420,15 @@ func (s *testRegionCacheSuite) TestDropStoreOnSendRequestFail(c *C) { loc, err := cache.LocateKey(bo, []byte{}) c.Assert(err, IsNil) - // First two drop attempts just switch peer. - for i := 0; i < storeCount-1; i++ { - rpcCtx, err := cache.GetRPCContext(bo, loc.Region) - c.Assert(err, IsNil) - cache.OnSendRequestFail(rpcCtx, errors.New("test error")) - c.Assert(workableRegions(cache, cache.mu.regions, ts), Equals, regionCnt+1) + // fail on one region make all stores be unavailable. + for j := 0; j < 20; j++ { + for i := 0; i < storeCount; i++ { + rpcCtx, err := cache.GetRPCContext(bo, loc.Region) + c.Assert(err, IsNil) + cache.OnSendRequestFail(rpcCtx, errors.New("test error")) + } } - - // Drop the regions on one store, all region will be drop because they are in three stores. - // and fail 3 times hit at each stores. - rpcCtx, err := cache.GetRPCContext(bo, loc.Region) - c.Assert(err, IsNil) - cache.OnSendRequestFail(rpcCtx, errors.New("test error")) c.Assert(workableRegions(cache, cache.mu.regions, ts), Equals, 0) - - loadRegionsToCache(cache, regionCnt) - c.Assert(len(cache.mu.regions), Equals, regionCnt+1) } const regionSplitKeyFormat = "t%08d" From d5c22826784ee984df0fe1c0eff25a9ddb452762 Mon Sep 17 00:00:00 2001 From: lysu Date: Wed, 8 May 2019 18:03:08 +0800 Subject: [PATCH 10/27] refine reload region --- store/tikv/region_cache.go | 70 ++++++++++++++++++++++----------- store/tikv/region_cache_test.go | 2 +- 2 files changed, 48 insertions(+), 24 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 1d1a90feb55a7..8df4a8b76ebae 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -48,12 +48,19 @@ var ( tikvRegionCacheCounterWithGetStoreError = metrics.TiKVRegionCacheCounter.WithLabelValues("get_store", "err") ) +const ( + updated int32 = iota // region is updated and no need to reload. + needSync // need sync new region info. +) + // Region presents kv region type Region struct { meta *metapb.Region // immutable after fetched from pd - lastAccess int64 // last region access time, see checkRegionCacheTTL stores []*Store // stores in this region workStoreIdx int32 // point to current work peer in meta.Peers and work store in stores(same idx) + syncStoreIdx int32 // point to init peer index and need reload if meet syncStoreIdx again + syncFlag int32 // mark region need be sync in next turn + lastAccess int64 // last region access time, see checkRegionCacheTTL } func (r *Region) initStores(c *RegionCache) { @@ -80,10 +87,32 @@ retry: return true } +// invalidate invalidates a region, next time it will got null result. func (r *Region) invalidate() { atomic.StoreInt64(&r.lastAccess, invalidatedLastAccessTime) } +// scheduleReload schedules reload region request in next LocateKey. +func (r *Region) scheduleReload() { +retry: + oldValue := atomic.LoadInt32(&r.syncFlag) + if oldValue != updated { + return + } + if !atomic.CompareAndSwapInt32(&r.syncFlag, oldValue, needSync) { + goto retry + } +} + +// needReload checks whether region need reload. +func (r *Region) needReload() bool { + oldValue := atomic.LoadInt32(&r.syncFlag) + if oldValue == updated { + return false + } + return atomic.CompareAndSwapInt32(&r.syncFlag, oldValue, updated) +} + // RegionCache caches Regions loaded from PD. type RegionCache struct { pdClient pd.Client @@ -239,24 +268,17 @@ func (l *KeyLocation) Contains(key []byte) bool { // LocateKey searches for the region and range that the key is located. func (c *RegionCache) LocateKey(bo *Backoffer, key []byte) (*KeyLocation, error) { r := c.searchCachedRegion(key, false) - if r != nil { - loc := &KeyLocation{ - Region: r.VerID(), - StartKey: r.StartKey(), - EndKey: r.EndKey(), + if r == nil || r.needReload() { + var err error + r, err = c.loadRegion(bo, key, false) + if err != nil { + return nil, errors.Trace(err) } - return loc, nil - } - r, err := c.loadRegion(bo, key, false) - if err != nil { - return nil, errors.Trace(err) + c.mu.Lock() + c.insertRegionToCache(r) + c.mu.Unlock() } - - c.mu.Lock() - c.insertRegionToCache(r) - c.mu.Unlock() - return &KeyLocation{ Region: r.VerID(), StartKey: r.StartKey(), @@ -548,12 +570,14 @@ func (c *RegionCache) routeStoreInRegion(region *Region, ts int64) (workStore *S } retry: cachedStore, cachedPeer, cachedIdx := region.WorkStorePeer() - if cachedStore != nil && cachedStore.Operational() { + // almost time requests be directly routed to stable leader. + if cachedStore != nil && cachedStore.stableLeader() { workStore = cachedStore workPeer = cachedPeer return } + // try round-robin find & switch to other peers when old leader meet error. newIdx := -1 storeNum := len(region.stores) i := (cachedIdx + 1) % storeNum @@ -572,10 +596,12 @@ retry: if newIdx < 0 { return } - //fmt.Println("change " , cachedIdx, "to", newIdx, string(debug.Stack())) if !atomic.CompareAndSwapInt32(®ion.workStoreIdx, int32(cachedIdx), int32(newIdx)) { goto retry } + if int32(newIdx) == atomic.LoadInt32(®ion.syncStoreIdx) { + region.scheduleReload() + } workStore = region.stores[newIdx] workPeer = region.meta.Peers[newIdx] return @@ -632,9 +658,6 @@ func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { if lastAccess == invalidatedLastAccessTime { return } - if !c.hasAvailableStore(r, time.Now().Unix()) { - r.invalidate() - } } // OnRegionEpochNotMatch removes the old region and inserts new regions into the cache. @@ -768,6 +791,7 @@ func (c *RegionCache) switchWorkStore(r *Region, storeID uint64) (foundLeader bo leaderIdx = 0 } atomic.StoreInt32(&r.workStoreIdx, int32(leaderIdx)) + atomic.StoreInt32(&r.syncStoreIdx, int32(leaderIdx)) return } @@ -915,8 +939,8 @@ const ( startBlackoutAfterAttempt = 20 ) -// Operational returns whether store is operational and no need retry other node. -func (s *Store) Operational() bool { +// stableLeader returns whether store is stable leader and no need retry other node. +func (s *Store) stableLeader() bool { var state storeState *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) if state.failedAttempt == 0 || state.lastFailedTime == 0 { diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index baa90dc2132aa..49a3c9657b587 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -378,9 +378,9 @@ func (s *testRegionCacheSuite) TestSendFailBlackTwoRegion(c *C) { ts = time.Now().Unix() c.Assert(reachableStore(s.cache.storeMu.stores, ts), Equals, 2) s.getRegion(c, []byte("a")) - c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 1) s.getRegion(c, []byte("x")) c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 2) + c.Assert(workableRegions(s.cache, s.cache.mu.regions, ts), Equals, 2) } func (s *testRegionCacheSuite) TestRegionEpochAheadOfTiKV(c *C) { From 21a511aac1166984dc356eabe94916244427cb0b Mon Sep 17 00:00:00 2001 From: lysu Date: Wed, 8 May 2019 22:29:32 +0800 Subject: [PATCH 11/27] fix locate key --- store/tikv/region_cache.go | 74 +++++++++++++++++++++++++++----------- 1 file changed, 53 insertions(+), 21 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 8df4a8b76ebae..274958896dc15 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -268,16 +268,30 @@ func (l *KeyLocation) Contains(key []byte) bool { // LocateKey searches for the region and range that the key is located. func (c *RegionCache) LocateKey(bo *Backoffer, key []byte) (*KeyLocation, error) { r := c.searchCachedRegion(key, false) - if r == nil || r.needReload() { - var err error - r, err = c.loadRegion(bo, key, false) + if r == nil { + // load region when it is not exists or expired. + lr, err := c.loadRegion(bo, key, false) if err != nil { - return nil, errors.Trace(err) + // no region data, return error if failure. + return nil, err } - + r = lr c.mu.Lock() c.insertRegionToCache(r) c.mu.Unlock() + } else if r.needReload() { + // load region when it be marked as need reload. + lr, err := c.loadRegion(bo, key, false) + if err != nil { + // ignore error and use old region info. + logutil.Logger(bo.ctx).Error("load region failure", + zap.ByteString("key", key), zap.Error(err)) + } else { + r = lr + c.mu.Lock() + c.insertRegionToCache(r) + c.mu.Unlock() + } } return &KeyLocation{ Region: r.VerID(), @@ -286,28 +300,46 @@ func (c *RegionCache) LocateKey(bo *Backoffer, key []byte) (*KeyLocation, error) }, nil } -// LocateEndKey searches for the region and range that the key is located. -// Unlike LocateKey, start key of a region is exclusive and end key is inclusive. -func (c *RegionCache) LocateEndKey(bo *Backoffer, key []byte) (*KeyLocation, error) { - r := c.searchCachedRegion(key, true) - if r != nil { - loc := &KeyLocation{ - Region: r.VerID(), - StartKey: r.StartKey(), - EndKey: r.EndKey(), - } - return loc, nil - } - - r, err := c.loadRegion(bo, key, true) +func (c *RegionCache) loadAndInsertRegion(bo *Backoffer, key []byte) (*Region, error) { + r, err := c.loadRegion(bo, key, false) if err != nil { - return nil, errors.Trace(err) + return nil, err } - c.mu.Lock() c.insertRegionToCache(r) c.mu.Unlock() + return r, nil +} +// LocateEndKey searches for the region and range that the key is located. +// Unlike LocateKey, start key of a region is exclusive and end key is inclusive. +func (c *RegionCache) LocateEndKey(bo *Backoffer, key []byte) (*KeyLocation, error) { + r := c.searchCachedRegion(key, true) + if r == nil { + // load region when it is not exists or expired. + lr, err := c.loadRegion(bo, key, true) + if err != nil { + // no region data, return error if failure. + return nil, err + } + r = lr + c.mu.Lock() + c.insertRegionToCache(r) + c.mu.Unlock() + } else if r.needReload() { + // load region when it be marked as need reload. + lr, err := c.loadRegion(bo, key, true) + if err != nil { + // ignore error and use old region info. + logutil.Logger(bo.ctx).Error("load region failure", + zap.ByteString("key", key), zap.Error(err)) + } else { + r = lr + c.mu.Lock() + c.insertRegionToCache(r) + c.mu.Unlock() + } + } return &KeyLocation{ Region: r.VerID(), StartKey: r.StartKey(), From c3cb4473d0300a39c9e824e4b1edad80a1ba5a23 Mon Sep 17 00:00:00 2001 From: lysu Date: Thu, 9 May 2019 12:53:02 +0800 Subject: [PATCH 12/27] refine region epoch not match handle --- store/tikv/region_cache.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 274958896dc15..5772246e2ce9f 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -694,15 +694,6 @@ func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { // OnRegionEpochNotMatch removes the old region and inserts new regions into the cache. func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, currentRegions []*metapb.Region) error { - c.mu.Lock() - defer c.mu.Unlock() - - cachedRegion, ok := c.mu.regions[ctx.Region] - if ok { - tikvRegionCacheCounterWithDropRegionFromCacheOK.Inc() - cachedRegion.invalidate() - } - // Find whether the region epoch in `ctx` is ahead of TiKV's. If so, backoff. for _, meta := range currentRegions { if meta.GetId() == ctx.Region.id && @@ -714,6 +705,9 @@ func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, curr } } + c.mu.Lock() + defer c.mu.Unlock() + needInvalidateOld := true // If the region epoch is not ahead of TiKV's, replace region meta in region cache. for _, meta := range currentRegions { if _, ok := c.pdClient.(*codecPDClient); ok { @@ -729,6 +723,16 @@ func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, curr region.initStores(c) c.switchWorkStore(region, ctx.Store.storeID) c.insertRegionToCache(region) + if ctx.Region == region.VerID() { + needInvalidateOld = false + } + } + if needInvalidateOld { + cachedRegion, ok := c.mu.regions[ctx.Region] + if ok { + tikvRegionCacheCounterWithDropRegionFromCacheOK.Inc() + cachedRegion.invalidate() + } } return nil } From ca2ef1b1dbe375577566f63103eaa4b5fbe6e103 Mon Sep 17 00:00:00 2001 From: lysu Date: Thu, 9 May 2019 13:32:54 +0800 Subject: [PATCH 13/27] notify check store via a channel --- store/tikv/region_cache.go | 29 ++++++++++++++++++++--------- store/tikv/region_request.go | 4 ++-- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 5772246e2ce9f..36746531b7e01 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -126,7 +126,8 @@ type RegionCache struct { sync.RWMutex stores map[uint64]*Store } - closeCh chan struct{} + notifyCheckCh chan struct{} + closeCh chan struct{} } // NewRegionCache creates a RegionCache. @@ -137,6 +138,7 @@ func NewRegionCache(pdClient pd.Client) *RegionCache { c.mu.regions = make(map[RegionVerID]*Region) c.mu.sorted = btree.New(btreeDegree) c.storeMu.stores = make(map[uint64]*Store) + c.notifyCheckCh = make(chan struct{}, 1) c.closeCh = make(chan struct{}) go c.asyncCheckAndResolveLoop() return c @@ -147,16 +149,12 @@ func (c *RegionCache) Close() { close(c.closeCh) } -const checkStoreInterval = 1 * time.Second - // asyncCheckAndResolveLoop with func (c *RegionCache) asyncCheckAndResolveLoop() { - tick := time.NewTicker(checkStoreInterval) - defer tick.Stop() var needCheckStores []*Store for { select { - case <-tick.C: + case <-c.notifyCheckCh: c.checkAndResolve(&needCheckStores) case <-c.closeCh: return @@ -680,7 +678,7 @@ func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { } c.storeMu.RUnlock() - store.markAccess(false) + store.markAccess(c, false) r := c.getCachedRegionWithRLock(ctx.Region) if r == nil { @@ -1022,8 +1020,9 @@ func (s *Store) needCheck() bool { } // markAccess marks the processing result. -func (s *Store) markAccess(success bool) { +func (s *Store) markAccess(c *RegionCache, success bool) { retry: + var triggerCheck bool oldValue := atomic.LoadUint64(&s.state) var state storeState *(*uint64)(unsafe.Pointer(&state)) = oldValue @@ -1038,6 +1037,7 @@ retry: state.failedAttempt = state.failedAttempt + 1 if state.resolveState == resolved { state.resolveState = needCheck + triggerCheck = true } } else { state.lastFailedTime = 0 @@ -1047,6 +1047,12 @@ retry: if !atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) { goto retry } + if triggerCheck { + select { + case c.notifyCheckCh <- struct{}{}: + default: + } + } } // markResolved marks store has be resolved. @@ -1066,7 +1072,7 @@ retry: } // markNeedCheck marks resolved store to be async resolve to check store addr change. -func (s *Store) markNeedCheck() { +func (s *Store) markNeedCheck(c *RegionCache) { retry: oldValue := atomic.LoadUint64(&s.state) var state storeState @@ -1079,4 +1085,9 @@ retry: if !atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) { goto retry } + select { + case c.notifyCheckCh <- struct{}{}: + default: + } + } diff --git a/store/tikv/region_request.go b/store/tikv/region_request.go index 17ad3e85ee0c3..905ff0ea14187 100644 --- a/store/tikv/region_request.go +++ b/store/tikv/region_request.go @@ -149,7 +149,7 @@ func (s *RegionRequestSender) sendReqToRegion(bo *Backoffer, ctx *RPCContext, re func (s *RegionRequestSender) onSendSuccess(ctx *RPCContext) { store := s.regionCache.getStoreByStoreID(ctx.Store.storeID) - store.markAccess(true) + store.markAccess(s.regionCache, true) } func (s *RegionRequestSender) onSendFail(bo *Backoffer, ctx *RPCContext, err error) error { @@ -226,7 +226,7 @@ func (s *RegionRequestSender) onRegionError(bo *Backoffer, ctx *RPCContext, regi logutil.Logger(context.Background()).Warn("tikv reports `StoreNotMatch` retry later", zap.Stringer("storeNotMatch", storeNotMatch), zap.Stringer("ctx", ctx)) - ctx.Store.markNeedCheck() + ctx.Store.markNeedCheck(s.regionCache) return true, nil } From 0311be29efcf820870fab2e4a24ad2a4075ff8f1 Mon Sep 17 00:00:00 2001 From: lysu Date: Sat, 11 May 2019 16:10:12 +0800 Subject: [PATCH 14/27] ac: reduce atomic.Load time --- store/tikv/region_cache.go | 381 ++++++++++++++++++-------------- store/tikv/region_cache_test.go | 18 +- 2 files changed, 223 insertions(+), 176 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 36746531b7e01..da5735fb2a57b 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -55,15 +55,37 @@ const ( // Region presents kv region type Region struct { - meta *metapb.Region // immutable after fetched from pd - stores []*Store // stores in this region - workStoreIdx int32 // point to current work peer in meta.Peers and work store in stores(same idx) - syncStoreIdx int32 // point to init peer index and need reload if meet syncStoreIdx again - syncFlag int32 // mark region need be sync in next turn - lastAccess int64 // last region access time, see checkRegionCacheTTL + meta *metapb.Region // raw region meta from PD immutable after init + store unsafe.Pointer // point to region store info, see RegionStore + syncFlag int32 // region need be sync in next turn + lastAccess int64 // last region access time, see checkRegionCacheTTL } -func (r *Region) initStores(c *RegionCache) { +// RegionStore presents region stores info +// it will be store as unsafe.Pointer and be load at once +type RegionStore struct { + workStoreIdx int32 // point to current work peer in meta.Peers and work store in stores(same idx) + syncStoreIdx int32 // need trigger reload if meet syncStoreIdx again + stores []*Store // stores in this region +} + +// clone clones region store struct. +func (r *RegionStore) clone() *RegionStore { + return &RegionStore{ + workStoreIdx: r.workStoreIdx, + syncStoreIdx: r.syncStoreIdx, + stores: r.stores, + } +} + +// init initializes region after constructed. +func (r *Region) init(c *RegionCache) { + // region store pull used store from global store map + // to avoid acquire storeMu in later access. + rs := &RegionStore{ + workStoreIdx: 0, + stores: make([]*Store, 0, len(r.meta.Peers)), + } for _, p := range r.meta.Peers { c.storeMu.RLock() store, exists := c.storeMu.stores[p.StoreId] @@ -71,8 +93,21 @@ func (r *Region) initStores(c *RegionCache) { if !exists { store = c.getStoreByStoreID(p.StoreId) } - r.stores = append(r.stores, store) + rs.stores = append(rs.stores, store) } + atomic.StorePointer(&r.store, unsafe.Pointer(rs)) + + // mark region has been init accessed. + r.lastAccess = time.Now().Unix() +} + +func (r *Region) getStore() (store *RegionStore) { + store = (*RegionStore)(atomic.LoadPointer(&r.store)) + return +} + +func (r *Region) compareAndSwapStore(oldStore, newStore *RegionStore) bool { + return atomic.CompareAndSwapPointer(&r.store, unsafe.Pointer(oldStore), unsafe.Pointer(newStore)) } func (r *Region) checkRegionCacheTTL(ts int64) bool { @@ -94,14 +129,11 @@ func (r *Region) invalidate() { // scheduleReload schedules reload region request in next LocateKey. func (r *Region) scheduleReload() { -retry: oldValue := atomic.LoadInt32(&r.syncFlag) if oldValue != updated { return } - if !atomic.CompareAndSwapInt32(&r.syncFlag, oldValue, needSync) { - goto retry - } + atomic.CompareAndSwapInt32(&r.syncFlag, oldValue, needSync) } // needReload checks whether region need reload. @@ -154,10 +186,10 @@ func (c *RegionCache) asyncCheckAndResolveLoop() { var needCheckStores []*Store for { select { - case <-c.notifyCheckCh: - c.checkAndResolve(&needCheckStores) case <-c.closeCh: return + case <-c.notifyCheckCh: + c.checkAndResolve(&needCheckStores) } } } @@ -177,14 +209,15 @@ func (c *RegionCache) checkAndResolve(needCheckStores *[]*Store) { *needCheckStores = (*needCheckStores)[0:] c.storeMu.RLock() for _, store := range c.storeMu.stores { - if store.needCheck() { + state := store.getState() + if state.resolveState == needCheck { *needCheckStores = append(*needCheckStores, store) } } c.storeMu.RUnlock() for _, store := range *needCheckStores { - store.reResolve() + store.reResolve(c) } } @@ -224,18 +257,11 @@ func (c *RegionCache) GetRPCContext(bo *Backoffer, id RegionVerID) (*RPCContext, return nil, nil } - store, peer := c.routeStoreInRegion(cachedRegion, ts) - if store == nil { - // Store not found, region must be out of date. - cachedRegion.invalidate() - return nil, nil - } - - addr, err := store.getAddr(bo) + store, peer, addr, err := c.routeStoreInRegion(bo, cachedRegion, ts) if err != nil { return nil, err } - if len(addr) == 0 { + if store == nil || len(addr) == 0 { // Store not found, region must be out of date. cachedRegion.invalidate() return nil, nil @@ -533,12 +559,8 @@ func (c *RegionCache) loadRegion(bo *Backoffer, key []byte, isEndKey bool) (*Reg searchPrev = true continue } - region := &Region{ - meta: meta, - lastAccess: time.Now().Unix(), - stores: make([]*Store, 0, len(meta.Peers)), - } - region.initStores(c) + region := &Region{meta: meta} + region.init(c) if leader != nil { c.switchWorkStore(region, leader.StoreId) } @@ -573,12 +595,8 @@ func (c *RegionCache) loadRegionByID(bo *Backoffer, regionID uint64) (*Region, e if len(meta.Peers) == 0 { return nil, errors.New("receive Region with no peer") } - region := &Region{ - meta: meta, - lastAccess: time.Now().Unix(), - stores: make([]*Store, 0, len(meta.Peers)), - } - region.initStores(c) + region := &Region{meta: meta} + region.init(c) if leader != nil { c.switchWorkStore(region, leader.GetStoreId()) } @@ -594,27 +612,30 @@ func (c *RegionCache) getCachedRegionWithRLock(regionID RegionVerID) (r *Region) } // routeStoreInRegion ensures region have workable store and return it. -func (c *RegionCache) routeStoreInRegion(region *Region, ts int64) (workStore *Store, workPeer *metapb.Peer) { - if len(region.stores) == 0 { - return - } +func (c *RegionCache) routeStoreInRegion(bo *Backoffer, region *Region, ts int64) (workStore *Store, workPeer *metapb.Peer, workAddr string, err error) { retry: - cachedStore, cachedPeer, cachedIdx := region.WorkStorePeer() + regionStore := region.getStore() + cachedStore, cachedPeer, cachedIdx := region.WorkStorePeer(regionStore) + // almost time requests be directly routed to stable leader. - if cachedStore != nil && cachedStore.stableLeader() { + // returns if store is stable leader and no need retry other node. + state := cachedStore.getState() + if cachedStore != nil && state.failedAttempt == 0 && state.lastFailedTime == 0 { workStore = cachedStore + workAddr, err = c.getStoreAddr(bo, region, workStore, cachedIdx, state) workPeer = cachedPeer return } // try round-robin find & switch to other peers when old leader meet error. newIdx := -1 - storeNum := len(region.stores) + storeNum := len(regionStore.stores) i := (cachedIdx + 1) % storeNum start := i for { - store := region.stores[i] - if store.Available(ts) { + store := regionStore.stores[i] + state = store.getState() + if state.Available(ts) { newIdx = i break } @@ -626,25 +647,61 @@ retry: if newIdx < 0 { return } - if !atomic.CompareAndSwapInt32(®ion.workStoreIdx, int32(cachedIdx), int32(newIdx)) { + newRegionStore := regionStore.clone() + newRegionStore.workStoreIdx = int32(newIdx) + if !region.compareAndSwapStore(regionStore, newRegionStore) { goto retry } - if int32(newIdx) == atomic.LoadInt32(®ion.syncStoreIdx) { + if int32(newIdx) == regionStore.syncStoreIdx { region.scheduleReload() } - workStore = region.stores[newIdx] + + workStore = newRegionStore.stores[newIdx] + workAddr, err = c.getStoreAddr(bo, region, workStore, newIdx, state) workPeer = region.meta.Peers[newIdx] return } +func (c *RegionCache) getStoreAddr(bo *Backoffer, region *Region, store *Store, storeIdx int, state storeState) (addr string, err error) { + switch state.resolveState { + case resolved, needCheck: + addr = store.addr + return + case unresolved: + addr, err = store.initResolve(bo) + return + case deleted: + c.storeMu.RLock() + store = c.storeMu.stores[store.storeID] + c.storeMu.RUnlock() + retry: + oldRegionStore := region.getStore() + newRegionStore := oldRegionStore.clone() + newRegionStore.stores = make([]*Store, 0, len(oldRegionStore.stores)) + for i, s := range oldRegionStore.stores { + if i == storeIdx { + newRegionStore.stores = append(newRegionStore.stores, store) + } else { + newRegionStore.stores = append(newRegionStore.stores, s) + } + } + if !region.compareAndSwapStore(oldRegionStore, newRegionStore) { + goto retry + } + addr = store.addr + return + default: + panic("unsupported resolve state") + } +} + // hasAvailableStore checks whether region has available store. // different to `routeStoreInRegion` just check and never change work store or peer. func (c *RegionCache) hasAvailableStore(region *Region, ts int64) bool { - if len(region.stores) == 0 { - return false - } - for _, store := range region.stores { - if store.Available(ts) { + regionStore := region.getStore() + for _, store := range regionStore.stores { + state := store.getState() + if state.Available(ts) { return true } } @@ -669,25 +726,13 @@ func (c *RegionCache) getStoreByStoreID(storeID uint64) (store *Store) { // OnSendRequestFail is used for clearing cache when a tikv server does not respond. func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { failedStoreID := ctx.Store.storeID - c.storeMu.RLock() store, exists := c.storeMu.stores[failedStoreID] + c.storeMu.RUnlock() if !exists { - c.storeMu.RUnlock() return } - c.storeMu.RUnlock() - store.markAccess(c, false) - - r := c.getCachedRegionWithRLock(ctx.Region) - if r == nil { - return - } - lastAccess := atomic.LoadInt64(&r.lastAccess) - if lastAccess == invalidatedLastAccessTime { - return - } } // OnRegionEpochNotMatch removes the old region and inserts new regions into the cache. @@ -713,12 +758,8 @@ func (c *RegionCache) OnRegionEpochNotMatch(bo *Backoffer, ctx *RPCContext, curr return errors.Errorf("newRegion's range key is not encoded: %v, %v", meta, err) } } - region := &Region{ - meta: meta, - lastAccess: time.Now().Unix(), - stores: make([]*Store, 0, len(meta.Peers)), - } - region.initStores(c) + region := &Region{meta: meta} + region.init(c) c.switchWorkStore(region, ctx.Store.storeID) c.insertRegionToCache(region) if ctx.Region == region.VerID() { @@ -769,10 +810,10 @@ func (r *Region) GetID() uint64 { } // WorkStorePeer returns current work store with work peer. -func (r *Region) WorkStorePeer() (store *Store, peer *metapb.Peer, idx int) { - idx = int(atomic.LoadInt32(&r.workStoreIdx)) - store = r.stores[idx] - peer = r.meta.Peers[idx] +func (r *Region) WorkStorePeer(rs *RegionStore) (store *Store, peer *metapb.Peer, idx int) { + idx = int(rs.workStoreIdx) + store = rs.stores[rs.workStoreIdx] + peer = r.meta.Peers[rs.workStoreIdx] return } @@ -813,19 +854,24 @@ func (c *RegionCache) switchWorkStore(r *Region, storeID uint64) (foundLeader bo if len(r.meta.Peers) == 0 { return } - leaderIdx := -1 + + leaderIdx := 0 for i, p := range r.meta.Peers { if p.GetStoreId() == storeID { leaderIdx = i + foundLeader = true } } - if leaderIdx >= 0 { - foundLeader = true - } else { - leaderIdx = 0 + +retry: + // switch to new leader. + oldRegionStore := r.getStore() + newRegionStore := oldRegionStore.clone() + newRegionStore.workStoreIdx = int32(leaderIdx) + newRegionStore.syncStoreIdx = int32(leaderIdx) + if !r.compareAndSwapStore(oldRegionStore, newRegionStore) { + goto retry } - atomic.StoreInt32(&r.workStoreIdx, int32(leaderIdx)) - atomic.StoreInt32(&r.syncStoreIdx, int32(leaderIdx)) return } @@ -849,9 +895,9 @@ type resolveFunc func(ctx context.Context, id uint64) (*metapb.Store, error) // Store contains a kv process's address. type Store struct { - addr atomic.Value // loaded store address(*string) - storeID uint64 // store's id - state uint64 // unsafe store storeState + addr string // loaded store address + storeID uint64 // store's id + state uint64 // unsafe store storeState resolve struct { sync.Mutex // protect pd from concurrent init requests @@ -866,44 +912,22 @@ type storeState struct { resolveState resolveState } -type resolveState uint8 +type resolveState uint16 const ( unresolved resolveState = iota resolved needCheck + deleted ) -// getAddr resolves the address of store. -// following up resolve request will reuse previous result until -// store become unreachable and after reResolveUnreachableStoreIntervalSec -func (s *Store) getAddr(bo *Backoffer) (addr string, err error) { - // always use current addr event if it maybe staled. - if !s.needInitResolve() { - v := s.addr.Load() - if v == nil { - addr = "" - return - } - addr = v.(string) - return - } - // only resolve store addr from init status at first time. - addr, err = s.initResolve(bo) - return -} - // initResolve resolves addr for store that never resolved. func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { s.resolve.Lock() - if !s.needInitResolve() { + state := s.getState() + if state.resolveState != unresolved { s.resolve.Unlock() - v := s.addr.Load() - if v == nil { - addr = "" - return - } - addr = v.(string) + addr = s.addr return } var store *metapb.Store @@ -932,15 +956,26 @@ func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { return } addr = store.GetAddress() - s.addr.Store(addr) - s.markResolved() + s.addr = addr + retry: + state = s.getState() + if state.resolveState != unresolved { + s.resolve.Unlock() + addr = s.addr + return + } + newState := state + newState.resolveState = resolved + if !s.compareAndSwapState(state, newState) { + goto retry + } s.resolve.Unlock() return } } // reResolve try to resolve addr for store that need check. -func (s *Store) reResolve() { +func (s *Store) reResolve(c *RegionCache) { var addr string store, err := s.resolve.fn(context.Background(), s.storeID) if err != nil { @@ -960,9 +995,40 @@ func (s *Store) reResolve() { if store == nil { return } + addr = store.GetAddress() - s.addr.Store(addr) - s.markResolved() + if s.addr != addr { + var state storeState + state.resolveState = resolved + newStore := &Store{storeID: s.storeID, addr: addr} + newStore.resolve.fn = c.pdClient.GetStore + newStore.state = *(*uint64)(unsafe.Pointer(&state)) + c.storeMu.Lock() + c.storeMu.stores[newStore.storeID] = newStore + c.storeMu.Unlock() + retryMarkDel: + // all region used those + oldState := s.getState() + if oldState.resolveState == deleted { + return + } + newState := oldState + newState.resolveState = deleted + if !s.compareAndSwapState(oldState, newState) { + goto retryMarkDel + } + return + } +retryMarkResolved: + oldState := s.getState() + if oldState.resolveState != needCheck { + return + } + newState := oldState + newState.resolveState = resolved + if !s.compareAndSwapState(oldState, newState) { + goto retryMarkResolved + } return } @@ -973,21 +1039,28 @@ const ( startBlackoutAfterAttempt = 20 ) -// stableLeader returns whether store is stable leader and no need retry other node. -func (s *Store) stableLeader() bool { +func (s *Store) getState() storeState { var state storeState - *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) - if state.failedAttempt == 0 || state.lastFailedTime == 0 { - // return quickly if it's continue success. - return true + if s == nil { + return state } - return false + x := atomic.LoadUint64(&s.state) + *(*uint64)(unsafe.Pointer(&state)) = x + return state +} + +func (s *Store) compareAndSwapState(oldState, newState storeState) bool { + oldValue, newValue := *(*uint64)(unsafe.Pointer(&oldState)), *(*uint64)(unsafe.Pointer(&newState)) + return atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) +} + +func (s *Store) storeState(newState storeState) { + newValue := *(*uint64)(unsafe.Pointer(&newState)) + atomic.StoreUint64(&s.state, newValue) } // Available returns whether store be available for current. -func (s *Store) Available(ts int64) bool { - var state storeState - *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) +func (state storeState) Available(ts int64) bool { if state.failedAttempt == 0 || state.lastFailedTime == 0 { // return quickly if it's continue success. return true @@ -1005,31 +1078,16 @@ func (s *Store) Available(ts int64) bool { return blackoutDeadline <= ts } -// needInitResolve checks whether store need to do init block resolve. -func (s *Store) needInitResolve() bool { - var state storeState - *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) - return state.resolveState == unresolved -} - -// needCheck checks whether store need to do async resolve check. -func (s *Store) needCheck() bool { - var state storeState - *(*uint64)(unsafe.Pointer(&state)) = atomic.LoadUint64(&s.state) - return state.resolveState == needCheck -} - // markAccess marks the processing result. func (s *Store) markAccess(c *RegionCache, success bool) { retry: var triggerCheck bool - oldValue := atomic.LoadUint64(&s.state) - var state storeState - *(*uint64)(unsafe.Pointer(&state)) = oldValue - if (state.failedAttempt == 0 && success) || (!success && state.failedAttempt >= (startBlackoutAfterAttempt+maxExponentAttempt)) { + oldState := s.getState() + if (oldState.failedAttempt == 0 && success) || (!success && oldState.failedAttempt >= (startBlackoutAfterAttempt+maxExponentAttempt)) { // return quickly if continue success, and no more mark when attempt meet max bound. return } + state := oldState if !success { if state.lastFailedTime == 0 { state.lastFailedTime = uint32(time.Now().Unix()) @@ -1043,8 +1101,7 @@ retry: state.lastFailedTime = 0 state.failedAttempt = 0 } - newValue := *(*uint64)(unsafe.Pointer(&state)) - if !atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) { + if !s.compareAndSwapState(oldState, state) { goto retry } if triggerCheck { @@ -1055,34 +1112,16 @@ retry: } } -// markResolved marks store has be resolved. -func (s *Store) markResolved() { -retry: - oldValue := atomic.LoadUint64(&s.state) - var state storeState - *(*uint64)(unsafe.Pointer(&state)) = oldValue - if state.resolveState == resolved { - return - } - state.resolveState = resolved - newValue := *(*uint64)(unsafe.Pointer(&state)) - if !atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) { - goto retry - } -} - // markNeedCheck marks resolved store to be async resolve to check store addr change. func (s *Store) markNeedCheck(c *RegionCache) { retry: - oldValue := atomic.LoadUint64(&s.state) - var state storeState - *(*uint64)(unsafe.Pointer(&state)) = oldValue - if state.resolveState != resolved { + oldState := s.getState() + if oldState.resolveState != resolved { return } + state := oldState state.resolveState = needCheck - newValue := *(*uint64)(unsafe.Pointer(&state)) - if !atomic.CompareAndSwapUint64(&s.state, oldValue, newValue) { + if !s.compareAndSwapState(oldState, state) { goto retry } select { diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 49a3c9657b587..49d2375530db2 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -63,7 +63,8 @@ func (s *testRegionCacheSuite) checkCache(c *C, len int) { c.Assert(workableRegionsInBtree(s.cache, s.cache.mu.sorted, ts), Equals, len) for _, r := range s.cache.mu.regions { if r.checkRegionCacheTTL(ts) { - if store, _ := s.cache.routeStoreInRegion(r, ts); store != nil { + bo := NewBackoffer(context.Background(), 100) + if store, _, _, _ := s.cache.routeStoreInRegion(bo, r, ts); store != nil { c.Assert(r, DeepEquals, s.cache.searchCachedRegion(r.StartKey(), false)) } } @@ -75,7 +76,8 @@ func workableRegions(cache *RegionCache, regions map[RegionVerID]*Region, ts int if !region.checkRegionCacheTTL(ts) { continue } - store, _ := cache.routeStoreInRegion(region, ts) + bo := NewBackoffer(context.Background(), 100) + store, _, _, _ := cache.routeStoreInRegion(bo, region, ts) if store != nil { len++ } @@ -89,7 +91,8 @@ func workableRegionsInBtree(cache *RegionCache, t *btree.BTree, ts int64) (len i if !r.checkRegionCacheTTL(ts) { return true } - store, _ := cache.routeStoreInRegion(r, ts) + bo := NewBackoffer(context.Background(), 100) + store, _, _, _ := cache.routeStoreInRegion(bo, r, ts) if store != nil { len++ } @@ -100,7 +103,8 @@ func workableRegionsInBtree(cache *RegionCache, t *btree.BTree, ts int64) (len i func reachableStore(stores map[uint64]*Store, ts int64) (cnt int) { for _, store := range stores { - if store.Available(ts) { + state := store.getState() + if state.Available(ts) { cnt++ } } @@ -347,6 +351,9 @@ func (s *testRegionCacheSuite) TestSendFailBlackTwoRegion(c *C) { region2 := s.cluster.AllocID() newPeers := s.cluster.AllocIDs(2) s.cluster.Split(s.region1, region2, []byte("m"), newPeers, newPeers[0]) + defer func() { + s.cache.Close() + }() // Check the two regions. loc1, err := s.cache.LocateKey(s.bo, []byte("a")) @@ -553,7 +560,8 @@ func BenchmarkOnRequestFail(b *testing.B) { } region := cache.getRegionByIDFromCache(loc.Region.id) b.ResetTimer() - store, peer, _ := region.WorkStorePeer() + regionStore := region.getStore() + store, peer, _ := region.WorkStorePeer(regionStore) b.RunParallel(func(pb *testing.PB) { for pb.Next() { rpcCtx := &RPCContext{ From fc54dde40bf2821e50f1fe73187e60daeade81ca Mon Sep 17 00:00:00 2001 From: lysu Date: Mon, 13 May 2019 11:34:21 +0800 Subject: [PATCH 15/27] release region-cache goroutine in testcase --- store/tikv/rawkv.go | 11 +++++++++-- store/tikv/region_cache.go | 3 ++- store/tikv/region_cache_test.go | 10 ++++++---- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/store/tikv/rawkv.go b/store/tikv/rawkv.go index 3286f02c8891d..aea8c9abf52dd 100644 --- a/store/tikv/rawkv.go +++ b/store/tikv/rawkv.go @@ -82,8 +82,15 @@ func NewRawKVClient(pdAddrs []string, security config.Security) (*RawKVClient, e // Close closes the client. func (c *RawKVClient) Close() error { - c.pdClient.Close() - c.regionCache.Close() + if c.pdClient != nil { + c.pdClient.Close() + } + if c.regionCache != nil { + c.regionCache.Close() + } + if c.rpcClient == nil { + return nil + } return c.rpcClient.Close() } diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index da5735fb2a57b..dd12c1dfceef2 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -910,9 +910,10 @@ type storeState struct { lastFailedTime uint32 failedAttempt uint16 resolveState resolveState + _Align int8 } -type resolveState uint16 +type resolveState uint8 const ( unresolved resolveState = iota diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 49d2375530db2..730f282746aad 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -17,10 +17,10 @@ import ( "context" "errors" "fmt" - "github.com/google/btree" "testing" "time" + "github.com/google/btree" . "github.com/pingcap/check" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/tidb/store/mockstore/mocktikv" @@ -53,6 +53,10 @@ func (s *testRegionCacheSuite) SetUpTest(c *C) { s.bo = NewBackoffer(context.Background(), 5000) } +func (s *testRegionCacheSuite) TearDownTest(c *C) { + s.cache.Close() +} + func (s *testRegionCacheSuite) storeAddr(id uint64) string { return fmt.Sprintf("store%d", id) } @@ -351,9 +355,6 @@ func (s *testRegionCacheSuite) TestSendFailBlackTwoRegion(c *C) { region2 := s.cluster.AllocID() newPeers := s.cluster.AllocIDs(2) s.cluster.Split(s.region1, region2, []byte("m"), newPeers, newPeers[0]) - defer func() { - s.cache.Close() - }() // Check the two regions. loc1, err := s.cache.LocateKey(s.bo, []byte("a")) @@ -472,6 +473,7 @@ func (s *testRegionCacheSuite) TestUpdateStoreAddr(c *C) { regionCache: NewRegionCache(mocktikv.NewPDClient(s.cluster)), rpcClient: mocktikv.NewRPCClient(s.cluster, mvccStore), } + defer client.Close() testKey := []byte("test_key") testValue := []byte("test_value") err := client.Put(testKey, testValue) From 438c590ff534d8966a5978e2dd59dc9ed4f39755 Mon Sep 17 00:00:00 2001 From: lysu Date: Tue, 14 May 2019 11:55:17 +0800 Subject: [PATCH 16/27] address comments --- store/tikv/region_cache.go | 107 +++++++++++++++----------------- store/tikv/region_cache_test.go | 4 +- 2 files changed, 52 insertions(+), 59 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index dd12c1dfceef2..57abdba24a9a3 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -61,7 +61,7 @@ type Region struct { lastAccess int64 // last region access time, see checkRegionCacheTTL } -// RegionStore presents region stores info +// RegionStore represents region stores info // it will be store as unsafe.Pointer and be load at once type RegionStore struct { workStoreIdx int32 // point to current work peer in meta.Peers and work store in stores(same idx) @@ -111,15 +111,15 @@ func (r *Region) compareAndSwapStore(oldStore, newStore *RegionStore) bool { } func (r *Region) checkRegionCacheTTL(ts int64) bool { -retry: - lastAccess := atomic.LoadInt64(&r.lastAccess) - if ts-lastAccess > rcDefaultRegionCacheTTLSec { - return false - } - if !atomic.CompareAndSwapInt64(&r.lastAccess, lastAccess, ts) { - goto retry + for { + lastAccess := atomic.LoadInt64(&r.lastAccess) + if ts-lastAccess > rcDefaultRegionCacheTTLSec { + return false + } + if atomic.CompareAndSwapInt64(&r.lastAccess, lastAccess, ts) { + return true + } } - return true } // invalidate invalidates a region, next time it will got null result. @@ -189,14 +189,14 @@ func (c *RegionCache) asyncCheckAndResolveLoop() { case <-c.closeCh: return case <-c.notifyCheckCh: - c.checkAndResolve(&needCheckStores) + needCheckStores = c.checkAndResolve(needCheckStores) } } } // checkAndResolve checks and resolve addr of failed stores. // this method isn't thread-safe and only be used by one goroutine. -func (c *RegionCache) checkAndResolve(needCheckStores *[]*Store) { +func (c *RegionCache) checkAndResolve(needCheckStores []*Store) []*Store { defer func() { r := recover() if r != nil { @@ -206,19 +206,20 @@ func (c *RegionCache) checkAndResolve(needCheckStores *[]*Store) { } }() - *needCheckStores = (*needCheckStores)[0:] + needCheckStores = needCheckStores[0:] c.storeMu.RLock() for _, store := range c.storeMu.stores { state := store.getState() if state.resolveState == needCheck { - *needCheckStores = append(*needCheckStores, store) + needCheckStores = append(needCheckStores, store) } } c.storeMu.RUnlock() - for _, store := range *needCheckStores { + for _, store := range needCheckStores { store.reResolve(c) } + return needCheckStores } // RPCContext contains data that is needed to send RPC to a region. @@ -291,31 +292,9 @@ func (l *KeyLocation) Contains(key []byte) bool { // LocateKey searches for the region and range that the key is located. func (c *RegionCache) LocateKey(bo *Backoffer, key []byte) (*KeyLocation, error) { - r := c.searchCachedRegion(key, false) - if r == nil { - // load region when it is not exists or expired. - lr, err := c.loadRegion(bo, key, false) - if err != nil { - // no region data, return error if failure. - return nil, err - } - r = lr - c.mu.Lock() - c.insertRegionToCache(r) - c.mu.Unlock() - } else if r.needReload() { - // load region when it be marked as need reload. - lr, err := c.loadRegion(bo, key, false) - if err != nil { - // ignore error and use old region info. - logutil.Logger(bo.ctx).Error("load region failure", - zap.ByteString("key", key), zap.Error(err)) - } else { - r = lr - c.mu.Lock() - c.insertRegionToCache(r) - c.mu.Unlock() - } + r, err := c.findRegionByKey(bo, key, false) + if err != nil { + return nil, err } return &KeyLocation{ Region: r.VerID(), @@ -338,10 +317,22 @@ func (c *RegionCache) loadAndInsertRegion(bo *Backoffer, key []byte) (*Region, e // LocateEndKey searches for the region and range that the key is located. // Unlike LocateKey, start key of a region is exclusive and end key is inclusive. func (c *RegionCache) LocateEndKey(bo *Backoffer, key []byte) (*KeyLocation, error) { - r := c.searchCachedRegion(key, true) + r, err := c.findRegionByKey(bo, key, true) + if err != nil { + return nil, err + } + return &KeyLocation{ + Region: r.VerID(), + StartKey: r.StartKey(), + EndKey: r.EndKey(), + }, nil +} + +func (c *RegionCache) findRegionByKey(bo *Backoffer, key []byte, isEndKey bool) (r *Region, err error) { + r = c.searchCachedRegion(key, isEndKey) if r == nil { // load region when it is not exists or expired. - lr, err := c.loadRegion(bo, key, true) + lr, err := c.loadRegion(bo, key, isEndKey) if err != nil { // no region data, return error if failure. return nil, err @@ -352,7 +343,7 @@ func (c *RegionCache) LocateEndKey(bo *Backoffer, key []byte) (*KeyLocation, err c.mu.Unlock() } else if r.needReload() { // load region when it be marked as need reload. - lr, err := c.loadRegion(bo, key, true) + lr, err := c.loadRegion(bo, key, isEndKey) if err != nil { // ignore error and use old region info. logutil.Logger(bo.ctx).Error("load region failure", @@ -364,11 +355,7 @@ func (c *RegionCache) LocateEndKey(bo *Backoffer, key []byte) (*KeyLocation, err c.mu.Unlock() } } - return &KeyLocation{ - Region: r.VerID(), - StartKey: r.StartKey(), - EndKey: r.EndKey(), - }, nil + return r, nil } // LocateRegionByID searches for the region with ID. @@ -671,10 +658,18 @@ func (c *RegionCache) getStoreAddr(bo *Backoffer, region *Region, store *Store, addr, err = store.initResolve(bo) return case deleted: - c.storeMu.RLock() - store = c.storeMu.stores[store.storeID] - c.storeMu.RUnlock() - retry: + addr = c.changeToActiveStore(region, store, storeIdx) + return + default: + panic("unsupported resolve state") + } +} + +func (c *RegionCache) changeToActiveStore(region *Region, store *Store, storeIdx int) (addr string) { + c.storeMu.RLock() + store = c.storeMu.stores[store.storeID] + c.storeMu.RUnlock() + for { oldRegionStore := region.getStore() newRegionStore := oldRegionStore.clone() newRegionStore.stores = make([]*Store, 0, len(oldRegionStore.stores)) @@ -685,14 +680,12 @@ func (c *RegionCache) getStoreAddr(bo *Backoffer, region *Region, store *Store, newRegionStore.stores = append(newRegionStore.stores, s) } } - if !region.compareAndSwapStore(oldRegionStore, newRegionStore) { - goto retry + if region.compareAndSwapStore(oldRegionStore, newRegionStore) { + break } - addr = store.addr - return - default: - panic("unsupported resolve state") } + addr = store.addr + return } // hasAvailableStore checks whether region has available store. diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 730f282746aad..1542d937f65d4 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -368,11 +368,11 @@ func (s *testRegionCacheSuite) TestSendFailBlackTwoRegion(c *C) { s.checkCache(c, 2) // send request fail in 2 regions backed by same 2 stores. - for i := 0; i < 20; i++ { + for i := 0; i < startBlackoutAfterAttempt; i++ { ctx, _ := s.cache.GetRPCContext(s.bo, loc1.Region) s.cache.OnSendRequestFail(ctx, errors.New("test error")) } - for i := 0; i < 20; i++ { + for i := 0; i < startBlackoutAfterAttempt; i++ { ctx, _ := s.cache.GetRPCContext(s.bo, loc2.Region) s.cache.OnSendRequestFail(ctx, errors.New("test error")) } From b2a99629adee9d7ce5e3ed1e3643467ced669c35 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 17 May 2019 16:02:17 +0800 Subject: [PATCH 17/27] address comments --- store/tikv/region_cache.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 57abdba24a9a3..e803c4b1620a0 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -189,14 +189,15 @@ func (c *RegionCache) asyncCheckAndResolveLoop() { case <-c.closeCh: return case <-c.notifyCheckCh: - needCheckStores = c.checkAndResolve(needCheckStores) + needCheckStores = needCheckStores[:0] + c.checkAndResolve(needCheckStores) } } } // checkAndResolve checks and resolve addr of failed stores. // this method isn't thread-safe and only be used by one goroutine. -func (c *RegionCache) checkAndResolve(needCheckStores []*Store) []*Store { +func (c *RegionCache) checkAndResolve(needCheckStores []*Store) { defer func() { r := recover() if r != nil { @@ -206,7 +207,6 @@ func (c *RegionCache) checkAndResolve(needCheckStores []*Store) []*Store { } }() - needCheckStores = needCheckStores[0:] c.storeMu.RLock() for _, store := range c.storeMu.stores { state := store.getState() @@ -219,7 +219,6 @@ func (c *RegionCache) checkAndResolve(needCheckStores []*Store) []*Store { for _, store := range needCheckStores { store.reResolve(c) } - return needCheckStores } // RPCContext contains data that is needed to send RPC to a region. From 74e084c19edbdd7b0a32cb0769430ff6fc4c6688 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 17 May 2019 16:31:56 +0800 Subject: [PATCH 18/27] address comments --- store/tikv/region_cache.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index e803c4b1620a0..91f617e8e3340 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -977,10 +977,6 @@ func (s *Store) reResolve(c *RegionCache) { tikvRegionCacheCounterWithGetStoreOK.Inc() } if err != nil { - // TODO: more refine PD error status handle. - if errors.Cause(err) == context.Canceled { - return - } logutil.Logger(context.Background()).Error("loadStore from PD failed", zap.Uint64("id", s.storeID), zap.Error(err)) // we cannot do backoff in reResolve loop but try check other store and wait tick. return From 7988050feea060c7055574d32bbdd2dd24f77869 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 17 May 2019 17:04:38 +0800 Subject: [PATCH 19/27] address comments --- store/tikv/region_cache.go | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 91f617e8e3340..98df2b7810001 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -654,7 +654,7 @@ func (c *RegionCache) getStoreAddr(bo *Backoffer, region *Region, store *Store, addr = store.addr return case unresolved: - addr, err = store.initResolve(bo) + addr, err = store.initResolve(bo, c) return case deleted: addr = c.changeToActiveStore(region, store, storeIdx) @@ -709,7 +709,6 @@ func (c *RegionCache) getStoreByStoreID(storeID uint64) (store *Store) { return } store = &Store{storeID: storeID} - store.resolve.fn = c.pdClient.GetStore c.storeMu.stores[storeID] = store c.storeMu.Unlock() return @@ -887,14 +886,10 @@ type resolveFunc func(ctx context.Context, id uint64) (*metapb.Store, error) // Store contains a kv process's address. type Store struct { - addr string // loaded store address - storeID uint64 // store's id - state uint64 // unsafe store storeState - - resolve struct { - sync.Mutex // protect pd from concurrent init requests - fn resolveFunc // func to get store address from PD - } + addr string // loaded store address + storeID uint64 // store's id + state uint64 // unsafe store storeState + resolveMutex sync.Mutex // protect pd from concurrent init requests } // storeState contains store's access info. @@ -915,17 +910,17 @@ const ( ) // initResolve resolves addr for store that never resolved. -func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { - s.resolve.Lock() +func (s *Store) initResolve(bo *Backoffer, c *RegionCache) (addr string, err error) { + s.resolveMutex.Lock() state := s.getState() if state.resolveState != unresolved { - s.resolve.Unlock() + s.resolveMutex.Unlock() addr = s.addr return } var store *metapb.Store for { - store, err = s.resolve.fn(bo.ctx, s.storeID) + store, err = c.pdClient.GetStore(bo.ctx, s.storeID) if err != nil { tikvRegionCacheCounterWithGetStoreError.Inc() } else { @@ -934,18 +929,18 @@ func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { if err != nil { // TODO: more refine PD error status handle. if errors.Cause(err) == context.Canceled { - s.resolve.Unlock() + s.resolveMutex.Unlock() return } err = errors.Errorf("loadStore from PD failed, id: %d, err: %v", s.storeID, err) if err = bo.Backoff(BoPDRPC, err); err != nil { - s.resolve.Unlock() + s.resolveMutex.Unlock() return } continue } if store == nil { - s.resolve.Unlock() + s.resolveMutex.Unlock() return } addr = store.GetAddress() @@ -953,7 +948,7 @@ func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { retry: state = s.getState() if state.resolveState != unresolved { - s.resolve.Unlock() + s.resolveMutex.Unlock() addr = s.addr return } @@ -962,7 +957,7 @@ func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { if !s.compareAndSwapState(state, newState) { goto retry } - s.resolve.Unlock() + s.resolveMutex.Unlock() return } } @@ -970,7 +965,7 @@ func (s *Store) initResolve(bo *Backoffer) (addr string, err error) { // reResolve try to resolve addr for store that need check. func (s *Store) reResolve(c *RegionCache) { var addr string - store, err := s.resolve.fn(context.Background(), s.storeID) + store, err := c.pdClient.GetStore(context.Background(), s.storeID) if err != nil { tikvRegionCacheCounterWithGetStoreError.Inc() } else { @@ -990,7 +985,6 @@ func (s *Store) reResolve(c *RegionCache) { var state storeState state.resolveState = resolved newStore := &Store{storeID: s.storeID, addr: addr} - newStore.resolve.fn = c.pdClient.GetStore newStore.state = *(*uint64)(unsafe.Pointer(&state)) c.storeMu.Lock() c.storeMu.stores[newStore.storeID] = newStore From 66cfe832c4a7c3d60340b90dc3474de2d63314f8 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 17 May 2019 18:09:48 +0800 Subject: [PATCH 20/27] simplify use defer --- store/tikv/region_cache.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 98df2b7810001..5efdcbda4f253 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -913,8 +913,8 @@ const ( func (s *Store) initResolve(bo *Backoffer, c *RegionCache) (addr string, err error) { s.resolveMutex.Lock() state := s.getState() + defer s.resolveMutex.Unlock() if state.resolveState != unresolved { - s.resolveMutex.Unlock() addr = s.addr return } @@ -929,18 +929,15 @@ func (s *Store) initResolve(bo *Backoffer, c *RegionCache) (addr string, err err if err != nil { // TODO: more refine PD error status handle. if errors.Cause(err) == context.Canceled { - s.resolveMutex.Unlock() return } err = errors.Errorf("loadStore from PD failed, id: %d, err: %v", s.storeID, err) if err = bo.Backoff(BoPDRPC, err); err != nil { - s.resolveMutex.Unlock() return } continue } if store == nil { - s.resolveMutex.Unlock() return } addr = store.GetAddress() @@ -948,7 +945,6 @@ func (s *Store) initResolve(bo *Backoffer, c *RegionCache) (addr string, err err retry: state = s.getState() if state.resolveState != unresolved { - s.resolveMutex.Unlock() addr = s.addr return } @@ -957,7 +953,6 @@ func (s *Store) initResolve(bo *Backoffer, c *RegionCache) (addr string, err err if !s.compareAndSwapState(state, newState) { goto retry } - s.resolveMutex.Unlock() return } } From 77f5f6e8cf7458c725ec1f8b31dfb2a615628646 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 17 May 2019 18:12:35 +0800 Subject: [PATCH 21/27] address comment --- store/tikv/region_cache.go | 10 +++++----- store/tikv/region_request.go | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 5efdcbda4f253..feb2429d93754 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -723,7 +723,7 @@ func (c *RegionCache) OnSendRequestFail(ctx *RPCContext, err error) { if !exists { return } - store.markAccess(c, false) + store.markAccess(c.notifyCheckCh, false) } // OnRegionEpochNotMatch removes the old region and inserts new regions into the cache. @@ -1057,7 +1057,7 @@ func (state storeState) Available(ts int64) bool { } // markAccess marks the processing result. -func (s *Store) markAccess(c *RegionCache, success bool) { +func (s *Store) markAccess(notifyCheckCh chan struct{}, success bool) { retry: var triggerCheck bool oldState := s.getState() @@ -1084,14 +1084,14 @@ retry: } if triggerCheck { select { - case c.notifyCheckCh <- struct{}{}: + case notifyCheckCh <- struct{}{}: default: } } } // markNeedCheck marks resolved store to be async resolve to check store addr change. -func (s *Store) markNeedCheck(c *RegionCache) { +func (s *Store) markNeedCheck(notifyCheckCh chan struct{}) { retry: oldState := s.getState() if oldState.resolveState != resolved { @@ -1103,7 +1103,7 @@ retry: goto retry } select { - case c.notifyCheckCh <- struct{}{}: + case notifyCheckCh <- struct{}{}: default: } diff --git a/store/tikv/region_request.go b/store/tikv/region_request.go index 905ff0ea14187..e69e29f6943db 100644 --- a/store/tikv/region_request.go +++ b/store/tikv/region_request.go @@ -149,7 +149,7 @@ func (s *RegionRequestSender) sendReqToRegion(bo *Backoffer, ctx *RPCContext, re func (s *RegionRequestSender) onSendSuccess(ctx *RPCContext) { store := s.regionCache.getStoreByStoreID(ctx.Store.storeID) - store.markAccess(s.regionCache, true) + store.markAccess(s.regionCache.notifyCheckCh, true) } func (s *RegionRequestSender) onSendFail(bo *Backoffer, ctx *RPCContext, err error) error { @@ -226,7 +226,7 @@ func (s *RegionRequestSender) onRegionError(bo *Backoffer, ctx *RPCContext, regi logutil.Logger(context.Background()).Warn("tikv reports `StoreNotMatch` retry later", zap.Stringer("storeNotMatch", storeNotMatch), zap.Stringer("ctx", ctx)) - ctx.Store.markNeedCheck(s.regionCache) + ctx.Store.markNeedCheck(s.regionCache.notifyCheckCh) return true, nil } From f74329e8b686f247a83a56b601f930f1fde2ed9b Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 17 May 2019 18:59:19 +0800 Subject: [PATCH 22/27] address comment --- store/tikv/region_cache.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index feb2429d93754..0548699856c9b 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -841,16 +841,16 @@ func (r *Region) EndKey() []byte { // switchWorkStore switches current store to the one on specific store. It returns // false if no peer matches the storeID. -func (c *RegionCache) switchWorkStore(r *Region, storeID uint64) (foundLeader bool) { +func (c *RegionCache) switchWorkStore(r *Region, targetStoreID uint64) (switchToTarget bool) { if len(r.meta.Peers) == 0 { return } leaderIdx := 0 for i, p := range r.meta.Peers { - if p.GetStoreId() == storeID { + if p.GetStoreId() == targetStoreID { leaderIdx = i - foundLeader = true + switchToTarget = true } } @@ -881,9 +881,6 @@ func (r *Region) ContainsByEnd(key []byte) bool { (bytes.Compare(key, r.meta.GetEndKey()) <= 0 || len(r.meta.GetEndKey()) == 0) } -// fn loads the Store info given by storeId. -type resolveFunc func(ctx context.Context, id uint64) (*metapb.Store, error) - // Store contains a kv process's address. type Store struct { addr string // loaded store address From f81765368292c6f82fc13e199eda0b9e4885bf0a Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 17 May 2019 22:02:18 +0800 Subject: [PATCH 23/27] address comment --- store/tikv/region_cache.go | 25 +++++++++++-------------- store/tikv/region_cache_test.go | 27 --------------------------- 2 files changed, 11 insertions(+), 41 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 0548699856c9b..27e9a8d67767c 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -64,17 +64,17 @@ type Region struct { // RegionStore represents region stores info // it will be store as unsafe.Pointer and be load at once type RegionStore struct { - workStoreIdx int32 // point to current work peer in meta.Peers and work store in stores(same idx) - syncStoreIdx int32 // need trigger reload if meet syncStoreIdx again - stores []*Store // stores in this region + workStoreIdx int32 // point to current work peer in meta.Peers and work store in stores(same idx) + startingLineIdx int32 // mark starting using idx, when meet starting line again will try sync region from pd + stores []*Store // stores in this region } // clone clones region store struct. func (r *RegionStore) clone() *RegionStore { return &RegionStore{ - workStoreIdx: r.workStoreIdx, - syncStoreIdx: r.syncStoreIdx, - stores: r.stores, + workStoreIdx: r.workStoreIdx, + startingLineIdx: r.startingLineIdx, + stores: r.stores, } } @@ -446,12 +446,7 @@ func (c *RegionCache) UpdateLeader(regionID RegionVerID, leaderStoreID uint64) { zap.Uint64("leaderStoreID", leaderStoreID)) return } - if !c.switchWorkStore(r, leaderStoreID) { - logutil.Logger(context.Background()).Debug("regionCache: cannot find peer when updating leader", - zap.Uint64("regionID", regionID.GetID()), - zap.Uint64("leaderStoreID", leaderStoreID)) - r.invalidate() - } + c.switchWorkStore(r, leaderStoreID) } // insertRegionToCache tries to insert the Region to cache. @@ -638,7 +633,9 @@ retry: if !region.compareAndSwapStore(regionStore, newRegionStore) { goto retry } - if int32(newIdx) == regionStore.syncStoreIdx { + + // try all peers and fetch region info from pd again, before start next round. + if int32(newIdx) == regionStore.startingLineIdx { region.scheduleReload() } @@ -859,7 +856,7 @@ retry: oldRegionStore := r.getStore() newRegionStore := oldRegionStore.clone() newRegionStore.workStoreIdx = int32(leaderIdx) - newRegionStore.syncStoreIdx = int32(leaderIdx) + newRegionStore.startingLineIdx = int32(leaderIdx) if !r.compareAndSwapStore(oldRegionStore, newRegionStore) { goto retry } diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 1542d937f65d4..7c9abf24afd20 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -195,33 +195,6 @@ func (s *testRegionCacheSuite) TestUpdateLeader(c *C) { c.Assert(s.getAddr(c, []byte("z")), Equals, s.storeAddr(s.store2)) } -func (s *testRegionCacheSuite) TestUpdateLeader2(c *C) { - loc, err := s.cache.LocateKey(s.bo, []byte("a")) - c.Assert(err, IsNil) - // new store3 becomes leader - store3 := s.cluster.AllocID() - peer3 := s.cluster.AllocID() - s.cluster.AddStore(store3, s.storeAddr(store3)) - s.cluster.AddPeer(s.region1, store3, peer3) - // tikv-server reports `NotLeader` - s.cache.UpdateLeader(loc.Region, store3) - - // Store3 does not exist in cache, causes a reload from PD. - r := s.getRegion(c, []byte("a")) - c.Assert(r, NotNil) - c.Assert(r.GetID(), Equals, s.region1) - c.Assert(s.getAddr(c, []byte("a")), Equals, s.storeAddr(s.store1)) - - // tikv-server notifies new leader to pd-server. - s.cluster.ChangeLeader(s.region1, peer3) - // tikv-server reports `NotLeader` again. - s.cache.UpdateLeader(r.VerID(), store3) - r = s.getRegion(c, []byte("a")) - c.Assert(r, NotNil) - c.Assert(r.GetID(), Equals, s.region1) - c.Assert(s.getAddr(c, []byte("a")), Equals, s.storeAddr(store3)) -} - func (s *testRegionCacheSuite) TestUpdateLeader3(c *C) { loc, err := s.cache.LocateKey(s.bo, []byte("a")) c.Assert(err, IsNil) From c34dcbb20ef071b64fb00e314327c1a9f46d4661 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 17 May 2019 22:09:24 +0800 Subject: [PATCH 24/27] address comment --- store/tikv/region_cache.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 27e9a8d67767c..69fcdff0a3f74 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -854,6 +854,9 @@ func (c *RegionCache) switchWorkStore(r *Region, targetStoreID uint64) (switchTo retry: // switch to new leader. oldRegionStore := r.getStore() + if oldRegionStore.workStoreIdx == int32(leaderIdx) && oldRegionStore.startingLineIdx == int32(leaderIdx) { + return + } newRegionStore := oldRegionStore.clone() newRegionStore.workStoreIdx = int32(leaderIdx) newRegionStore.startingLineIdx = int32(leaderIdx) From 78044038f1ff1593bb8aa2ecde18e4ecacff1f00 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 17 May 2019 23:14:10 +0800 Subject: [PATCH 25/27] reload region after try 5 peers --- store/tikv/region_cache.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index 69fcdff0a3f74..a7c1f3def8e82 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -36,6 +36,7 @@ const ( btreeDegree = 32 rcDefaultRegionCacheTTLSec = 600 invalidatedLastAccessTime = -1 + reloadRegionThreshold = 5 ) var ( @@ -64,17 +65,17 @@ type Region struct { // RegionStore represents region stores info // it will be store as unsafe.Pointer and be load at once type RegionStore struct { - workStoreIdx int32 // point to current work peer in meta.Peers and work store in stores(same idx) - startingLineIdx int32 // mark starting using idx, when meet starting line again will try sync region from pd - stores []*Store // stores in this region + workStoreIdx int32 // point to current work peer in meta.Peers and work store in stores(same idx) + stores []*Store // stores in this region + attemptAfterLoad uint8 // indicate switch peer attempts after load region info } // clone clones region store struct. func (r *RegionStore) clone() *RegionStore { return &RegionStore{ - workStoreIdx: r.workStoreIdx, - startingLineIdx: r.startingLineIdx, - stores: r.stores, + workStoreIdx: r.workStoreIdx, + stores: r.stores, + attemptAfterLoad: r.attemptAfterLoad, } } @@ -630,12 +631,17 @@ retry: } newRegionStore := regionStore.clone() newRegionStore.workStoreIdx = int32(newIdx) + newRegionStore.attemptAfterLoad++ + attemptOverThreshold := newRegionStore.attemptAfterLoad == reloadRegionThreshold + if attemptOverThreshold { + newRegionStore.attemptAfterLoad = 0 + } if !region.compareAndSwapStore(regionStore, newRegionStore) { goto retry } - // try all peers and fetch region info from pd again, before start next round. - if int32(newIdx) == regionStore.startingLineIdx { + // reload region info after attempts more than reloadRegionThreshold + if attemptOverThreshold { region.scheduleReload() } @@ -854,12 +860,11 @@ func (c *RegionCache) switchWorkStore(r *Region, targetStoreID uint64) (switchTo retry: // switch to new leader. oldRegionStore := r.getStore() - if oldRegionStore.workStoreIdx == int32(leaderIdx) && oldRegionStore.startingLineIdx == int32(leaderIdx) { + if oldRegionStore.workStoreIdx == int32(leaderIdx) { return } newRegionStore := oldRegionStore.clone() newRegionStore.workStoreIdx = int32(leaderIdx) - newRegionStore.startingLineIdx = int32(leaderIdx) if !r.compareAndSwapStore(oldRegionStore, newRegionStore) { goto retry } From cbfd25efac67f85c77878e4642b08611a2578aa4 Mon Sep 17 00:00:00 2001 From: lysu Date: Mon, 20 May 2019 11:06:53 +0800 Subject: [PATCH 26/27] address comment --- store/tikv/region_cache.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index a7c1f3def8e82..d0a03617907b9 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -599,7 +599,7 @@ retry: regionStore := region.getStore() cachedStore, cachedPeer, cachedIdx := region.WorkStorePeer(regionStore) - // almost time requests be directly routed to stable leader. + // Most of the time, requests are directly routed to stable leader. // returns if store is stable leader and no need retry other node. state := cachedStore.getState() if cachedStore != nil && state.failedAttempt == 0 && state.lastFailedTime == 0 { @@ -844,7 +844,7 @@ func (r *Region) EndKey() []byte { // switchWorkStore switches current store to the one on specific store. It returns // false if no peer matches the storeID. -func (c *RegionCache) switchWorkStore(r *Region, targetStoreID uint64) (switchToTarget bool) { +func (c *RegionCache) switchWorkStore(r *Region, targetStoreID uint64) { if len(r.meta.Peers) == 0 { return } @@ -853,7 +853,7 @@ func (c *RegionCache) switchWorkStore(r *Region, targetStoreID uint64) (switchTo for i, p := range r.meta.Peers { if p.GetStoreId() == targetStoreID { leaderIdx = i - switchToTarget = true + break } } From 6e9f6f4e38033dab975b1d1f56b7395a141004ca Mon Sep 17 00:00:00 2001 From: lysu Date: Mon, 20 May 2019 12:16:48 +0800 Subject: [PATCH 27/27] NotLeader's leader maybe miss in current cache kv return NotLeader before EpochNotMatch --- store/tikv/region_cache.go | 10 ++++++++-- store/tikv/region_cache_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/store/tikv/region_cache.go b/store/tikv/region_cache.go index d0a03617907b9..537113696b36d 100644 --- a/store/tikv/region_cache.go +++ b/store/tikv/region_cache.go @@ -447,7 +447,12 @@ func (c *RegionCache) UpdateLeader(regionID RegionVerID, leaderStoreID uint64) { zap.Uint64("leaderStoreID", leaderStoreID)) return } - c.switchWorkStore(r, leaderStoreID) + if !c.switchWorkStore(r, leaderStoreID) { + logutil.Logger(context.Background()).Debug("regionCache: cannot find peer when updating leader", + zap.Uint64("regionID", regionID.GetID()), + zap.Uint64("leaderStoreID", leaderStoreID)) + r.invalidate() + } } // insertRegionToCache tries to insert the Region to cache. @@ -844,7 +849,7 @@ func (r *Region) EndKey() []byte { // switchWorkStore switches current store to the one on specific store. It returns // false if no peer matches the storeID. -func (c *RegionCache) switchWorkStore(r *Region, targetStoreID uint64) { +func (c *RegionCache) switchWorkStore(r *Region, targetStoreID uint64) (switchToTarget bool) { if len(r.meta.Peers) == 0 { return } @@ -853,6 +858,7 @@ func (c *RegionCache) switchWorkStore(r *Region, targetStoreID uint64) { for i, p := range r.meta.Peers { if p.GetStoreId() == targetStoreID { leaderIdx = i + switchToTarget = true break } } diff --git a/store/tikv/region_cache_test.go b/store/tikv/region_cache_test.go index 7c9abf24afd20..1542d937f65d4 100644 --- a/store/tikv/region_cache_test.go +++ b/store/tikv/region_cache_test.go @@ -195,6 +195,33 @@ func (s *testRegionCacheSuite) TestUpdateLeader(c *C) { c.Assert(s.getAddr(c, []byte("z")), Equals, s.storeAddr(s.store2)) } +func (s *testRegionCacheSuite) TestUpdateLeader2(c *C) { + loc, err := s.cache.LocateKey(s.bo, []byte("a")) + c.Assert(err, IsNil) + // new store3 becomes leader + store3 := s.cluster.AllocID() + peer3 := s.cluster.AllocID() + s.cluster.AddStore(store3, s.storeAddr(store3)) + s.cluster.AddPeer(s.region1, store3, peer3) + // tikv-server reports `NotLeader` + s.cache.UpdateLeader(loc.Region, store3) + + // Store3 does not exist in cache, causes a reload from PD. + r := s.getRegion(c, []byte("a")) + c.Assert(r, NotNil) + c.Assert(r.GetID(), Equals, s.region1) + c.Assert(s.getAddr(c, []byte("a")), Equals, s.storeAddr(s.store1)) + + // tikv-server notifies new leader to pd-server. + s.cluster.ChangeLeader(s.region1, peer3) + // tikv-server reports `NotLeader` again. + s.cache.UpdateLeader(r.VerID(), store3) + r = s.getRegion(c, []byte("a")) + c.Assert(r, NotNil) + c.Assert(r.GetID(), Equals, s.region1) + c.Assert(s.getAddr(c, []byte("a")), Equals, s.storeAddr(store3)) +} + func (s *testRegionCacheSuite) TestUpdateLeader3(c *C) { loc, err := s.cache.LocateKey(s.bo, []byte("a")) c.Assert(err, IsNil)