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
api: add region stats. #840
Conversation
server/core/region.go
Outdated
// Observe adds a region's statistics into RegionStats. | ||
func (s *RegionStats) Observe(r *RegionInfo) { | ||
s.Count++ | ||
if r.ApproximateSize <= 1 { |
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.
why 1 here?
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.
Regions that have approximate size less than 1MB is record as 1.
See: https://github.com/pingcap/pd/blob/96db717124a5bd980e8a244af2db8952e04159d6/server/grpc_service.go#L325
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.
oh, I think we should the size should be more accurate, maybe KB is better.
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.
As @zhangjinpeng1987 suggested, tikv's calculation of size is not accurate in the first place, using MB will reduce inaccuracy relatively.
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.
please define a const var
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.
please define a const var
LGTM |
func (r *RegionsInfo) GetRegionStats(startKey, endKey []byte) *RegionStats { | ||
stats := newRegionStats() | ||
r.tree.scanRange(startKey, func(meta *metapb.Region) bool { | ||
if len(endKey) > 0 && (len(meta.EndKey) == 0 || bytes.Compare(meta.EndKey, endKey) >= 0) { |
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.
how about to move "len(endKey) > 0" out of scanRange
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.
Here endKey==""
means never break and scan to the last region, seems impossible to move out.
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.
LGTM
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.
Rest LGTM
Fix #832
Fix #817