[Improvement][Log] optimize task log path to reduce task running time by choucmei · Pull Request #6731 · apache/dolphinscheduler

choucmei · 2021-11-07T13:22:53Z

Purpose of the pull request

Reduce the creation of log directories
Consider that schedulerTime may be null,and taskExecutionContext does not have submitTime, so use firstSubmitTime as the task log path

Brief change log

Use firstSubmitTime as the task log path
TaskLogName: [taskAppId=TASK-798_1-4084-15210] -> [taskAppId=TASK-20211107-798_1-4084-15210]
TaskLogPath: /logs/defintion/code_defintion_version/processInstanceId/taskInstanceId.log->/logs/YYYYMMDD/defintion-code_defintion_version-processInstanceId-taskInstanceId.log

Verify this pull request

LoggerUtils,LogUtilsTest is already covered by existing tests,
others pass local test.

* Reduce the creation of log directories * Consider that schedulerTime may be null,and taskExecutionContext does not have submitTime, so use firstSubmitTime as the task log path

codecov-commenter · 2021-11-07T13:32:28Z

Codecov Report

Merging #6731 (4180dcd) into dev (9113817) will increase coverage by 0.03%.
The diff coverage is 75.00%.

@@             Coverage Diff              @@
##                dev    #6731      +/-   ##
============================================
+ Coverage     41.83%   41.87%   +0.03%     
- Complexity     3618     3622       +4     
============================================
  Files           641      641              
  Lines         25895    25922      +27     
  Branches       2795     2797       +2     
============================================
+ Hits          10833    10854      +21     
- Misses        14081    14085       +4     
- Partials        981      983       +2

Impacted Files	Coverage Δ
.../org/apache/dolphinscheduler/common/Constants.java	`79.16% <ø> (ø)`
...pache/dolphinscheduler/common/utils/DateUtils.java	`81.10% <ø> (ø)`
...che/dolphinscheduler/common/utils/LoggerUtils.java	`0.00% <0.00%> (ø)`
...ver/master/runner/task/ConditionTaskProcessor.java	`0.00% <0.00%> (ø)`
...ver/master/runner/task/DependentTaskProcessor.java	`0.00% <0.00%> (ø)`
...server/master/runner/task/SwitchTaskProcessor.java	`0.00% <0.00%> (ø)`
...eduler/server/worker/runner/TaskExecuteThread.java	`0.00% <0.00%> (ø)`
...phinscheduler/server/log/TaskLogDiscriminator.java	`87.50% <100.00%> (ø)`
...apache/dolphinscheduler/server/utils/LogUtils.java	`92.30% <100.00%> (+3.41%)`	⬆️
...rg/apache/dolphinscheduler/api/utils/PageInfo.java	`61.81% <0.00%> (+9.31%)`	⬆️

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 9113817...4180dcd. Read the comment docs.

lenboo · 2021-11-08T02:31:53Z

well done!
But there are some code style errors, can you fix them?

CalvinKirs · 2021-11-08T07:37:21Z

you can refer this doc :https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html

choucmei · 2021-11-08T14:03:14Z

well done!
But there are some code style errors, can you fix them?

Of course, this is my mistake.

* format code

kezhenxu94 · 2021-11-08T16:06:14Z

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java

+
+
+


Remove the extra blank lines and only keep one blank line

kezhenxu94 · 2021-11-08T16:11:53Z

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java

    public static final String YYYY_MM_DD_HH_MM_SS = "yyyy-MM-dd HH:mm:ss";


+


Remove the extra blank lines and only keep one blank line.

BTW, @zhongjiajie can you please set up a code style rule to only allow at most one blank line between member fields / methods?

👌 , would try today

kezhenxu94 · 2021-11-08T16:13:08Z

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/DateUtils.java

+    /**
+     * get specify date
+     */
+    public static Date getSpecifyDate(int year, int month, int date) {


Please, this method is only for test, it's not necessary to add this method to increase maintainers' burden, please just move it into the test.

* remove extra blank lines * remove unnecessary test function

zhongjiajie · 2021-11-11T02:50:02Z

Hi @choucmei code look good to me now, but unit test failed, could you take a look and fix it?

* modify UT

sonarqubecloud · 2021-11-11T07:01:22Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

61.9% Coverage
0.0% Duplication

zhongjiajie · 2021-11-11T07:14:14Z

@CalvinKirs Seem CI green, could you rebase again and merge it?

CalvinKirs

LGTM @lenboo PTAL

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/LoggerUtils.java

ruanwenjun

LGTM, I have test on standalone, it works well.

zhongjiajie · 2021-11-17T09:10:08Z

@CalvinKirs Sooooo fast! 🚀

davidzollo · 2021-11-18T10:39:27Z

good job

[Improvement][Log] optimize task log path to reduce task running time

cbea964

* Reduce the creation of log directories * Consider that schedulerTime may be null,and taskExecutionContext does not have submitTime, so use firstSubmitTime as the task log path

choucmei added 2 commits November 8, 2021 22:27

[Improvement][Log] optimize task log path to reduce task running time

7ba2d44

* format code

[Improvement][Log] optimize task log path to reduce task running time

6b69916

* format code

kezhenxu94 requested changes Nov 8, 2021

View reviewed changes

choucmei and others added 3 commits November 9, 2021 18:37

[Improvement][Log] optimize task log path to reduce task running time

5517ccc

* remove extra blank lines * remove unnecessary test function

Merge branch 'apache-dev' into dev

96cf233

Merge branch 'apache:dev' into dev

ea06646

CalvinKirs and others added 3 commits November 11, 2021 11:03

Merge branch 'dev' into dev

896c5ea

[Improvement][Log] optimize task log path to reduce task running time

46ef820

* modify UT

Merge remote-tracking branch 'origin/dev' into dev

4180dcd

CalvinKirs approved these changes Nov 17, 2021

View reviewed changes

CalvinKirs added this to the 2.0.1-release milestone Nov 17, 2021

ruanwenjun reviewed Nov 17, 2021

View reviewed changes

dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/LoggerUtils.java Show resolved Hide resolved

ruanwenjun approved these changes Nov 17, 2021

View reviewed changes

CalvinKirs merged commit f564687 into apache:dev Nov 17, 2021

CalvinKirs modified the milestones: 2.0.1-release, 2.0.2-release Nov 30, 2021

		public static final String YYYY_MM_DD_HH_MM_SS = "yyyy-MM-dd HH:mm:ss";

Conversation

choucmei commented Nov 7, 2021 • edited by lenboo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of the pull request

Brief change log

Verify this pull request

Uh oh!

codecov-commenter commented Nov 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lenboo commented Nov 8, 2021

Uh oh!

CalvinKirs commented Nov 8, 2021

Uh oh!

choucmei commented Nov 8, 2021

Uh oh!

kezhenxu94 Nov 8, 2021

Choose a reason for hiding this comment

Uh oh!

kezhenxu94 Nov 8, 2021

Choose a reason for hiding this comment

Uh oh!

zhongjiajie Nov 9, 2021

Choose a reason for hiding this comment

Uh oh!

kezhenxu94 Nov 8, 2021

Choose a reason for hiding this comment

Uh oh!

zhongjiajie commented Nov 11, 2021

Uh oh!

sonarqubecloud bot commented Nov 11, 2021

Uh oh!

zhongjiajie commented Nov 11, 2021

Uh oh!

CalvinKirs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

zhongjiajie commented Nov 17, 2021

Uh oh!

davidzollo commented Nov 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

choucmei commented Nov 7, 2021 •

edited by lenboo

Loading

codecov-commenter commented Nov 7, 2021 •

edited

Loading