yangyichao-mango commented on pull request #2965:
URL: 
https://github.com/apache/incubator-dolphinscheduler/pull/2965#issuecomment-645923391


   > > > @yangyichao-mango What's purpose of this PR? Why are there so many 
irrelevant files to change ?
   > > 
   > > 
   > > Thx a lot for your review @gabrywu .
   > > This PR is essentially to get the abnormal exit value caused by the 
error (exceptions caused by permissions, etc.) when the worker starts to create 
the working directory, to judge if the user info and work dir is created 
successfully.
   > > But after my change, I found that if the worker failed to create the 
working directory, the exception it threw would be caught, only recorded in the 
log of the worker node, not in the log of a single task instance. Although the 
task will fail at the end of execution, the exception log is very few and it is 
difficult to locate the exception (permission exception, etc.) directly.
   > > In my opinion, I understand that this error should fail fast, or at 
least recorded by logs. So in order to match the change of my exitval judgment 
work, I put the initialization change of tasklogger to the beginning of 
`TaskExecuteProcessor`. And I understand that the task logger should record all 
the logs related to the task, not only the task runtime logs.
   > > If I have any misunderstanding, please give me suggestions.
   > > 非常感谢你的review。
   > > 这个pr本质上是为了获取在worker执行task时创建工作目录时的错误(由于权限导致的问题等)值,来判断工作目录,用户权限是否创建成功。
   > > 
但是在改动完成后,我发现如果worker创作工作目录失败其抛出的异常会被catch住,异常信息只会在worker节点的日志中被记录下来,不会在单个task的日志中记录下来,这相当于创建工作目录相关的所有错误信息完全不会被直接外显给用户。虽然最后task执行的时候会失败,但是异常日志非常少,很难直接定位到工作目录中的异常(权限异常等)。
   > > 
我理解这种错误应该fail-fast,或者至少有日志来记录,所以为了配合我这次针对exitval判断的改动,我就把taskLogger的初始化改动到了最前面,用来记录涉及到这个task的所有日志。并且我理解taskLogger的原本目的就是用来记录所有涉及到这个task的日志,不仅仅是task运行时的日志。
   > > 如果我有理解不正确的地方,敬请指出。
   > 
   > You'd better give a suitable title to this issue, otherwise others may 
have a misunderstanding. If you want to fix different issue, different PRs 
should be created
   
   Thx a lot for your review @gabrywu .
   I've change the description of this issue and PR and connect the [FEATURE] 
#3014 and [BUG] #2925.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to