yangwwei commented on a change in pull request #216:
URL:
https://github.com/apache/incubator-yunikorn-core/pull/216#discussion_r563348013
##########
File path: pkg/scheduler/partition.go
##########
@@ -1099,3 +1099,15 @@ func (pc *PartitionContext) removeAllocationAsk(appID
string, allocationKey stri
}
}
}
+
+func (pc *PartitionContext) GetCurrentState() string {
+ return pc.stateMachine.Current()
+}
+
+func (pc *PartitionContext) GetStateTime() time.Time {
+ return pc.stateTime
+}
+
+func (pc *PartitionContext) GetNodeSortingPolicy() *policies.NodeSortingPolicy
{
+ return pc.nodeSortingPolicy
+}
Review comment:
I think we need to add a read lock for `GetStateTime()`, and the write
lock in `handlePartitionEvent()`. because `handlePartitionEvent()` and the REST
API could be called at the same time.
```
func (pc *PartitionContext) handlePartitionEvent(event objects.ObjectEvent)
error {
err := pc.stateMachine.Event(event.String(), pc.Name)
if err == nil {
pc.Lock()
pc.stateTime = time.Now()
pc.Unlock()
return nil
}
// handle the same state transition not nil error (limit of fsm).
if err.Error() == "no transition" {
return nil
}
return err
}
```
`GetCurrentState()` and `GetNodeSortingPolicy()` should be safe because
state machine Current() holds the lock already, and the nodeSortingPolicy only
gets modified when updating the config, that is a swapping of the old object.
##########
File path: pkg/scheduler/partition.go
##########
@@ -262,6 +262,8 @@ func (pc *PartitionContext) isStopped() bool {
// Handle the state event for the partition.
// The state machine handles the locking.
func (pc *PartitionContext) handlePartitionEvent(event objects.ObjectEvent)
error {
+ pc.Lock()
+ defer pc.Unlock()
Review comment:
Can we move this to line 269?
I think we do not need to lock the partition when calling
`pc.stateMachine.Event(event.String(), pc.Name)`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
