Mrhs121 commented on issue #17798:
URL: 
https://github.com/apache/dolphinscheduler/issues/17798#issuecomment-3658554942

   > > > Solution 1: Extreme Simplification
   > > > The page should allow free workgroup assignment, but the workflow 
workgroups and scheduled job workgroups should be
   > > > combined and displayed during the echo.
   > > 
   > > 
   > > Current workgroup permissions and project binding, which violates the 
design of permission management.
   > > > Solution 2: Complexity
   > > > When assigning workgroups on the page, let's assume the new workgroup 
is A and the project workgroup is B.
   > > > If the difference between A and B is greater than 0, directly add a 
new workgroup based on the difference.
   > > > If the difference between B and A is greater than 0 (including cases 
where A is empty), check if the workgroups in the
   > > > difference have already been assigned to workflows or scheduled jobs. 
If not, they can be deleted; if already assigned, they
   > > > cannot be deleted.
   > > 
   > > 
   > > What do you mean by `If the difference between A and B is greater than 
0, directly add a new workgroup based on the difference`? I'm a little 
confused. Please provide an example.
   > 
   > For example , the new workgroup A now has [a,b,c,d] ,and the project 
workgroup now has [b,c,d,f] so difference between A and B is A-B = [a] ,is 
greater than 0, we can directly add the group [a] into the project workgroup so 
difference between B and A is B-A = [f] ,is greater than 0, we can directly 
delete group [f] when it doesn't belong to a workflow or scheduled job. if [f] 
has been assigned, deletion is not allowed and an exception should be displayed.
   
   The logic you mentioned about this addition and deletion workergroup should 
be fine in the current code logic
   `B-A = [f] `
   
https://github.com/apache/dolphinscheduler/blob/8c4d921c63ff4d91e50fc4549adfe50bee4464f5/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectWorkerGroupRelationServiceImpl.java#L128-L129
   `A-B = [a]`
   
https://github.com/apache/dolphinscheduler/blob/8c4d921c63ff4d91e50fc4549adfe50bee4464f5/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectWorkerGroupRelationServiceImpl.java#L150-L151
   
   
   
   
   The main problem lies here,  For instance, I clear all the workers under the 
current project from the page and then click Submit.  
   (In this sample project, there is only a shell task, and this shell task is 
assigned to workergroup `new`)
   <img width="1473" height="599" alt="Image" 
src="https://github.com/user-attachments/assets/e392872b-a563-4413-aa38-f7ff206909d9";
 />
   
   Because we clicked "Clear", the workerGroups passed by the front end through 
the api are an empty collection. Therefore, it will go to this if branch. It 
can be seen that before deletion, no judgment was made on whether the worker 
groups to be deleted could be deleted, but they were directly deleted from the 
database.
   
   
https://github.com/apache/dolphinscheduler/blob/8c4d921c63ff4d91e50fc4549adfe50bee4464f5/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectWorkerGroupRelationServiceImpl.java#L97-L105
   
   and  there is also a bit of a problem with the logic for determining whether 
there are tasks occupying the workergroup here. For instance, if 
usedWorkerGroups is [new] and needDeletedWorkerGroups is [default,new], it will 
be determined that no task is occupying the Workergroups to be deleted.  
   Here, intersection should be used to determine whether there is a task 
occupying this workergroup instead of containsAll. For example: `boolean 
hasIntersection =! Collections.disjoint(usedWorkerGroups, 
needDeletedWorkerGroups);
   `
   
https://github.com/apache/dolphinscheduler/blob/8c4d921c63ff4d91e50fc4549adfe50bee4464f5/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectWorkerGroupRelationServiceImpl.java#L132-L137
   


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

Reply via email to