You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "zhuyaogai (via GitHub)" <gi...@apache.org> on 2023/08/19 06:22:21 UTC
[GitHub] [incubator-uniffle] zhuyaogai opened a new pull request, #1160: [#1108] [FEATURE] Add labels with disk path for local storage total_localfile_write_data metrics.
zhuyaogai opened a new pull request, #1160:
URL: https://github.com/apache/incubator-uniffle/pull/1160
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
Examples:
- "[#123] feat(operator): support xxx"
- "[#233] fix: check null before access result in xxx"
- "[MINOR] refactor: fix typo in variable name"
- "[MINOR] docs: fix typo in README"
- "[#255] test: fix flaky test NameOfTheTest"
Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. Contributor guidelines:
https://github.com/apache/incubator-uniffle/blob/master/CONTRIBUTING.md
3. If the PR is unfinished, please mark this PR as draft.
-->
### What changes were proposed in this pull request?
Add labels with disk path for local storage total_localfile_write_data metrics.
### Why are the changes needed?
Fix: https://github.com/apache/incubator-uniffle/issues/1108
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Add more unit test case.
--
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
[GitHub] [incubator-uniffle] zuston merged pull request #1160: [#1108] feat(server): Add labels with disk path for local storage total_localfile_write_data metrics.
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston merged PR #1160:
URL: https://github.com/apache/incubator-uniffle/pull/1160
--
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
[GitHub] [incubator-uniffle] zuston commented on a diff in pull request #1160: [#1108] feat(server): Add labels with disk path for local storage total_localfile_write_data metrics.
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #1160:
URL: https://github.com/apache/incubator-uniffle/pull/1160#discussion_r1299674820
##########
server/src/main/java/org/apache/uniffle/server/storage/LocalStorageManager.java:
##########
@@ -237,7 +237,9 @@ public Storage selectStorage(ShuffleDataReadEvent event) {
@Override
public void updateWriteMetrics(ShuffleDataFlushEvent event, long writeTime) {
super.updateWriteMetrics(event, writeTime);
- ShuffleServerMetrics.counterTotalLocalFileWriteDataSize.inc(event.getSize());
+ ShuffleServerMetrics.counterTotalLocalFileWriteDataSize
+ .labels(event.getUnderStorage().getStoragePath())
Review Comment:
I think we should check wheater the underStorage is null?
--
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
[GitHub] [incubator-uniffle] zhuyaogai commented on pull request #1160: [#1108] feat(server): Add labels with disk path for local storage total_localfile_write_data metrics.
Posted by "zhuyaogai (via GitHub)" <gi...@apache.org>.
zhuyaogai commented on PR #1160:
URL: https://github.com/apache/incubator-uniffle/pull/1160#issuecomment-1685029654
@zuston BTW, do we need to add an aggregate label like `LOCAL_DISK_PATH_LABEL_ALL`?
--
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
[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #1160: [#1108] feat(server): Add labels with disk path for local storage total_localfile_write_data metrics.
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1160:
URL: https://github.com/apache/incubator-uniffle/pull/1160#issuecomment-1685747086
## [Codecov](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1160?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#1160](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1160?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (b5cf1f2) into [master](https://app.codecov.io/gh/apache/incubator-uniffle/commit/680b4eaa8fc19b0eff19c320bb249049b72f9c2d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (680b4ea) will **increase** coverage by `0.51%`.
> Report is 1 commits behind head on master.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #1160 +/- ##
============================================
+ Coverage 53.63% 54.14% +0.51%
+ Complexity 2576 2107 -469
============================================
Files 391 319 -72
Lines 22359 14442 -7917
Branches 1875 1314 -561
============================================
- Hits 11992 7820 -4172
+ Misses 9660 6202 -3458
+ Partials 707 420 -287
```
| [Files Changed](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1160?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...rg/apache/uniffle/server/ShuffleServerMetrics.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1160?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlU2VydmVyTWV0cmljcy5qYXZh) | `94.91% <100.00%> (ø)` | |
| [...he/uniffle/server/storage/LocalStorageManager.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1160?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9zdG9yYWdlL0xvY2FsU3RvcmFnZU1hbmFnZXIuamF2YQ==) | `87.89% <100.00%> (+0.12%)` | :arrow_up: |
... and [76 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-uniffle/pull/1160/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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
[GitHub] [incubator-uniffle] zhuyaogai commented on a diff in pull request #1160: [#1108] feat(server): Add labels with disk path for local storage total_localfile_write_data metrics.
Posted by "zhuyaogai (via GitHub)" <gi...@apache.org>.
zhuyaogai commented on code in PR #1160:
URL: https://github.com/apache/incubator-uniffle/pull/1160#discussion_r1299730374
##########
server/src/main/java/org/apache/uniffle/server/storage/LocalStorageManager.java:
##########
@@ -237,7 +237,9 @@ public Storage selectStorage(ShuffleDataReadEvent event) {
@Override
public void updateWriteMetrics(ShuffleDataFlushEvent event, long writeTime) {
super.updateWriteMetrics(event, writeTime);
- ShuffleServerMetrics.counterTotalLocalFileWriteDataSize.inc(event.getSize());
+ ShuffleServerMetrics.counterTotalLocalFileWriteDataSize
+ .labels(event.getUnderStorage().getStoragePath())
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
[GitHub] [incubator-uniffle] zuston commented on pull request #1160: [#1108] feat(server): Add labels with disk path for local storage total_localfile_write_data metrics.
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #1160:
URL: https://github.com/apache/incubator-uniffle/pull/1160#issuecomment-1685918440
Thanks for your contribution! @zhuyaogai Merged.
--
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