Codecov Report

Merging #2965 into dev will increase coverage by 0.05%.
The diff coverage is 33.33%.

@@             Coverage Diff              @@
##                dev    #2965      +/-   ##
============================================
+ Coverage     39.11%   39.17%   +0.05%     
- Complexity     2678     2682       +4     
============================================
  Files           435      435              
  Lines         20119    20161      +42     
  Branches       2454     2459       +5     
============================================
+ Hits           7870     7898      +28     
- Misses        11514    11525      +11     
- Partials        735      738       +3     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a7fd72...f36af53. Read the comment docs.

Hi @gabrywu @dailidong , Can you help to review this PR? Thx a lot.

@yangyichao-mango What's purpose of this PR? Why are there so many irrelevant files to change ?

@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运行时的日志。

如果我有理解不正确的地方，敬请指出。

@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

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

What is the function of this ThreadLocal? Is it necessary? I think we changed our log format and implemented it with loggerutils

What is the function of this ThreadLocal? Is it necessary? I think we changed our log format and implemented it with loggerutils

Thx a lot for your review @dailidong .
In my opinion, it's better to set the tasklogger threadlocal value to log the task detail but not to pass the tasklogger through the function's input params in the utils class of FileUtils and OSUtils.

What is the function of this ThreadLocal? Is it necessary? I think we changed our log format and implemented it with loggerutils

Thx a lot for your review and work of merging dev branch to current branch @dailidong .
In my opinion, although my branch is out-of-date with the base branch, maybe it's better to rebase the current branch and it will make the dev branch cleaner. If it's needed, I will rebase my branch after your approval this PR and reply me.

I think no need to rebase if no confilct files

I think no need to rebase if no confilct files

Thx a lot for your review. @dailidong

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
2 Code Smells

49.5% Coverage
0.0% Duplication

Hello, there is still a problem under the modified logic:
If you copy a resource file and there is FileNotFoundException (the file is in the resource list, but not in HDFS), you will still not see the relevant error message in the task log of the Web UI, making it difficult to troubleshoot the problem

@yangyichao-mango

您好，在现在修改后的逻辑下，还存在一个问题是：
如果在copy资源文件时发生java.io.FileNotFoundException（文件在资源列表中，但是在hdfs上不存在的特殊情况下），那么在web UI的task日志中还是看不到相关错误信息，不便于排查问题