reele commented on code in PR #15289:
URL:
https://github.com/apache/dolphinscheduler/pull/15289#discussion_r1418549556
##########
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:
Yes, I understand the reasons for the former, in my opinion:
findLastProcessInterval returns lastManualProcess, which is run through
complementation (A in '20231130'), because its `startTime` is 2023-12-03.
'queryLastManualProcessInterval' will capture it, so the returned
lastManualProcess is not null.
We may have subjective differences in opinion, so I will reproduce the
process later using the dev version, along with the relevant logs...
--
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]
