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 hot region schedule #611

Merged
merged 34 commits into from
Apr 27, 2017
Merged

*: add hot region schedule #611

merged 34 commits into from
Apr 27, 2017

Conversation

nolouch
Copy link
Contributor

@nolouch nolouch commented Apr 11, 2017

now, collect region writes status from the heartbeat. randomly select a hot region in best write hot store then do some transfer.
cc @siddontang @disksing @zhangjinpeng1987

@@ -297,3 +302,107 @@ func (c *fifoCache) len() int {

return c.ll.Len()
}

type queueCache struct {
Copy link
Member

Choose a reason for hiding this comment

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

Can we abstract this to a LRUCache?

Copy link
Contributor

Choose a reason for hiding this comment

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

}

func (q *queueCache) Push(key interface{}, value interface{}) {
q.Lock()
Copy link
Member

Choose a reason for hiding this comment

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

defer q.Unlock()

return q.head == q.tail
}

func (q *queueCache) Push(key interface{}, value interface{}) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need return bool to indicate that if push success?

return
}
v := kvItem{key: key, value: value}
q.items[q.tail] = v
Copy link
Member

Choose a reason for hiding this comment

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

do we need check if q.tail is in not out of range?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I check is it full firstly.

return v.value
}

func (q *queueCache) Delete(key interface{}) {
Copy link
Member

Choose a reason for hiding this comment

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

Please add bool return value to indicate if deleted or not exist.

}

