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

pd-ctl: list all regions in a specified store #1231

Merged
merged 7 commits into from
Sep 4, 2018

Conversation

rleungx
Copy link
Member

@rleungx rleungx commented Sep 3, 2018

What problem does this PR solve?

Closes #511.

What is changed and how it works?

This PR adds API to list all regions in a specified store. After this PR is merged, we can use region store <id> to print all regions in a specified store by using pd-ctl.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)

@rleungx rleungx changed the title pd-client: list all regions in a specified store pd-ctl list all regions in a specified store Sep 3, 2018
@rleungx rleungx changed the title pd-ctl list all regions in a specified store pd-ctl: list all regions in a specified store Sep 3, 2018
// GetStoreRegions gets all RegionInfo with a given storeID
func (r *RegionsInfo) GetStoreRegions(storeID uint64) []*RegionInfo {
var regions []*RegionInfo
for _, region := range r.regions.m {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think better to iterate leaders and followers.

@disksing
Copy link
Contributor

disksing commented Sep 3, 2018

The rest LGTM.

@disksing
Copy link
Contributor

disksing commented Sep 3, 2018

Oh, don't forget to update api.raml file.

Copy link
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -630,6 +630,22 @@ func (r *RegionsInfo) GetRegions() []*RegionInfo {
return regions
}

// GetStoreRegions gets all RegionInfo with a given storeID
func (r *RegionsInfo) GetStoreRegions(storeID uint64) []*RegionInfo {
var regions []*RegionInfo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we pre-allocates the slice with capacity?

var regions []*RegionInfo
if leaders, ok := r.leaders[storeID]; ok {
for _, region := range leaders.m {
regions = append(regions, region.Clone())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems no need to Clone any more after the refactor of RegionInfo. /cc @nolouch

@disksing disksing merged commit 3e749b0 into tikv:master Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants