Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

store/tikv: fix misuse of PD client's GetStore #23695

Merged
merged 7 commits into from
Mar 31, 2021
Merged
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions store/tikv/region_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"math/rand"
"strings"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -1768,6 +1769,12 @@ func (s *Store) initResolve(bo *Backoffer, c *RegionCache) (addr string, err err
}
}

// A quick and dirty solution to find out whether an err is caused by StoreNotFound.
// todo: A better solution, maybe some err-code based error handling?
func isStoreNotFoundError(err error) bool {
return strings.Contains(err.Error(), "invalid store ID") && strings.Contains(err.Error(), "not found")
}

// reResolve try to resolve addr for store that need check. Returns false if the region is in tombstone state or is
// deleted.
func (s *Store) reResolve(c *RegionCache) (bool, error) {
Expand All @@ -1778,16 +1785,20 @@ func (s *Store) reResolve(c *RegionCache) (bool, error) {
} else {
metrics.RegionCacheCounterWithGetStoreOK.Inc()
}
if err != nil {
// `err` here can mean either "load Store from PD failed" or "store not found"
// If load Store from PD is successful but PD didn't find the store
// the err should be handled by next `if` instead of here
if err != nil && !isStoreNotFoundError(err) {
logutil.BgLogger().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 false, err
}
if store == nil || store.State == metapb.StoreState_Tombstone {
longfangsong marked this conversation as resolved.
Show resolved Hide resolved
if store == nil {
// store has be removed in PD, we should invalidate all regions using those store.
logutil.BgLogger().Info("invalidate regions in removed store",
zap.Uint64("store", s.storeID), zap.String("add", s.addr))
atomic.AddUint32(&s.epoch, 1)
atomic.StoreUint64(&s.state, uint64(deleted))
metrics.RegionCacheCounterWithInvalidateStoreRegionsOK.Inc()
return false, nil
}
Expand Down