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/03/10 09:02:55 UTC

[GitHub] [dolphinscheduler] guoshupei opened a new issue #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

guoshupei opened a new issue #8810:
URL: https://github.com/apache/dolphinscheduler/issues/8810


   ### 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
   
   In `EventExecuteService`class, If an exception is throw in the `onSuccess` method, then `eventHandlerMap`  (the dev branch is `multiThreadFilterMap`)  cannot remove the thread of  `WorkflowExecuteThread` normally, which has two effects: 1)The `WorkflowExecuteThread` will not handle any events, so processInstance state does not change; 2. There is a risk of memory leak. 
   
   Also, I found that exceptions in the `onSuccess` method do not print out logs, making it difficult to locate the problem
   
   ![image](https://user-images.githubusercontent.com/29919212/157623159-30f6aa2b-0864-4e08-90d6-5a01246984c5.png)
   
   > 
   
   ### What you expected to happen
   
   The state of `ProcessInstance`changes normally
   
   ### How to reproduce
   
   In the `onSuccess` method throw an exception, can reproduce.
   
   Note: 
   1. You need to print `eventHandlerMap`and will find that `WorkflowExecuteThread` is always held and will not be removed
   2. You won't see any exception messages in the master log
   
   ### Anything else
   
   _No response_
   
   ### Version
   
   2.0.1
   
   ### 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

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



[GitHub] [dolphinscheduler] guoshupei commented on issue #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

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


   > 
   
   sure, I agree with you.
   
   


-- 
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 #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

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


   Hi:
   * 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 subscribe to the developer's email,Mail subscription steps reference https://dolphinscheduler.apache.org/en-us/community/development/subscribe.html ,Then write the issue URL in the email content and send question to dev@dolphinscheduler.apache.org.


-- 
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] guoshupei commented on issue #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

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


   ok. @caishunfeng I use `try catch`, first, ensure `WorkflowExecuteThread`  can be deleted and continue to handle events; second, print exception message to locate problem 
   
   For now, I'm just printing out exception messages so you can locate problem.
   
   The following is my personal opinion. I have studied and found the errors are mainly in the `notifyProcessChanged` method. If `try catch` is used, although the cause of the error is located, the solution cost is high, such as `repeat running` to solve the problem, which wastes time and resources.
   if transfer to `command`, insert a command to resolve
   
   
   
   ![image](https://user-images.githubusercontent.com/29919212/157670893-570f15ad-dd03-4045-bc89-51e0142c1117.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] caishunfeng commented on issue #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

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


   > ok. @caishunfeng I use `try catch`, first, ensure `WorkflowExecuteThread` can be deleted and continue to handle events; second, print exception message to locate problem
   > 
   > For now, I'm just printing out exception messages so you can locate problem.
   > 
   > The following is my personal opinion. I have studied and found the errors are mainly in the `notifyProcessChanged` method. If `try catch` is used, although the cause of the error is located, the solution cost is high, such as `repeat running` to solve the problem, which wastes time and resources. if transfer to `command`, insert a command to resolve
   > 
   > ![image](https://user-images.githubusercontent.com/29919212/157670893-570f15ad-dd03-4045-bc89-51e0142c1117.png)
   
   I think add `try catch` and log if exception is OK. And it is better to add into `notifyProcessChanged`. What do you think?


-- 
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] guoshupei commented on issue #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

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


   > Hi @guoshupei you should checkout from the dev branch and fix it. If merge, we will cherry-pick to the next 2.0.x version. And I think you can submit a PR and link to this issue.
   
   Hi @caishunfeng I get it now, thanks


-- 
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 closed issue #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

Posted by GitBox <gi...@apache.org>.
caishunfeng closed issue #8810:
URL: https://github.com/apache/dolphinscheduler/issues/8810


   


-- 
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 #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

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


   Hi @guoshupei Good for your issue description of `What happened` and `How to reproduce`,  can you add your solution in description too?


-- 
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 #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

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


   > The following are the modifications of the `dev` branch,what do you think?
   
   Hi @guoshupei you should checkout from the dev branch and fix it. If merge, we will cherry-pick to the next 2.0.x version. And I think you can submit a PR and link to this issue.
   


-- 
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] guoshupei edited a comment on issue #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

Posted by GitBox <gi...@apache.org>.
guoshupei edited a comment on issue #8810:
URL: https://github.com/apache/dolphinscheduler/issues/8810#issuecomment-1065892141


   @caishunfeng sure, I agree with you.
   
   


-- 
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] guoshupei commented on issue #8810: [Bug] [Master] Process Instance state does not change correctly and may cause Memory Leak

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


   Hi @caishunfeng This problem is not only in version of 2.0.1, but also exists in other versions. I need to submit the PR of this version or the `dev` version?
   
   The following are the modifications of the `dev` branch,what do you think?
   ![image](https://user-images.githubusercontent.com/29919212/158098529-4a6b0f24-8c61-4d28-888f-393d505c6912.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