Skip to content

[Feature][Monitoring] Added disk available capacity monitoring#10186

Closed
retime123 wants to merge 18 commits intoapache:devfrom
retime123:dev
Closed

[Feature][Monitoring] Added disk available capacity monitoring#10186
retime123 wants to merge 18 commits intoapache:devfrom
retime123:dev

Conversation

@retime123
Copy link
Contributor

@retime123 retime123 commented May 23, 2022 •
edited
Loading

Purpose of the pull request

This method is added because the available disk capacity cannot be monitored

Brief change log

Change dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java
Change dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HeartBeat.java
Change dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/OSUtils.java
Change dolphinscheduler-ui/src/views/monitor/servers/master/index.tsx
Change dolphinscheduler-ui/src/views/monitor/servers/worker/index.tsx
Change dolphinscheduler-ui/src/locales/modules/en_US.ts
Change dolphinscheduler-ui/src/locales/modules/zh_CN.ts

unit testing
Change dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/os/OSUtilsTest.java
Change dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/HeartBeatTest.java

Verify this pull request

retry

image

@codecov-commenter
Copy link

codecov-commenter commented May 23, 2022 •
edited
Loading

Codecov Report

Merging #10186 (b5cd421) into dev (d8db2b5) will decrease coverage by 0.00%.
The diff coverage is 61.53%.

❗ Current head b5cd421 differs from pull request most recent head 1c85d88. Consider uploading reports for the commit 1c85d88 to get more accurate results

@@             Coverage Diff              @@
##                dev   #10186      +/-   ##
============================================
- Coverage     40.96%   40.96%   -0.01%     
+ Complexity     4736     4735       -1     
============================================
  Files           854      854              
  Lines         34565    34577      +12     
  Branches       3819     3819              
============================================
+ Hits          14159    14163       +4     
- Misses        19051    19060       +9     
+ Partials       1355     1354       -1
Impacted Files Coverage Δ
.../org/apache/dolphinscheduler/common/Constants.java 80.95% <ø> (ø)
...pache/dolphinscheduler/common/utils/HeartBeat.java 51.28% <28.57%> (-0.98%) ⬇️
.../apache/dolphinscheduler/common/utils/OSUtils.java 20.38% <100.00%> (+3.16%) ⬆️
...er/master/dispatch/host/assign/RandomSelector.java 77.77% <0.00%> (-5.56%) ⬇️
...org/apache/dolphinscheduler/remote/utils/Host.java 37.77% <0.00%> (-2.23%) ⬇️
...e/dolphinscheduler/remote/NettyRemotingClient.java 52.11% <0.00%> (-1.41%) ⬇️

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 d8db2b5...1c85d88. Read the comment docs.

@songjianet songjianet changed the title Monitoring: Added disk available capacity monitoring [Feature][Monitoring] Added disk available capacity monitoring May 23, 2022
@songjianet songjianet added feature new feature UI ui and front end related backend labels May 23, 2022
@songjianet songjianet added this to the 3.1.0-alpha milestone May 23, 2022
@songjianet
Copy link
Member

songjianet commented May 23, 2022

Please create an issue and associate with it. @retime123

@retime123
Copy link
Contributor Author

retime123 commented May 23, 2022

image
E2E,Still can't pass! I need you guys to take a look

@retime123 retime123 mentioned this pull request May 23, 2022
Closed
3 tasks
@SbloodyS
Copy link
Member

SbloodyS commented May 23, 2022

@retime123 You can take a look at this error.

@retime123
Copy link
Contributor Author

retime123 commented May 23, 2022

@retime123你可以看看这个错误。

Can't see the specific error! Can you send me the generated logs? Or could you show me how to change it

@SbloodyS
Copy link
Member

SbloodyS commented May 23, 2022


@retime123你可以看看这个错误。

Can't see the specific error! Can you send me the generated logs? Or could you show me how to change it

The error logs in https://github.com/apache/dolphinscheduler/runs/6548541522?check_suite_focus=true

@SbloodyS SbloodyS linked an issue May 23, 2022 that may be closed by this pull request
[Bug] [E2E] The front-end page changes failed the E2E test #10190
Closed
3 tasks
@retime123
Copy link
Contributor Author

retime123 commented May 24, 2022

Can this function be combined?

@SbloodyS SbloodyS requested a review from caishunfeng May 24, 2022 11:01
caishunfeng
caishunfeng reviewed May 24, 2022
View reviewed changes
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/OSUtils.java Outdated
*/
public static double diskAvailable() {
File file = new File(".");
long totalSpace = file.getTotalSpace(); //total disk space in bytes.
Copy link
Contributor

@caishunfeng caishunfeng May 24, 2022

Choose a reason for hiding this comment

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

It seems not used.

Copy link
Contributor Author

@retime123 retime123 May 24, 2022

Choose a reason for hiding this comment

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

totalSpace Yes. It's not used

Copy link
Contributor

@caishunfeng caishunfeng May 24, 2022

Choose a reason for hiding this comment

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

totalSpace Yes. It's not used

Please remove it if not use.

Copy link
Contributor Author

@retime123 retime123 May 24, 2022

Choose a reason for hiding this comment

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

Resubmitted

@caishunfeng
Copy link
Contributor

caishunfeng commented May 24, 2022

Hi @retime123 I found that you had submit a same pr and the comment which not resolve #10041 (review),
can you test in k8s?

@retime123
Copy link
Contributor Author

retime123 commented May 24, 2022

Hi @retime123 I found that you had submit a same pr and the comment which not resolve #10041 (review), can you test in k8s?

This probably has nothing to do with K8S.

@retime123
Copy link
Contributor Author

retime123 commented May 24, 2022 •
edited
Loading

image
It's always a surprise！
Run try again

@caishunfeng
Copy link
Contributor

caishunfeng commented May 25, 2022

image It's always a surprise！ Run try again

I rerun it.

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 26, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

61.5% 61.5% Coverage
0.0% 0.0% Duplication

caishunfeng
caishunfeng approved these changes May 30, 2022
View reviewed changes
Copy link
Contributor

@caishunfeng caishunfeng left a comment

Choose a reason for hiding this comment

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

LGTM of the backend part. PTAL @songjianet @labbomb

@caishunfeng
Copy link
Contributor

caishunfeng commented May 30, 2022

@retime123 please resolve the conflicts.

@retime123
Copy link
Contributor Author

retime123 commented May 30, 2022

This is refactoring! I'll resubmit it

@retime123 retime123 mentioned this pull request May 30, 2022
Merged
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@caishunfeng caishunfeng caishunfeng approved these changes

Assignees

@retime123 retime123

Labels

backend feature new feature UI ui and front end related

Projects

None yet

Milestone

3.1.0

Development

Successfully merging this pull request may close these issues.

[Bug] [E2E] The front-end page changes failed the E2E test

5 participants

@retime123 @codecov-commenter @songjianet @SbloodyS @caishunfeng