You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/08/26 13:10:48 UTC

[GitHub] [dolphinscheduler] DarkAssassinator opened a new issue, #11669: [Bug] [TaskInstance] the value of duration in Workflow Instance table is wrong

DarkAssassinator opened a new issue, #11669:
URL: https://github.com/apache/dolphinscheduler/issues/11669

   ### Search before asking
   
   - [X] I had searched in the [issues](https://github.com/apache/dolphinscheduler/issues?q=is%3Aissue) and found no similar issues.
   
   
   ### What happened
   
   When the workflow instance is in the process of executing, the result should be empty, regardless of whether the workflow instance 
    was run for the first time or re-run. 
   But when we re-run the workflow instance , the status show instance is running, but the ***duration*** is displayed as `(the task re-run start time) - (the last task end time)`, which is obviously wrong.
   
   Because the workflow instance page will keep query the data from api - ProcessInstanceController-queryProcessInstanceList, the source code just use **_Math.abs(startTime - endTime)_**. 
   
   Please check the following code:
   `for (ProcessInstance processInstance : processInstances) {
               processInstance.setDuration(
                       DateUtils.format2Duration(processInstance.getStartTime(), processInstance.getEndTime()));
               ...
           }`
   
   
   ### What you expected to happen
   
   When the workflow instanceis running, the duration should be null,
   
   ### How to reproduce
   
   Step 1. Create a workflow instance and run it.
   Step 2. After the workflow instance finished, then click "re-run" button, and check the duration when the state show executing.
   
   ### Anything else
   
   Need to determine in which workflow instance state this duration needs to be empty, i think just **_RUNNING_EXECUTION_** and **_SERIAL_WAIT_**. 
   
   ### Version
   
   dev
   
   ### Are you willing to submit PR?
   
   - [X] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
   


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] caishunfeng commented on issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong

Posted by GitBox <gi...@apache.org>.
caishunfeng commented on issue #11669:
URL: https://github.com/apache/dolphinscheduler/issues/11669#issuecomment-1229109213

   Hi @DarkAssassinator Now in ds, if a workflow instance re-run or failover, it will use the original workflow instance but not create a new one, so we keep the first start time as startTime and just update the endTime.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] DarkAssassinator commented on issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong

Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on issue #11669:
URL: https://github.com/apache/dolphinscheduler/issues/11669#issuecomment-1229178671

   Hi @caishunfeng thank u for ur reply, I'm also talking about the original record on workflow instance page. I agree with your design, but it's not actually implemented that way. There are two problems that are completely different from this design. Please check the following screenshot:
   _org.apache.dolphinscheduler.service.process.ProcessServiceImpl.constructProcessInstance_
   ![image](https://user-images.githubusercontent.com/20518339/187028602-16ac5a89-f65e-42ee-80ee-d878571edfee.png)
   When the _**MasterSchedulerBootstrap**_ thread gets the command from the db and converts it to the process instance, it will update the process instance to db, including the startTime and endTime fields. So there are two issues.
   **Issue 1: The start time of this workflow record will be refreshed every time it is re-run.** It cannot keep the first time as startTime.
   **Issue 2: update endTime to null cannot take affect.** because Mybatis plus updateById will skip the null params (FieldStrategy.NOT_NULL is default update strategy). We should set the FieldStrategy to IGNORED for the ProcessInstance.endTime, please review the following change:
   `@TableField(updateStrategy = FieldStrategy.IGNORED)
       private Date endTime;`
   
   


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] caishunfeng commented on issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong

