Skip to content

remove unnecessary lock & fix syncWorkerHostWeight variable reference#2734

Merged
wen-hemin merged 5 commits intoapache:devfrom
gabry-lab:removeUnnecessaryLock
Jun 15, 2020
Merged

remove unnecessary lock & fix syncWorkerHostWeight variable reference#2734
wen-hemin merged 5 commits intoapache:devfrom
gabry-lab:removeUnnecessaryLock

Conversation

@gabry-lab
Copy link
Copy Markdown
Member

@gabry-lab gabry-lab commented May 18, 2020

What is the purpose of the pull request

  1. remove unnecessary lock to ConcurrentHashMap
  2. fix wrong variable reference inside LowerWeightHostManager.syncWorkerHostWeight method

Brief change log

  • optimize LowerWeightHostManager.java

@codecov-io
Copy link
Copy Markdown

codecov-io commented May 18, 2020

Codecov Report

Merging #2734 into dev will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #2734      +/-   ##
============================================
+ Coverage     37.17%   37.19%   +0.01%     
  Complexity     2553     2553              
============================================
  Files           434      434              
  Lines         19977    19972       -5     
  Branches       2423     2423              
============================================
+ Hits           7427     7429       +2     
+ Misses        11882    11878       -4     
+ Partials        668      665       -3
Impacted Files Coverage Δ Complexity Δ
...r/master/dispatch/host/LowerWeightHostManager.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...org/apache/dolphinscheduler/remote/utils/Host.java 25.71% <0.00%> (-5.72%) 5.00% <0.00%> (-1.00%)
...e/dolphinscheduler/remote/NettyRemotingClient.java 52.51% <0.00%> (+1.43%) 10.00% <0.00%> (+1.00%)
...he/dolphinscheduler/common/thread/ThreadUtils.java 74.60% <0.00%> (+3.17%) 14.00% <0.00%> (ø%)

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 1771aa5...1864f87. Read the comment docs.

@qiaozhanwei qiaozhanwei requested a review from Technoboy- May 19, 2020 02:22
@gabry-lab gabry-lab requested a review from wen-hemin May 22, 2020 05:08
@gabry-lab
Copy link
Copy Markdown
Member Author

gabry-lab commented May 22, 2020

@rubik-w could you help to review this PR ? Thanks a lot

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 30, 2020 •
edited
Loading

Codecov Report

Merging #2734 into dev will decrease coverage by 0.14%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #2734      +/-   ##
============================================
- Coverage     39.55%   39.40%   -0.15%     
+ Complexity     2709     2701       -8     
============================================
  Files           435      435              
  Lines         20121    20116       -5     
  Branches       2453     2453              
============================================
- Hits           7958     7927      -31     
- Misses        11406    11432      +26     
  Partials        757      757
Impacted Files Coverage Δ Complexity Δ
...r/master/dispatch/host/LowerWeightHostManager.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../server/worker/processor/TaskExecuteProcessor.java 10.16% <0.00%> (-27.12%) 1.00% <0.00%> (-2.00%)
...nscheduler/server/entity/TaskExecutionContext.java 89.10% <0.00%> (-9.91%) 61.00% <0.00%> (-5.00%)
...he/dolphinscheduler/common/thread/ThreadUtils.java 68.25% <0.00%> (-6.35%) 13.00% <0.00%> (-1.00%)
...inscheduler/common/task/sqoop/SqoopParameters.java 74.00% <0.00%> (-2.00%) 25.00% <0.00%> (ø%)

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 a58d1b3...f532853. Read the comment docs.

wen-hemin
wen-hemin approved these changes Jun 13, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@wen-hemin wen-hemin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@gabry-lab
Copy link
Copy Markdown
Member Author

gabry-lab commented Jun 13, 2020

+1

If you approved and nobody will deny, so could you please merge this PR ?

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jun 15, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_252) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@wen-hemin wen-hemin merged commit 24c7f76 into apache:dev Jun 15, 2020
@gabry-lab gabry-lab deleted the removeUnnecessaryLock branch June 15, 2020 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wen-hemin wen-hemin wen-hemin approved these changes

@Technoboy- Technoboy- Awaiting requested review from Technoboy-

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@gabry-lab @codecov-io @codecov-commenter @wen-hemin @davidzollo