reele commented on code in PR #15289:
URL:
https://github.com/apache/dolphinscheduler/pull/15289#discussion_r1418280002
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/utils/DependentExecute.java:
##########
@@ -318,21 +316,12 @@ private void addItemVarPool(String varPoolStr, Long
endTime) {
*/
private ProcessInstance findLastProcessInterval(Long definitionCode,
DateInterval dateInterval, int testFlag) {
- ProcessInstance lastSchedulerProcess =
-
processInstanceDao.queryLastSchedulerProcessInterval(definitionCode,
dateInterval, testFlag);
+ // Task instance cannot run without process instance,
+ // we should only use `scheduleTime` to search for process instances,
+ // as `dateInterval` is calculated based on the `scheduleTime` of the
+ // process instance where the `dependent` node is located.
- ProcessInstance lastManualProcess =
-
processInstanceDao.queryLastManualProcessInterval(definitionCode, dateInterval,
testFlag);
-
- if (lastManualProcess == null) {
- return lastSchedulerProcess;
- }
- if (lastSchedulerProcess == null) {
- return lastManualProcess;
- }
-
- // In the time range, there are both manual and scheduled workflow
instances, return the last workflow instance
- return lastManualProcess.getId() > lastSchedulerProcess.getId() ?
lastManualProcess : lastSchedulerProcess;
Review Comment:
For example:
There are two workflows, A and B.
In workflow `A`, there is a task A1.
In workflow `B`, there are B1 and B2. B1 is a dependency node that relies on
`A.A1` of the `today`. The preceding task of B2 is B1.
Assumption:
Workflows `A` and `B` are scheduled to run on three specific dates:
`20231201`, `20231202`, and `20231203`. So, their execution times are also on
`20231201`, `20231202`, and `20231203` respectively.
However, due to some reasons, `A.A1` did not complete execution on all three
days. As a result, the tasks for `B.B1` on all three days remain in a waiting
state.
Assuming today is `20231203`:
Due to other reasons, a manual run of `A`'s backfill was executed with a
complementary (scheduling) date of `20231130`. At this point, all currently
running `B.B1` dependency nodes invoking `queryLastManualProcessInterval` will
detect and succeed through `startTime` by finding instances of `A.A1` on the
complementary (scheduling) date of `20231130`. However, in reality, `A.A1` did
not complete on `20231201`, `20231202`, and `20231203`.
| `A.A1` scheduleTime | `A.A1` startTime |
for `B.B1` at scheduleTime `2023-12-03 (${system.biz.date} == '2023-12-02')` |
| ------------------------------------------ | ---------- |
------------------------------------------------------------ |
| 2023-12-03 `(${system.biz.date} == '2023-12-02')` | |
`queryLastSchedulerProcessInterval` waiting... |
| 2023-11-30 `(${system.biz.date} == '2023-11-29')` | 2023-12-03 |
`queryLastManualProcessInterval` wrong detected!!! |
then 'B.B1' at scheduleTime `2023-12-03 (${system.biz.date} ==
'2023-12-02')` will got wrong data.
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/utils/DependentExecute.java:
##########
@@ -318,21 +316,12 @@ private void addItemVarPool(String varPoolStr, Long
endTime) {
*/
private ProcessInstance findLastProcessInterval(Long definitionCode,
DateInterval dateInterval, int testFlag) {
- ProcessInstance lastSchedulerProcess =
-
processInstanceDao.queryLastSchedulerProcessInterval(definitionCode,
dateInterval, testFlag);
+ // Task instance cannot run without process instance,
+ // we should only use `scheduleTime` to search for process instances,
+ // as `dateInterval` is calculated based on the `scheduleTime` of the
+ // process instance where the `dependent` node is located.
- ProcessInstance lastManualProcess =
-
processInstanceDao.queryLastManualProcessInterval(definitionCode, dateInterval,
testFlag);
-
- if (lastManualProcess == null) {
- return lastSchedulerProcess;
- }
- if (lastSchedulerProcess == null) {
- return lastManualProcess;
- }
-
- // In the time range, there are both manual and scheduled workflow
instances, return the last workflow instance
- return lastManualProcess.getId() > lastSchedulerProcess.getId() ?
lastManualProcess : lastSchedulerProcess;
Review Comment:
Oh it's passed from `system.biz.date` built-in parameter, generated through
`scheduleTime`
I have modified the previous description.
--
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]