func (q *queueCache) Delete(key interface{}) {
q.Lock()
Copy link
Member

Choose a reason for hiding this comment

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

defer q.Unlock()


func (q *queueCache) Delete(key interface{}) {
q.Lock()
id, ok := q.index[key]
Copy link
Member

Choose a reason for hiding this comment

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

s/id/idx

func (m MetaRegionStatus) Less(i, j int) bool { return m[i].WriteBytes > m[j].WriteBytes }

type RegionHotStatus struct {
TotalWriteBytes uint64 `json:"total_write"`
Copy link
Member

Choose a reason for hiding this comment

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

What the difference between TotalWriteBytes and MetaStatus.WriteBytes?

type balanceHotRegionScheduler struct {
opt *scheduleOption
limit uint64
scoreStatus map[uint64]RegionHotStatus
Copy link
Member

Choose a reason for hiding this comment

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

Please add comment for this map, what the key is.

func newBalanceHotRegionScheduler(opt *scheduleOption) *balanceHotRegionScheduler {
return &balanceHotRegionScheduler{
opt: opt,
limit: 1,
Copy link
Member

Choose a reason for hiding this comment

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

Can we modify the limit?

Copy link
Member

Choose a reason for hiding this comment

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

use a const var

@@ -60,6 +60,10 @@ func createRouter(prefix string, svr *server.Server) *mux.Router {
router.HandleFunc("/api/v1/labels", labelsHandler.Get).Methods("GET")
router.HandleFunc("/api/v1/labels/stores", labelsHandler.GetStores).Methods("GET")

hotStatusHandler := newHotStatusHandler(svr, rd)
Copy link
Contributor

Choose a reason for hiding this comment

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

hot or hotspot?

Copy link
Contributor

Choose a reason for hiding this comment

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

/api/v1/hotspot/host
/api/v1/hotspot/store?

items := cluster.writeStatus.GetList()
for _, item := range items {
r := item.value.(RegionStateValue)
if r.UpdateTimes < 5 {
Copy link
Contributor

Choose a reason for hiding this comment

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

make this 5 a const?


type RegionStateValue struct {
RegionID uint64 `json:"region_id"`
WriteBytes uint64 `json:"write_bytes"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Written

@@ -297,3 +302,107 @@ func (c *fifoCache) len() int {

return c.ll.Len()
}

type queueCache struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

}

func (l *balanceHotRegionScheduler) clearScore() {
for sid, status := range l.scoreStatus {
Copy link
Member

Choose a reason for hiding this comment

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

I prefer clear the map directly.

return q.isFull()
}

func (q *hashCache) isFull() bool {
Copy link
Member

Choose a reason for hiding this comment

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

why there are two isFull?

Copy link
Contributor

Choose a reason for hiding this comment

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

isFullLocked

server/region.go Outdated
@@ -50,6 +51,7 @@ func (r *RegionInfo) clone() *RegionInfo {
Leader: proto.Clone(r.Leader).(*metapb.Peer),
DownPeers: downPeers,
PendingPeers: pendingPeers,
WriteBytes: r.WriteBytes,
Copy link
Member

Choose a reason for hiding this comment

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

WrittenBytes

Scheduler: s,
opt: c.opt,
limiter: c.limiter,
interval: minInterval,
Copy link
Member

Choose a reason for hiding this comment

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

Why there are two interval?

Copy link
Member

Choose a reason for hiding this comment

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

please add comments.

server/cache.go Outdated
res := make(map[uint64]uint64)
for _, s := range c.getStores() {
res[s.GetId()] = s.status.GetBytesWritten()
res[0] += s.status.GetBytesWritten()
Copy link
Member

Choose a reason for hiding this comment

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

remove this line. If you need total written bytes, please sum all the element in the map.

type RegionStateValue struct {
RegionID uint64 `json:"region_id"`
WriteBytes uint64 `json:"write_bytes"`
UpdateTimes int `json:"update_times"`
Copy link
Contributor

Choose a reason for hiding this comment

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

UpdateCount

UpdateTimes int `json:"update_times"`
LastUpdateTime time.Time `json:"last_update_time"`
StoreID uint64 `json:"-"`
antiTimes int
Copy link
Contributor

Choose a reason for hiding this comment

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

what is antiTimes?

@@ -297,3 +297,70 @@ func (c *fifoCache) len() int {

return c.ll.Len()
}

type hashCache struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

please us another PR and add test for this structure

q.Lock()
defer q.Unlock()
if q.isFull() {
log.Info("Debug queue is full")
Copy link
Contributor

Choose a reason for hiding this comment

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

better to return a full error.
Btw, what's Debug mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove those log later. just for observe in test now.

return item
}

func (q *hashCache) GetList() []interface{} {
Copy link
Contributor

Choose a reason for hiding this comment

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

/GetList/List/s

if !s.AllowSchedule() {
continue
}
if op := s.Schedule(c.cluster); op != nil {
c.addOperator(op)
}
if s.Scheduler.GetName() == "balance-hot-region-scheduler" {
Copy link
Contributor

Choose a reason for hiding this comment

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

use a const var

@disksing
Copy link
Contributor

LGTM.

@siddontang
Copy link
Contributor

PTAL @andelf @zhangjinpeng1987

Copy link
Contributor

@andelf andelf left a comment

Choose a reason for hiding this comment

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

LGTM

@CLAassistant
Copy link

CLAassistant commented Apr 26, 2017

CLA assistant check
All committers have signed the CLA.

@@ -60,6 +60,10 @@ func createRouter(prefix string, svr *server.Server) *mux.Router {
router.HandleFunc("/api/v1/labels", labelsHandler.Get).Methods("GET")
router.HandleFunc("/api/v1/labels/stores", labelsHandler.GetStores).Methods("GET")

hotStatusHandler := newHotStatusHandler(handler, rd)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please let pd-ctl support this later.

status, ok := h.scoreStatus[storeID]
if !ok {
status = &StoreHotRegions{
RegionsStat: make(RegionsStat, 0, 100),
Copy link
Contributor

Choose a reason for hiding this comment

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

use const for 100

// the hottest region in the store not move
// randomly pick a region from 1 .. length-1
// TODO: consider hot degree when pick
rr := h.r.Int31n(int32(length-1)) + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

seem you can use Intn(length - 1) + 1


avgRegionCount := hotRegionTotalCount / float64(len(h.scoreStatus))
// Multiplied by 0.75 to avoid transfer back and forth
limit := uint64((float64(s.RegionsStat.Len()) - avgRegionCount) * 0.75)
Copy link
Contributor

Choose a reason for hiding this comment

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

define a const var for 0.75

@siddontang
Copy link
Contributor

Rest LGTM


writeStatLRUMaxLen = 1000
storeHotRegionsDefaultLen = 100
hotRegionLimitCoeff = 0.75
Copy link
Contributor

Choose a reason for hiding this comment

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

factor may be better than coeff?

@siddontang siddontang merged commit 103c0ad into master Apr 27, 2017
@siddontang siddontang deleted the shuning/hot-region-schedule branch April 27, 2017 12:20
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.

7 participants