imbajin commented on code in PR #336:
URL: 
https://github.com/apache/incubator-hugegraph-computer/pull/336#discussion_r2422693337


##########
vermeer/apps/master/bl/scheduler_bl.go:
##########
@@ -72,81 +225,161 @@ func (s *ScheduleBl) QueueTask(taskInfo 
*structure.TaskInfo) (bool, error) {
                return false, errors.New("the property `SpaceName` of taskInfo 
is empty")
        }
 
-       //defer s.Unlock(s.Lock())
+       defer s.Unlock(s.Lock())
        if err := taskMgr.SetState(taskInfo, structure.TaskStateWaiting); err 
!= nil {
                return false, err
        }
 
+       logrus.Debugf("queuing task %d with parameters: %+v", taskInfo.ID, 
taskInfo)
+

Review Comment:
   **Incomplete validation of preorder dependencies**
   
   The code checks if the dependency task exists, but doesn't verify:
   1. Whether the dependency task is in a valid state (completed, running, 
waiting)
   2. Circular dependency detection
   3. Whether dependencies are in the same space/worker group
   
   **Suggested improvements:**
   ```go
   if len(taskInfo.Preorders) > 0 {
       visited := make(map[int32]bool)
       for _, depTaskID := range taskInfo.Preorders {
           // Check existence
           depTask := taskMgr.GetTaskByID(depTaskID)
           if depTask == nil {
               err := fmt.Errorf("dependency task with ID %d does not exist", 
depTaskID)
               logrus.Error(err)
               taskMgr.SetError(taskInfo, err.Error())
               return false, err
           }
           
           // Check for circular dependencies
           if err := s.checkCircularDependency(taskInfo.ID, depTaskID, 
visited); err \!= nil {
               logrus.Error(err)
               taskMgr.SetError(taskInfo, err.Error())
               return false, err
           }
           
           // Validate dependency state
           if depTask.State == structure.TaskStateError || depTask.State == 
structure.TaskStateCanceled {
               err := fmt.Errorf("dependency task %d is in invalid state: %s", 
depTaskID, depTask.State)
               logrus.Error(err)
               taskMgr.SetError(taskInfo, err.Error())
               return false, err
           }
       }
   }
   ```



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to