[Fix-17613] [Master] Task group queue priority always remains 0 #17614
Conversation
KwongHing
requested review from
SbloodyS,
caishunfeng and
ruanwenjun
as code owners
SbloodyS
added
bug
.../apache/dolphinscheduler/server/master/engine/task/runnable/AbstractTaskInstanceFactory.java
Show resolved
Hide resolved
ruanwenjun
left a comment
ruanwenjun
left a comment
There was a problem hiding this comment.
Good catch, this bug also exist at cloneTaskInstance, please fix this.
And you can add test case at WorkflowStartTestCase, you can add assertion attestStartWorkflow_with_oneSuccessTaskUsingTaskGroup.
ruanwenjun
force-pushed
the
dev_zhongguangxing_fix17613
branch
from
acbaf9b to
ee18208
Compare
|
@SbloodyS @ruanwenjun all done |
SbloodyS
left a comment
SbloodyS
left a comment
There was a problem hiding this comment.
Please run mvn spotless:apply to format the code. @KwongHing
Done. |
...r-master/src/test/java/org/apache/dolphinscheduler/server/master/integration/Repository.java
Outdated
Show resolved
Hide resolved
|
We also need to add IT case for retry task instance can clone the queue priority |
|
PTAL @ruanwenjun |
|
This is still unaddressed. |
.../java/org/apache/dolphinscheduler/server/master/integration/cases/WorkflowStartTestCase.java
Show resolved
Hide resolved
...duler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupQueueMapper.xml
Outdated
Show resolved
Hide resolved
...dao/src/main/java/org/apache/dolphinscheduler/dao/repository/impl/TaskGroupQueueDaoImpl.java
Outdated
Show resolved
Hide resolved
...cheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/repository/TaskGroupQueueDao.java
Outdated
Show resolved
Hide resolved
...scheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TaskGroupQueueMapper.java
Outdated
Show resolved
Hide resolved
.../test/resources/it/start/workflow_with_one_fake_task_failed_with_retry_using_task_group.yaml
Outdated
Show resolved
Hide resolved
|
|
|
||
| /** | ||
| * Query all {@link TaskGroupQueue} belonging to the specified workflow instance. | ||
| * | ||
| * @param workflowInstanceId workflowInstanceId | ||
| * @return TaskGroupQueue | ||
| */ | ||
| List<TaskGroupQueue> queryByWorkflowInstanceId(Integer workflowInstanceId); | ||
|
|
There was a problem hiding this comment.
it's better to remove this method.
.../java/org/apache/dolphinscheduler/server/master/integration/cases/WorkflowStartTestCase.java
Show resolved
Hide resolved
| repository.queryAllInQueueTaskGroupQueue().forEach(taskGroupQueue -> { | ||
| taskGroupQueueMap.put(taskGroupQueue.getId(), taskGroupQueue); | ||
| }); |
There was a problem hiding this comment.
It's better to remove this assertion, this might easily failure. If we want to do this assertion we should avoid delete this in db in ci.
3 participants
Purpose of the pull request
close #17613
Brief change log
Pass
taskDefinition.taskGroupPrioritytotaskInstance.taskGroupPriorityto restore the task group queue’s sequential scheduling capability.Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
Pull Request Notice
Pull Request Notice
If your pull request contains incompatible change, you should also add it to
docs/docs/en/guide/upgrade/incompatible.md