-
Notifications
You must be signed in to change notification settings - Fork 219
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
region cache: check if the pd returned regions covers the ranges #1377
Changes from 8 commits
d7d994c
ac636f8
8f9515c
36f7d6f
81f2a7c
6f07898
a1ced8d
ca52cb7
6a0d73e
f6688eb
3339583
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2154,7 +2154,12 @@ func (c *RegionCache) scanRegions(bo *retry.Backoffer, startKey, endKey []byte, | |
metrics.RegionCacheCounterWithScanRegionsOK.Inc() | ||
|
||
if len(regionsInfo) == 0 { | ||
return nil, errors.Errorf("PD returned no region, limit: %d", limit) | ||
backoffErr = errors.Errorf("PD returned no region, limit: %d", limit) | ||
continue | ||
} | ||
if regionsHaveGapInRanges([]pd.KeyRange{{StartKey: startKey, EndKey: endKey}}, regionsInfo, limit) { | ||
backoffErr = errors.Errorf("PD returned regions have gaps, limit: %d", limit) | ||
continue | ||
} | ||
return c.handleRegionInfos(bo, regionsInfo, true) | ||
} | ||
|
@@ -2210,15 +2215,78 @@ func (c *RegionCache) batchScanRegions(bo *retry.Backoffer, keyRanges []pd.KeyRa | |
|
||
metrics.RegionCacheCounterWithBatchScanRegionsOK.Inc() | ||
if len(regionsInfo) == 0 { | ||
return nil, errors.Errorf( | ||
backoffErr = errors.Errorf( | ||
"PD returned no region, range num: %d, limit: %d", | ||
len(keyRanges), limit, | ||
) | ||
continue | ||
} | ||
if regionsHaveGapInRanges(keyRanges, regionsInfo, limit) { | ||
backoffErr = errors.Errorf( | ||
"PD returned regions have gaps, range num: %d, limit: %d", | ||
len(keyRanges), limit, | ||
) | ||
continue | ||
} | ||
return c.handleRegionInfos(bo, regionsInfo, opt.needRegionHasLeaderPeer) | ||
} | ||
} | ||
|
||
// regionsHaveGapInRanges checks if the loaded regions can fully cover the key ranges. | ||
// If there are any gaps between the regions, it returns true, then the requests might be retried. | ||
// TODO: remove this function after PD client supports gap detection and handling it. | ||
func regionsHaveGapInRanges(ranges []pd.KeyRange, regionsInfo []*pd.Region, limit int) bool { | ||
if len(ranges) == 0 { | ||
return false | ||
} | ||
if len(regionsInfo) == 0 { | ||
return true | ||
} | ||
checkIdx := 0 // checked index of ranges | ||
checkKey := ranges[0].StartKey // checked key of ranges | ||
for _, r := range regionsInfo { | ||
if r.Meta == nil { | ||
return true | ||
} | ||
if len(r.Meta.StartKey) != 0 && bytes.Compare(r.Meta.StartKey, checkKey) > 0 { | ||
// there is a gap between returned region's start_key and current check key | ||
return true | ||
} | ||
if len(r.Meta.EndKey) == 0 { | ||
cfzjywxk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// the current region contains all the rest ranges. | ||
return false | ||
} | ||
checkKey = r.Meta.EndKey | ||
for len(ranges[checkIdx].EndKey) > 0 && bytes.Compare(checkKey, ranges[checkIdx].EndKey) >= 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not very familiar with the usage of it. Is the ranges here guaranteed to be already sorted by their start key? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the input ranges are sorted and cannot be overlapped. |
||
// the end_key of returned region can cover multi ranges. | ||
checkIdx++ | ||
if checkIdx == len(ranges) { | ||
// all ranges are covered. | ||
return false | ||
} | ||
} | ||
if bytes.Compare(checkKey, ranges[checkIdx].StartKey) < 0 { | ||
cfzjywxk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// if check_key < start_key, move it forward to start_key. | ||
checkKey = ranges[checkIdx].StartKey | ||
} | ||
} | ||
if limit > 0 && len(regionsInfo) == limit { | ||
// the regionsInfo is limited by the limit, so there may be some ranges not covered. | ||
// But the previous regions are continuous, so we just need to check the rest ranges. | ||
return false | ||
} | ||
if checkIdx < len(ranges)-1 { | ||
// there are still some ranges not covered. | ||
return true | ||
} | ||
if len(checkKey) == 0 { | ||
return false | ||
} else if len(ranges[checkIdx].EndKey) == 0 { | ||
return true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible the end key of the last region is also empty, should the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, sorry, it still looks fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Half unbounded cases added. |
||
} | ||
return bytes.Compare(checkKey, ranges[checkIdx].EndKey) < 0 | ||
} | ||
|
||
func (c *RegionCache) batchScanRegionsFallback(bo *retry.Backoffer, keyRanges []pd.KeyRange, limit int, opts ...BatchLocateKeyRangesOpt) ([]*Region, error) { | ||
logutil.BgLogger().Warn("batch scan regions fallback to scan regions", zap.Int("range-num", len(keyRanges))) | ||
res := make([]*Region, 0, len(keyRanges)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The left half of the condition seems unnecessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.