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

*: store balance weight. #713

Merged
merged 9 commits into from Sep 1, 2017

Conversation

Projects
None yet
5 participants
@disksing
Member

disksing commented Aug 29, 2017

No description provided.

@disksing disksing requested review from siddontang, nolouch and huachaohuang Aug 29, 2017

}
func newStoreInfo(store *metapb.Store) *StoreInfo {
return &StoreInfo{
Store: store,
Store: store,
LeaderWeight: 1.0,

This comment has been minimized.

@zhangjinpeng1987

zhangjinpeng1987 Aug 29, 2017

Contributor

How about set default value as 100?

@zhangjinpeng1987

zhangjinpeng1987 Aug 29, 2017

Contributor

How about set default value as 100?

This comment has been minimized.

@disksing

disksing Aug 29, 2017

Member

If the weight is unset, the score will be the same as old score, because newScore = oldScore / weight.

@disksing

disksing Aug 29, 2017

Member

If the weight is unset, the score will be the same as old score, because newScore = oldScore / weight.

@@ -169,8 +180,20 @@ func (kv *kv) loadStores(stores *storesInfo, rangeLimit int64) error {
return errors.Trace(err)
}
storeInfo := newStoreInfo(store)
leaderWeight, err := kv.loadFloatWithDefaultValue(kv.storeLeaderWeightPath(storeInfo.GetId()), 1.0)

This comment has been minimized.

@huachaohuang

huachaohuang Aug 30, 2017

Member

Consider extracting a loadStoreWeight and test them.

@huachaohuang

huachaohuang Aug 30, 2017

Member

Consider extracting a loadStoreWeight and test them.

return errors.Trace(errStoreNotFound(storeID))
}
if err := c.s.kv.saveStoreWeight(storeID, leader, region); err != nil {

This comment has been minimized.

@huachaohuang

huachaohuang Aug 31, 2017

Member

Consider moving this inside clusterInfo.putStore.

@huachaohuang

huachaohuang Aug 31, 2017

Member

Consider moving this inside clusterInfo.putStore.

This comment has been minimized.

@disksing

disksing Aug 31, 2017

Member

clusterInfo.putStore has other callers, SetStoreLabel, DeleteStore, etc. They won't change the weight, so I think put saveStoreWeight outside is more suitable.

@disksing

disksing Aug 31, 2017

Member

clusterInfo.putStore has other callers, SetStoreLabel, DeleteStore, etc. They won't change the weight, so I think put saveStoreWeight outside is more suitable.

@huachaohuang

This comment has been minimized.

Show comment
Hide comment
@huachaohuang

huachaohuang Aug 31, 2017

Member

Will this affect BalanceHotRegionScheduler?
If one of the stores has a larger weight, it may contain more hot regions than others, will it fight with the BalanceLeaderScheduler and BalanceRegionScheduler?

Member

huachaohuang commented Aug 31, 2017

Will this affect BalanceHotRegionScheduler?
If one of the stores has a larger weight, it may contain more hot regions than others, will it fight with the BalanceLeaderScheduler and BalanceRegionScheduler?

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Aug 31, 2017

Member

If BalanceLeaderScheduler and BalanceRegionScheduler try to balance a hot region, it will conflict with BalanceHotRegionScheduler. It is not directly related to store weight, they have conflicts with or without store weight. We resolve it by setting up higher priorities for operators created by BalanceHotRegionScheduler.

Member

disksing commented Aug 31, 2017

If BalanceLeaderScheduler and BalanceRegionScheduler try to balance a hot region, it will conflict with BalanceHotRegionScheduler. It is not directly related to store weight, they have conflicts with or without store weight. We resolve it by setting up higher priorities for operators created by BalanceHotRegionScheduler.

@huachaohuang

This comment has been minimized.

Show comment
Hide comment
@huachaohuang

huachaohuang Aug 31, 2017

Member

Maybe BalanceHotRegionScheduler need to consider the store weight too?
Otherwise, BalanceHotRegionScheduler may keep trying to transfer leaders from a store with a larger weight to a store with a smaller weight, while BalanceLeaderScheduler may keep trying to transfer leaders backward, will that happen?

Member

huachaohuang commented Aug 31, 2017

Maybe BalanceHotRegionScheduler need to consider the store weight too?
Otherwise, BalanceHotRegionScheduler may keep trying to transfer leaders from a store with a larger weight to a store with a smaller weight, while BalanceLeaderScheduler may keep trying to transfer leaders backward, will that happen?

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Aug 31, 2017

Member

There are too many hacks in hotRegionScheduler. Seems more easy to skip hot regions in BalanceLeaderScheduler and BalanceRegionScheduler. Any second thought?

Member

disksing commented Aug 31, 2017

There are too many hacks in hotRegionScheduler. Seems more easy to skip hot regions in BalanceLeaderScheduler and BalanceRegionScheduler. Any second thought?

@huachaohuang

This comment has been minimized.

Show comment
Hide comment
@huachaohuang

huachaohuang Sep 1, 2017

Member

I still think that BalanceHotRegionScheduler should consider store weight too, maybe we can find a more general way to make them work well together later.
LGTM

Member

huachaohuang commented Sep 1, 2017

I still think that BalanceHotRegionScheduler should consider store weight too, maybe we can find a more general way to make them work well together later.
LGTM

@nolouch

nolouch approved these changes Sep 1, 2017

LGTM. maybe we can add hot region weight later.

disksing added some commits Sep 1, 2017

@disksing disksing merged commit fd35f50 into master Sep 1, 2017

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@disksing disksing deleted the disksing/store-weight branch Sep 1, 2017

@keroro520

This comment has been minimized.

Show comment
Hide comment
@keroro520

keroro520 Apr 8, 2018

Hi. Could anyone explain the actual meaning of Weight and Score, and the relation between them ?

keroro520 commented Apr 8, 2018

Hi. Could anyone explain the actual meaning of Weight and Score, and the relation between them ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment