server/operator: add operator state #647
Conversation
server/operator.go
Outdated
| // OperatorUnKnownState indicates the unknown state | ||
| OperatorUnKnownState OperatorState = iota | ||
| // OperatorDoing indicates the doing state | ||
| OperatorDoing |
There was a problem hiding this comment.
Or Wait as the initial state, after the operator is sent to tikv, update to Running.
server/operator.go
Outdated
| if ok { | ||
| return s | ||
| } | ||
| return operatorStateToName[0] |
There was a problem hiding this comment.
/0/OperatorUnKnownState/s
server/operator.go
Outdated
| if !ok { | ||
| *o = OperatorUnKnownState | ||
| } | ||
| *o = state |
There was a problem hiding this comment.
should in else closure.
server/operator.go
Outdated
| } | ||
|
|
||
| func newAdminOperator(region *RegionInfo, ops ...Operator) *adminOperator { | ||
| return &adminOperator{ | ||
| Region: region, | ||
| Start: time.Now(), | ||
| Ops: ops, | ||
| State: OperatorDoing, |
server/operator.go
Outdated
| "unknown": OperatorUnKnownState, | ||
| "doing": OperatorDoing, | ||
| "finished": OperatorFinished, | ||
| "time_out": OperatorTimeOut, |
server/operator_test.go
Outdated
| c.Assert(err, IsNil) | ||
| c.Assert(newState, Equals, s) | ||
| } | ||
| } |
There was a problem hiding this comment.
use an invalid marshaled data to check Unmarshal.
server/coordinator.go
Outdated
| @@ -409,3 +411,7 @@ func collectOperatorCounterMetrics(op Operator) { | |||
| operatorCounter.WithLabelValues(label).Add(float64(value)) | |||
| } | |||
| } | |||
|
|
|||
| func collectOperatorStateCounterMetrics(op Operator) { | |||
| operatorCounter.WithLabelValues(op.GetState().String()).Add(1) | |||
There was a problem hiding this comment.
I think it is better to use labels (operator, state) here.
server/operator.go
Outdated
| // OperatorUnKnownState indicates the unknown state | ||
| OperatorUnKnownState OperatorState = iota | ||
| // OperatorDoing indicates the doing state | ||
| OperatorDoing |
server/coordinator.go
Outdated
| @@ -260,6 +261,7 @@ func (c *coordinator) removeOperator(op Operator) { | |||
| delete(c.operators, regionID) | |||
|
|
|||
| c.histories.add(regionID, op) | |||
| collectOperatorStateCounterMetrics(op) | |||
There was a problem hiding this comment.
If the operator is replaced, its status will still be Doing.
server/coordinator.go
Outdated
| @@ -222,22 +222,26 @@ func (c *coordinator) runScheduler(s *scheduleController) { | |||
|
|
|||
| func (c *coordinator) addOperator(op Operator) bool { | |||
| c.Lock() | |||
| defer c.Unlock() | |||
There was a problem hiding this comment.
the function removeOperator also need to access lock
There was a problem hiding this comment.
How about add another func called removeOperatorWithLock?
server/event.go
Outdated
| return OperatorFinished | ||
| } | ||
|
|
||
| func (op *splitOperator) SetState(state OperatorState) { |
| } | ||
|
|
||
| func (op *regionOperator) SetState(state OperatorState) { | ||
| if op.State == OperatorFinished || op.State == OperatorTimeOut { |
There was a problem hiding this comment.
can we meet the case that operator is finished or timeout, but the state is still set?
There was a problem hiding this comment.
if one operator combined by multi-operator, some operator in it may be finished or timeout, and now we replace this operator.
|
any test to check operator state? |
|
LGTM PTAL @disksing |
|
LGTM. |
PTAL @disksing @siddontang