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

*: add region option for random pick region #1005

Merged
merged 4 commits into from
Mar 27, 2018

Conversation

nolouch
Copy link
Contributor

@nolouch nolouch commented Mar 26, 2018

same as the title.

server/cache.go Outdated
@@ -279,10 +279,10 @@ func (c *clusterInfo) getRegions() []*core.RegionInfo {
return c.Regions.GetRegions()
}

func (c *clusterInfo) randomRegion() *core.RegionInfo {
func (c *clusterInfo) randomRegion(ops ...core.RegionOption) *core.RegionInfo {
Copy link
Member

Choose a reason for hiding this comment

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

use opts is less confusing。

@disksing
Copy link
Contributor

Can we abstract 'has leader on store x' and 'has follower on store x' as options?

if len(region.DownPeers) > 0 || len(region.PendingPeers) > 0 {
return false
}
return true
Copy link
Contributor

Choose a reason for hiding this comment

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

return len(downPeers) == 0 && len(pendingPeers) == 0

server/cache.go Outdated
@@ -324,17 +324,17 @@ func (c *clusterInfo) getStoreLeaderCount(storeID uint64) int {
}

// RandLeaderRegion returns a random region that has leader on the store.
func (c *clusterInfo) RandLeaderRegion(storeID uint64) *core.RegionInfo {
func (c *clusterInfo) RandLeaderRegion(storeID uint64, ops ...core.RegionOption) *core.RegionInfo {
Copy link
Member

Choose a reason for hiding this comment

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

s/ops/opts

server/cache.go Outdated
}

// RandFollowerRegion returns a random region that has a follower on the store.
func (c *clusterInfo) RandFollowerRegion(storeID uint64) *core.RegionInfo {
func (c *clusterInfo) RandFollowerRegion(storeID uint64, ops ...core.RegionOption) *core.RegionInfo {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@@ -60,9 +60,9 @@ func (c *namespaceCluster) checkRegion(region *core.RegionInfo) bool {
const randRegionMaxRetry = 10

// RandFollowerRegion returns a random region that has a follower on the store.
func (c *namespaceCluster) RandFollowerRegion(storeID uint64) *core.RegionInfo {
func (c *namespaceCluster) RandFollowerRegion(storeID uint64, ops ...core.RegionOption) *core.RegionInfo {
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

@@ -74,9 +74,9 @@ func (c *namespaceCluster) RandFollowerRegion(storeID uint64) *core.RegionInfo {
}

// RandLeaderRegion returns a random region that has leader on the store.
func (c *namespaceCluster) RandLeaderRegion(storeID uint64) *core.RegionInfo {
func (c *namespaceCluster) RandLeaderRegion(storeID uint64, ops ...core.RegionOption) *core.RegionInfo {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@@ -146,13 +146,13 @@ func (bc *BasicCluster) UnblockStore(storeID uint64) {
}

// RandFollowerRegion returns a random region that has a follower on the store.
func (bc *BasicCluster) RandFollowerRegion(storeID uint64) *core.RegionInfo {
return bc.Regions.RandFollowerRegion(storeID)
func (bc *BasicCluster) RandFollowerRegion(storeID uint64, ops ...core.RegionOption) *core.RegionInfo {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

}

// RandLeaderRegion returns a random region that has leader on the store.
func (bc *BasicCluster) RandLeaderRegion(storeID uint64) *core.RegionInfo {
return bc.Regions.RandLeaderRegion(storeID)
func (bc *BasicCluster) RandLeaderRegion(storeID uint64, ops ...core.RegionOption) *core.RegionInfo {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Member

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

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

rest LGTM

Copy link
Contributor

@disksing disksing left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

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

LGTM

@Connor1996 Connor1996 merged commit 2374eb4 into tikv:master Mar 27, 2018
@nolouch nolouch deleted the region-option branch September 8, 2021 07:22
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