[improve-10506] [server] add MasterHeartBeat and WorkerHeartBeat to extend HeartBeat by guoshupei · Pull Request #10563 · apache/dolphinscheduler

guoshupei · 2022-06-23T03:27:23Z

Add MasterHeartBeat to module of dolphinscheduler-master
Add WorkerHeartBeat to module of dolphinscheduler-worker

this closes #10506

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

… Improvement-10506

ruanwenjun · 2022-06-23T13:47:35Z

...duler-service/src/main/java/org/apache/dolphinscheduler/service/registry/RegistryClient.java

        List<Server> serverList = new ArrayList<>();
        for (Map.Entry<String, String> entry : serverMaps.entrySet()) {
-            HeartBeat heartBeat = HeartBeat.decodeHeartBeat(entry.getValue());
+            HeartBeatModel heartBeat = HeartBeatUtils.decodeHeartBeat(entry.getValue(), nodeType);


You can make HeartBeatUtil.decodeHeartBeat return AbstractHeartBeat , then the HeartBeatModel can be removed.

Worker fields are needed in some places. like host manager, Of course, it can also be obtained by split string of heartbeat ,but it is not graceful.

You can make HeartBeatUtil.decodeHeartBeat return AbstractHeartBeat , then the HeartBeatModel can be removed.

When I designed WorkerHeartBeat and MasterHeartBeat, I have a dependency cycle problem, so I defined HeartBeatModel in module dolphinscheduler-common that contains all heartbeatInfo, which also avoids the need to hard code the worker's Heartbeat in the master, For example, the workWaitingTaskCount attribute is used in HostManager.

current design:
MasterHeartBeat in dolphinscheduler-master
WorkerHeartBeat in dolphinscheduler-worker
AbstractHeartBeat in dolphinscheduler-server (move into dolphinscheduler-common is also ok)
HeartBeatModel in dolphinscheduler-common

ps:
WorkerHeartBeat dependency injection WorkerManagerThread of dolphinscheduler-worker
HostWeight of dolphinscheduler-master use workWaitingTaskCount of WorkerHeartBeat

I don't have any good ideas right now. I need your help.

Is there any problem if you remove the HeartBeatModel and only keep AbstractHeart/MasterHeartBeat/WorkHeartBeat ?

When we get a HeartBeatInfo, we need to know this HeartBeatInfo is represent to worker or master, rather than use a big pojo to contains all fields. If we use a big pojo to represent the HeartBeatInfo, it's hard to know which field is available.

First, we need to reach a consensus that WorkerHeartBeat must autowared WorkerManagerThread in order to get WorkerWaitingTaskCount.

As you said，WorkerHeartBeat in common module, but worker module depend common module and WorkerManagerThread in worker module.

WorkHeartBeat is just a pojo, it should not depend anything, if I have any misunderstand, please let me know.

The worker needs to update workWaitingTaskCount to report the heartbeat information.

Yes, the worker will schedule a WorkerHeartbeatTask to update the info in WorkHeartBeat, there seems no problem.

I have some questions to confirm.

Do we need to define a WorkerHeartbeatTask ?

What do you mean WorkerHeartbeatTask autowared WorkerManagerThread ?

Which module is WorkerHeartbeatTask in ?

guoshupei · 2022-06-23T14:02:35Z

Worker fields are needed in some places. like host manager, Of course, it can also be obtained by split string of heartbeat ,but it is not graceful. 发自我的iPhone

…

在 2022年6月23日，21:48，Wenjun Ruan ***@***.***> 写道： @ruanwenjun commented on this pull request. In dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/registry/RegistryClient.java: > @@ -97,7 +98,7 @@ public List<Server> getServerList(NodeType nodeType) { List<Server> serverList = new ArrayList<>(); for (Map.Entry<String, String> entry : serverMaps.entrySet()) { - HeartBeat heartBeat = HeartBeat.decodeHeartBeat(entry.getValue()); + HeartBeatModel heartBeat = HeartBeatUtils.decodeHeartBeat(entry.getValue(), nodeType); You can make HeartBeatUtil.decodeHeartBeat return AbstractHeartBeat , then the HeartBeatModel can be removed. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were assigned.

caishunfeng

LGTM overall

codecov-commenter · 2022-06-24T12:57:06Z

Codecov Report

Merging #10563 (b9bbc8a) into dev (393cb64) will increase coverage by 0.15%.
The diff coverage is 73.56%.

@@             Coverage Diff              @@
##                dev   #10563      +/-   ##
============================================
+ Coverage     40.92%   41.08%   +0.15%     
- Complexity     4874     4896      +22     
============================================
  Files           895      899       +4     
  Lines         36201    36242      +41     
  Branches       3986     3986              
============================================
+ Hits          14817    14889      +72     
+ Misses        19919    19890      -29     
+ Partials       1465     1463       -2

Impacted Files	Coverage Δ
.../org/apache/dolphinscheduler/common/Constants.java	`81.81% <ø> (ø)`
...server/master/dispatch/host/CommonHostManager.java	`74.07% <0.00%> (ø)`
...r/master/dispatch/host/LowerWeightHostManager.java	`0.00% <0.00%> (ø)`
...inscheduler/server/registry/AbstractHeartBeat.java	`0.00% <0.00%> (ø)`
...olphinscheduler/server/registry/HeartBeatTask.java	`0.00% <0.00%> (ø)`
...phinscheduler/service/registry/RegistryClient.java	`0.79% <0.00%> (ø)`
.../dolphinscheduler/common/utils/HeartBeatUtils.java	`95.12% <95.12%> (ø)`
...eduler/server/worker/registry/WorkerHeartBeat.java	`95.23% <95.23%> (ø)`
.../dolphinscheduler/common/model/HeartBeatModel.java	`100.00% <100.00%> (ø)`
...eduler/server/master/registry/MasterHeartBeat.java	`100.00% <100.00%> (ø)`
... and 7 more

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 393cb64...b9bbc8a. Read the comment docs.

…Improvement-10506

sonarqubecloud · 2022-06-24T15:36:20Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

77.5% Coverage
8.7% Duplication

caishunfeng · 2023-02-02T04:34:11Z

I close this pr because it had been implemented by #11702

closes apache#10506

a60fb37

guoshupei requested review from SbloodyS, caishunfeng and ruanwenjun as code owners June 23, 2022 03:27

SbloodyS assigned SbloodyS and guoshupei and unassigned SbloodyS Jun 23, 2022

SbloodyS added improvement make more easy to user or prompt friendly backend labels Jun 23, 2022

SbloodyS added this to the 3.1.0-alpha milestone Jun 23, 2022

Remove unnecessary code

7413906

ruanwenjun requested changes Jun 23, 2022

View reviewed changes

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

guoshupei added 2 commits June 23, 2022 21:21

move some fields to WorkerHeartBeat

151347e

Merge branch 'dev' of https://github.com/apache/dolphinscheduler into…

ecefa90

… Improvement-10506

ruanwenjun reviewed Jun 23, 2022

View reviewed changes

edit testNormalState and testBusyState of WorkerHeartBeatTest

42fe3e2

caishunfeng reviewed Jun 24, 2022

View reviewed changes

guoshupei added 5 commits June 24, 2022 11:42

edit testNormalState and testBusyState of WorkerHeartBeatTest

6e29b24

remove testNormalState and update testBusyState of WorkerHeartBeatTest

f1a7a40

update loadAverage and reservedMemory default value

796aee9

remove testNormalState from MasterHeartBeatTest

da102ac

add test cases

c932bdd

guoshupei requested a review from caishunfeng June 24, 2022 13:17

guoshupei added 2 commits June 24, 2022 21:58

update test cases

b9bbc8a

Merge branch 'dev' of http://github.com/apache/dolphinscheduler into …

a0fc028

…Improvement-10506

guoshupei requested a review from ruanwenjun June 25, 2022 03:13

caishunfeng removed this from the 3.1.0 milestone Sep 27, 2022

caishunfeng added this to the 3.2.0 milestone Sep 27, 2022

caishunfeng closed this Feb 2, 2023

Conversation

guoshupei commented Jun 23, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guoshupei commented Jun 23, 2022 via email

Uh oh!

caishunfeng left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sonarqubecloud bot commented Jun 24, 2022

Uh oh!

caishunfeng commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Jun 24, 2022 •

edited

Loading

caishunfeng commented Feb 2, 2023 •

edited

Loading