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

Health check for stores #203

Merged
merged 6 commits into from
Jul 24, 2016
Merged

Conversation

huachaohuang
Copy link
Contributor

Filter out unknown and down stores from balance.

@@ -127,7 +128,7 @@ func (cb *capacityBalancer) ScoreType() scoreType {
// selectBalanceRegion tries to select a store leader region to do balance and returns true, but if we cannot find any,
// we try to find a store follower region and returns false.
func (cb *capacityBalancer) selectBalanceRegion(cluster *clusterInfo, stores []*storeInfo) (*metapb.Region, *metapb.Peer, *metapb.Peer, bool) {
store := selectFromStore(stores, cluster.getUnknownStores(), cb.filters, cb.st)
store := selectFromStore(stores, nil, cb.filters, cb.st)
Copy link
Contributor

Choose a reason for hiding this comment

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

why nil now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unknown stores will be filtered out in stateFilter, no need to maintain another unknown stores map now.

Copy link
Contributor

Choose a reason for hiding this comment

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

so need to remove this arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some places still need this arg, we may remove it after we do some refactor, but not in this PR.

@huachaohuang huachaohuang force-pushed the huachaohuang/health-check-store branch from 17a69d8 to 5f86b01 Compare July 22, 2016 09:12
@@ -187,6 +187,10 @@ type BalanceConfig struct {

// MaxTransferWaitCount is the max heartbeat count to wait leader transfer to finish.
MaxTransferWaitCount uint64 `toml:"max-transfer-wait-count" json:"max-transfer-wait-count"`

// MaxStoreDownInterval is the max interval (in seconds)
Copy link
Contributor

Choose a reason for hiding this comment

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

@queenypingcap please help to check comments.

@huachaohuang huachaohuang force-pushed the huachaohuang/health-check-store branch from 208d4f7 to 9302f40 Compare July 23, 2016 05:14
@@ -49,6 +49,7 @@ var (
maxBalanceRetryPerLoop = flag.Uint64("max-balance-retry-per-loop", 10, "the max retry count to balance in a balance schedule")
maxBalanceCountPerLoop = flag.Uint64("max-balance-count-per-loop", 3, "the max region count to balance in a balance schedule")
maxTransferWaitCount = flag.Uint64("max-transfer-wait-count", 3, "the max heartbeat count to wait leader transfer to finish")
maxStoreDownDuration = flag.Uint64("max-store-down-duration", 60, "the max duration a store without heartbeat will be considered to be down")
Copy link
Contributor

Choose a reason for hiding this comment

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

@queenypingcap please review the explanation.

@siddontang
Copy link
Contributor

LGTM

@huachaohuang huachaohuang changed the title [DNM] Health check for stores Health check for stores Jul 23, 2016
@QueenyJin
Copy link

LGTM

1 similar comment
@qiuyesuifeng
Copy link
Contributor

LGTM

@huachaohuang huachaohuang merged commit aa6dfd9 into master Jul 24, 2016
@huachaohuang huachaohuang deleted the huachaohuang/health-check-store branch July 24, 2016 04:52
rleungx pushed a commit to rleungx/pd that referenced this pull request Dec 1, 2023
Signed-off-by: guo-shaoge <shaoge1994@163.com>
rleungx pushed a commit to rleungx/pd that referenced this pull request Dec 1, 2023
rleungx pushed a commit to rleungx/pd that referenced this pull request Dec 1, 2023
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.

4 participants