Posted by GitBox <gi...@apache.org>.
caishunfeng commented on issue #11669:
URL: https://github.com/apache/dolphinscheduler/issues/11669#issuecomment-1229361987

   > Hi @caishunfeng thank u for ur reply, I'm also talking about the original record on workflow instance page. I agree with your design, but it's not actually implemented that way. There are two problems that are completely different from this design. Please check the following screenshot: _org.apache.dolphinscheduler.service.process.ProcessServiceImpl.constructProcessInstance_ ![image](https://user-images.githubusercontent.com/20518339/187028602-16ac5a89-f65e-42ee-80ee-d878571edfee.png) When the _**MasterSchedulerBootstrap**_ thread gets the command from the db and converts it to the process instance, it will update the process instance to db, including the startTime and endTime fields. So there are two issues. **Issue 1: The start time of this workflow record will be refreshed every time it is re-run.** It cannot keep the first time as startTime. **Issue 2: update endTime to null cannot take affect.** because Mybatis plus updateById will skip the null params (FieldStrategy.NOT_NU
 LL is default update strategy). We should set the FieldStrategy to IGNORED for the ProcessInstance.endTime, please review the following change: `@TableField(updateStrategy = FieldStrategy.IGNORED) private Date endTime;`
   
   @DarkAssassinator Thanks for your reply. I think `re-run` is different from `recovery failure` or `recovery pause`, it's better to create a new workflow instance with the name such as `workflowName-2`, and then it can keep the old one. 
   What do you think? cc @lenboo @ruanwenjun 


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] DarkAssassinator commented on issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong

Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on issue #11669:
URL: https://github.com/apache/dolphinscheduler/issues/11669#issuecomment-1229382955

   Hi @caishunfeng but i think `re-run` should keep only one workflow instance, because semantically speaking, re-run is indeed the same task. I think we just need to determine the assignment logic of the two field values of startTime and Duration.
   We need to decide if the start time should be the first start time or the last start time, and the duation should be the last duration or the sum of each duration. 
   I prefer the values for both start time and duration to represent the values from the last run, because it will create some new task instance when we re-run the workflow instance. If users want to the hostory records, the users can check the task instance records. thx


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] ruanwenjun closed issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong

Posted by GitBox <gi...@apache.org>.
ruanwenjun closed issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong
URL: https://github.com/apache/dolphinscheduler/issues/11669


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] github-actions[bot] commented on issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #11669:
URL: https://github.com/apache/dolphinscheduler/issues/11669#issuecomment-1228478272

   Thank you for your feedback, we have received your issue, Please wait patiently for a reply.
   * In order for us to understand your request as soon as possible, please provide detailed information、version or pictures.
   * If you haven't received a reply for a long time, you can [join our slack](https://s.apache.org/dolphinscheduler-slack) and send your question to channel `#troubleshooting`


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] ruanwenjun commented on issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on issue #11669:
URL: https://github.com/apache/dolphinscheduler/issues/11669#issuecomment-1263026071

   > > Hi @caishunfeng thank u for ur reply, I'm also talking about the original record on workflow instance page. I agree with your design, but it's not actually implemented that way. There are two problems that are completely different from this design. Please check the following screenshot: _org.apache.dolphinscheduler.service.process.ProcessServiceImpl.constructProcessInstance_ ![image](https://user-images.githubusercontent.com/20518339/187028602-16ac5a89-f65e-42ee-80ee-d878571edfee.png) When the _**MasterSchedulerBootstrap**_ thread gets the command from the db and converts it to the process instance, it will update the process instance to db, including the startTime and endTime fields. So there are two issues. **Issue 1: The start time of this workflow record will be refreshed every time it is re-run.** It cannot keep the first time as startTime. **Issue 2: update endTime to null cannot take affect.** because Mybatis plus updateById will skip the null params (FieldStrategy.NOT_
 NULL is default update strategy). We should set the FieldStrategy to IGNORED for the ProcessInstance.endTime, please review the following change: `@TableField(updateStrategy = FieldStrategy.IGNORED) private Date endTime;`
   > 
   > @DarkAssassinator Thanks for your reply. I think `re-run` is different from `recovery failure` or `recovery pause`, it's better to create a new workflow instance with the name such as `workflowName-2`, and then it can keep the old one. What do you think? cc @lenboo @ruanwenjun
   
   +1.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] DarkAssassinator commented on issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong

Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on issue #11669:
URL: https://github.com/apache/dolphinscheduler/issues/11669#issuecomment-1229179578

   In addition, based on this question, I think a judgment should be added, the start time must be guaranteed to be earlier than the end time, otherwise the Duration value should be set to null. But I don't know if it's because of what design needs to use Math.abs(start time, end time).
   ![image](https://user-images.githubusercontent.com/20518339/187029243-63047050-6808-4c1d-8aaf-666f45b31f52.png)
   


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [dolphinscheduler] github-actions[bot] commented on issue #11669: [Bug] [WorkflowInstance] the value of duration in Workflow Instance table is wrong

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #11669:
URL: https://github.com/apache/dolphinscheduler/issues/11669#issuecomment-1260239582

   This issue has been automatically marked as stale because it has not had recent activity for 30 days. It will be closed in next 7 days if no further activity occurs.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org