server, schedule: limiter record operator count group by store #1051
Conversation
Connor1996
changed the title
|
please add a description of the PR. |
server/coordinator.go
Outdated
| @@ -520,7 +518,7 @@ func (c *coordinator) removeOperator(op *schedule.Operator) { | |||
| func (c *coordinator) removeOperatorLocked(op *schedule.Operator) { | |||
| regionID := op.RegionID() | |||
| delete(c.operators, regionID) | |||
| c.limiter.UpdateCounts(c.operators) | |||
| c.limiter.RemoveOperator(op) | |||
There was a problem hiding this comment.
refer to #874, we meet some not synchronized cases so change to updatecounts.
There was a problem hiding this comment.
I have noticed that. But now we have a background goroutine to check all operators, all path will reach RemoveOperator() or AddOperator(). it's unlikely not synchronized.
There was a problem hiding this comment.
I vote for update limiter using all operators too.
server/coordinator.go
Outdated
| } else { | ||
| c.addOperators(op...) | ||
| } | ||
| if ops := s.Schedule(c.cluster, opInfluence); ops != nil { |
There was a problem hiding this comment.
ops != nil -> len(ops) > 0.
server/coordinator.go
Outdated
| } | ||
|
|
||
| operatorCounter.WithLabelValues(op.Desc(), "create").Inc() | ||
| return true | ||
| log.Warnf("add operator %v on nonexistent region %d", op, regionID) |
server/schedule/operator.go
Outdated
| @@ -36,6 +36,7 @@ type OperatorStep interface { | |||
| fmt.Stringer | |||
| IsFinish(region *core.RegionInfo) bool | |||
| Influence(opInfluence OpInfluence, region *core.RegionInfo) | |||
| InvolvedStores(region *core.RegionInfo) []uint64 | |||
There was a problem hiding this comment.
It's kind weird to have a region parameter here. I think you can add a OriginLeader field to operators that need original leader id to return affect stores.
server/coordinator.go
Outdated
| @@ -520,7 +518,7 @@ func (c *coordinator) removeOperator(op *schedule.Operator) { | |||
| func (c *coordinator) removeOperatorLocked(op *schedule.Operator) { | |||
| regionID := op.RegionID() | |||
| delete(c.operators, regionID) | |||
| c.limiter.UpdateCounts(c.operators) | |||
| c.limiter.RemoveOperator(op) | |||
There was a problem hiding this comment.
I vote for update limiter using all operators too.
limiter records operator count grouping by store