reele commented on code in PR #15289:
URL:
https://github.com/apache/dolphinscheduler/pull/15289#discussion_r1418508338
##########
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:
> In this case B.B1 in scheduleTime 2023-12-03 will find A.A1 in
2023-11-30's processInstance through queryLastManualProcessInterval.
At that time will got 2 objects: `lastSchedulerProcess(== null)`, and
`lastManualProcess(!= null)` so function `findLastProcessInterval` will return
`lastManualProcess` at
https://github.com/apache/dolphinscheduler/blob/0bb48f342973663b3466934701eea79aabcec6b3/dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/utils/DependentExecute.java#L330-L332
A.A1 is a task, B.B1 depends on a normal task code, not a subprocess, when
A.A1 in `2023-11-30` succeed, that will goes into
`dependResultBySingleTaskInstance`
even if goes into `dependResultByProcessInstance`, the processInstance(`A`
in `2023-11-30`) parameter is already finished and got a SUCCESS result.
So am I right? At least I have verified this situation myself
--
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]
