You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "kuszz (via GitHub)" <gi...@apache.org> on 2024/07/15 03:30:26 UTC
[PR] Add some metric about shuffle server [incubator-uniffle]
kuszz opened a new pull request, #1904:
URL: https://github.com/apache/incubator-uniffle/pull/1904
### What changes were proposed in this pull request?
(Please outline the changes and how this PR fixes the issue.)
### Why are the changes needed?
add some metric would be helpful to understand the status of the shuffle server
Fix: #1903
### Does this PR introduce _any_ user-facing change?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] Add some metric about shuffle server [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on code in PR #1904:
URL: https://github.com/apache/incubator-uniffle/pull/1904#discussion_r1679177197
##########
server/src/main/java/org/apache/uniffle/server/storage/SingleStorageManager.java:
##########
@@ -149,4 +164,15 @@ public void start() {
public void stop() {
// do nothing
}
+
+ public void removeAppStorageInfo(PurgeEvent event) {
+ String appId = event.getAppId();
+ ApplicationStorageInfo info = appStorageInfoMap.remove(appId);
+ if (info != null) {
+ ShuffleServerMetrics.gaugeStorageUsedBytes.dec(info.getUsedBytes());
+ ShuffleServerMetrics.gaugeFlushFileNum.dec(info.getFileNum());
Review Comment:
We do not need to dec these two metrics, and we can call these `counterStorageWriteBytes` and `counsterFlushFileNum`, base on this, we can get currentStorageUsedBytes by `counterStorageWriteBytes-counterTotalDeleteDataSize `
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] Add some metric about shuffle server [incubator-uniffle]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1904:
URL: https://github.com/apache/incubator-uniffle/pull/1904#issuecomment-2227664099
## Test Results
2 657 files ±0 2 657 suites ±0 5h 32m 49s :stopwatch: +2s
946 tests ±0 944 :white_check_mark: ±0 1 :zzz: ±0 1 :x: ±0
11 799 runs ±0 11 783 :white_check_mark: ±0 15 :zzz: ±0 1 :x: ±0
For more details on these failures, see [this check](https://github.com/apache/incubator-uniffle/runs/27436387837).
Results for commit 80978c03. ± Comparison against base commit 4a1de067.
[test-results]:data:application/gzip;base64,H4sIAE6flGYC/1XMyw6DIBCF4VcxrLvgIoPTl2koQkLqpUFYmb57R6OWLs/H8K8sxMEv7N5I0ObWsKXEXO++JJvjPJEIREAies/bBbZwrsdSnNup/dErvrdfFwQbhz/wKc2JhJOkMm1NIQziMc8oWacuO6r6hKq67zrq5nGMmQbrOJrOcdUaMFqBEtJjLyGIJ4LjQaM1XAou2ecLbDw2sBIBAAA=
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1903] improvement(server): Add some metrics for shuffle server [incubator-uniffle]
Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma merged PR #1904:
URL: https://github.com/apache/incubator-uniffle/pull/1904
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] Add some metric about shuffle server [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on code in PR #1904:
URL: https://github.com/apache/incubator-uniffle/pull/1904#discussion_r1679177197
##########
server/src/main/java/org/apache/uniffle/server/storage/SingleStorageManager.java:
##########
@@ -149,4 +164,15 @@ public void start() {
public void stop() {
// do nothing
}
+
+ public void removeAppStorageInfo(PurgeEvent event) {
+ String appId = event.getAppId();
+ ApplicationStorageInfo info = appStorageInfoMap.remove(appId);
+ if (info != null) {
+ ShuffleServerMetrics.gaugeStorageUsedBytes.dec(info.getUsedBytes());
+ ShuffleServerMetrics.gaugeFlushFileNum.dec(info.getFileNum());
Review Comment:
We do not need to dec these two metrics, and we can call these `counterStorageWriteBytes` and `counsterFlushFileNum`, base on this, we can get currentStorageUsedBytes by `counterStorageWriteBytes-counterTotalDeleteDataSize `
##########
server/src/main/java/org/apache/uniffle/server/storage/SingleStorageManager.java:
##########
@@ -92,6 +97,16 @@ public void updateWriteMetrics(ShuffleDataFlushEvent event, long writeTime) {
} else {
ShuffleServerMetrics.counterEventSizeThresholdLevel4.inc();
}
+ String appId = event.getAppId();
+ ApplicationStorageInfo appStorage = appStorageInfoMap.computeIfAbsent(appId,
+ id -> new ApplicationStorageInfo(appId));
+ appStorage.incUsedBytes(event.getSize());
+ ShuffleServerMetrics.gaugeStorageUsedBytes.inc(event.getSize());
+ if (event.getUnderStorage().containsWriteHandler(appId)) {
+ appStorage.incFileNum(1);
+ ShuffleServerMetrics.gaugeFlushFileNum.inc(1);
Review Comment:
It can be replaced by `inc()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1903] improvment: Add some metric about shuffle server [incubator-uniffle]
Posted by "kuszz (via GitHub)" <gi...@apache.org>.
kuszz commented on code in PR #1904:
URL: https://github.com/apache/incubator-uniffle/pull/1904#discussion_r1680883510
##########
server/src/main/java/org/apache/uniffle/server/storage/SingleStorageManager.java:
##########
@@ -92,6 +97,16 @@ public void updateWriteMetrics(ShuffleDataFlushEvent event, long writeTime) {
} else {
ShuffleServerMetrics.counterEventSizeThresholdLevel4.inc();
}
+ String appId = event.getAppId();
+ ApplicationStorageInfo appStorage = appStorageInfoMap.computeIfAbsent(appId,
+ id -> new ApplicationStorageInfo(appId));
+ appStorage.incUsedBytes(event.getSize());
+ ShuffleServerMetrics.gaugeStorageUsedBytes.inc(event.getSize());
+ if (event.getUnderStorage().containsWriteHandler(appId)) {
+ appStorage.incFileNum(1);
+ ShuffleServerMetrics.gaugeFlushFileNum.inc(1);
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1903] improvment: Add some metric about shuffle server [incubator-uniffle]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1904:
URL: https://github.com/apache/incubator-uniffle/pull/1904#issuecomment-2233112927
## [Codecov](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1904?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `63.41463%` with `15 lines` in your changes missing coverage. Please review.
> Project coverage is 53.59%. Comparing base [(`34b5187`)](https://app.codecov.io/gh/apache/incubator-uniffle/commit/34b5187fdf2959e543be75b2bf82824400bba60b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`9198a5b`)](https://app.codecov.io/gh/apache/incubator-uniffle/commit/9198a5b1d135b181dc137386a817daf1de6596e6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 13 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1904?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...uniffle/common/storage/ApplicationStorageInfo.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1904?src=pr&el=tree&filepath=common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Funiffle%2Fcommon%2Fstorage%2FApplicationStorageInfo.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NvbW1vbi9zdG9yYWdlL0FwcGxpY2F0aW9uU3RvcmFnZUluZm8uamF2YQ==) | 0.00% | [14 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1904?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...apache/uniffle/storage/common/AbstractStorage.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1904?src=pr&el=tree&filepath=storage%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Funiffle%2Fstorage%2Fcommon%2FAbstractStorage.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3RvcmFnZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvdW5pZmZsZS9zdG9yYWdlL2NvbW1vbi9BYnN0cmFjdFN0b3JhZ2UuamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1904?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #1904 +/- ##
============================================
+ Coverage 52.01% 53.59% +1.58%
- Complexity 2467 2472 +5
============================================
Files 411 389 -22
Lines 20444 17598 -2846
Branches 1629 1608 -21
============================================
- Hits 10633 9432 -1201
+ Misses 9164 7600 -1564
+ Partials 647 566 -81
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1904?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1903] improvement(server): Add some metrics for shuffle server [incubator-uniffle]
Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on PR #1904:
URL: https://github.com/apache/incubator-uniffle/pull/1904#issuecomment-2234000251
Merged to master.
Thanks for your contribution. @kuszz
Thanks for your reviews. @maobaolong @jerqi
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org