You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "SteNicholas (via GitHub)" <gi...@apache.org> on 2023/12/20 03:18:42 UTC
[PR] [CELEBORN-1189] Introduce RunningApplicationCount metric to record the count of running applications in worker [incubator-celeborn]
SteNicholas opened a new pull request, #2172:
URL: https://github.com/apache/incubator-celeborn/pull/2172
### What changes were proposed in this pull request?
Introduce `RunningApplicationCount` metric to record the count of running applications for Celeborn worker.
### Why are the changes needed?
`RunningApplicationCount` metrics only monitors the count of running applications in the cluster for master. Meanwhile, `/listTopDiskUsedApps` API lists the top disk usage application ids for master and worker. Therefore `RunningApplicationCount` metric could be introduced to monitor the count of running applications in worker.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Internal tests.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker [incubator-celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas closed pull request #2172: [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker
URL: https://github.com/apache/incubator-celeborn/pull/2172
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker [incubator-celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas closed pull request #2172: [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker
URL: https://github.com/apache/incubator-celeborn/pull/2172
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker [incubator-celeborn]
Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX commented on code in PR #2172:
URL: https://github.com/apache/incubator-celeborn/pull/2172#discussion_r1436077431
##########
common/src/main/scala/org/apache/celeborn/common/meta/DeviceInfo.scala:
##########
@@ -84,6 +85,7 @@ class DiskInfo(
var storageType: StorageInfo.Type = StorageInfo.Type.SSD
var maxSlots: Long = 0
lazy val shuffleAllocations = new util.HashMap[String, Integer]()
+ lazy val applicationShuffles = new util.HashMap[String, util.HashSet[String]]()
Review Comment:
Looks like you have only used the application's key. There is no need to record the shuffle key set.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker [incubator-celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #2172:
URL: https://github.com/apache/incubator-celeborn/pull/2172#discussion_r1436329360
##########
assets/grafana/celeborn-dashboard.json:
##########
@@ -334,7 +334,7 @@
"type": "prometheus",
"uid": "${DS_PROMETHEUS}"
},
- "expr": "metrics_ApplicationCount_Value",
+ "expr": "metrics_ApplicationCount_Value{role=\"Master\"}",
Review Comment:
@FMX, I have reverted this change. Meanwhile, this panel could provide the application count of master and worker instances.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker [incubator-celeborn]
Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX closed pull request #2172: [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker
URL: https://github.com/apache/incubator-celeborn/pull/2172
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker [incubator-celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas closed pull request #2172: [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker
URL: https://github.com/apache/incubator-celeborn/pull/2172
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker [incubator-celeborn]
Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX commented on code in PR #2172:
URL: https://github.com/apache/incubator-celeborn/pull/2172#discussion_r1436366712
##########
common/src/main/scala/org/apache/celeborn/common/meta/DeviceInfo.scala:
##########
@@ -121,28 +123,45 @@ class DiskInfo(
}
def allocateSlots(shuffleKey: String, slots: Int): Unit = this.synchronized {
- val allocated = shuffleAllocations.getOrDefault(shuffleKey, 0)
- shuffleAllocations.put(shuffleKey, allocated + slots)
+ val shuffleAllocated = shuffleAllocations.getOrDefault(shuffleKey, 0)
+ shuffleAllocations.put(shuffleKey, shuffleAllocated + slots)
+ val applicationId = Utils.splitShuffleKey(shuffleKey)._1
+ val applicationAllocated = applicationAllocations.getOrDefault(applicationId, 0)
+ applicationAllocations.put(applicationId, applicationAllocated + slots)
activeSlots = activeSlots + slots
}
def releaseSlots(shuffleKey: String, slots: Int): Unit = this.synchronized {
- val allocated = shuffleAllocations.getOrDefault(shuffleKey, 0)
- if (allocated < slots) {
- logError(s"allocated $allocated is less than to release $slots !")
+ var shuffleAllocated = shuffleAllocations.getOrDefault(shuffleKey, 0)
+ if (shuffleAllocated < slots) {
+ logError(s"allocated $shuffleAllocated is less than to release $slots !")
}
- val delta = Math.min(allocated, slots)
+ val applicationId = Utils.splitShuffleKey(shuffleKey)._1
+ var applicationAllocated = applicationAllocations.getOrDefault(applicationId, 0)
+ val delta = Math.min(shuffleAllocated, slots)
activeSlots = activeSlots - delta
- if (allocated > slots) {
- shuffleAllocations.put(shuffleKey, allocated - slots)
+ if (shuffleAllocated > slots) {
+ shuffleAllocated = shuffleAllocated - slots
+ applicationAllocated = applicationAllocated - slots
} else {
- shuffleAllocations.put(shuffleKey, 0)
+ shuffleAllocated = 0
Review Comment:
Delete this branch.
##########
common/src/main/scala/org/apache/celeborn/common/meta/DeviceInfo.scala:
##########
@@ -121,28 +123,45 @@ class DiskInfo(
}
def allocateSlots(shuffleKey: String, slots: Int): Unit = this.synchronized {
- val allocated = shuffleAllocations.getOrDefault(shuffleKey, 0)
- shuffleAllocations.put(shuffleKey, allocated + slots)
+ val shuffleAllocated = shuffleAllocations.getOrDefault(shuffleKey, 0)
+ shuffleAllocations.put(shuffleKey, shuffleAllocated + slots)
+ val applicationId = Utils.splitShuffleKey(shuffleKey)._1
+ val applicationAllocated = applicationAllocations.getOrDefault(applicationId, 0)
+ applicationAllocations.put(applicationId, applicationAllocated + slots)
activeSlots = activeSlots + slots
}
def releaseSlots(shuffleKey: String, slots: Int): Unit = this.synchronized {
- val allocated = shuffleAllocations.getOrDefault(shuffleKey, 0)
- if (allocated < slots) {
- logError(s"allocated $allocated is less than to release $slots !")
+ var shuffleAllocated = shuffleAllocations.getOrDefault(shuffleKey, 0)
+ if (shuffleAllocated < slots) {
+ logError(s"allocated $shuffleAllocated is less than to release $slots !")
}
- val delta = Math.min(allocated, slots)
+ val applicationId = Utils.splitShuffleKey(shuffleKey)._1
+ var applicationAllocated = applicationAllocations.getOrDefault(applicationId, 0)
+ val delta = Math.min(shuffleAllocated, slots)
activeSlots = activeSlots - delta
- if (allocated > slots) {
- shuffleAllocations.put(shuffleKey, allocated - slots)
+ if (shuffleAllocated > slots) {
Review Comment:
This operator can be changed to >=
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1189] Introduce RunningApplicationCount metric and /applications API to record running applications of worker [incubator-celeborn]
Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX commented on code in PR #2172:
URL: https://github.com/apache/incubator-celeborn/pull/2172#discussion_r1436077683
##########
assets/grafana/celeborn-dashboard.json:
##########
@@ -334,7 +334,7 @@
"type": "prometheus",
"uid": "${DS_PROMETHEUS}"
},
- "expr": "metrics_ApplicationCount_Value",
+ "expr": "metrics_ApplicationCount_Value{role=\"Master\"}",
Review Comment:
We need to add the worker's application count panel.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1189] Introduce RunningApplicationCount metric to record the count of running applications in worker [incubator-celeborn]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2172:
URL: https://github.com/apache/incubator-celeborn/pull/2172#issuecomment-1863803981
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2172?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `11 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`4dacf72`)](https://app.codecov.io/gh/apache/incubator-celeborn/commit/4dacf72a6d662dd6b2bf6c60669a8baad0f1566d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 47.26% compared to head [(`dbae2c7`)](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2172?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 47.10%.
| [Files](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2172?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...a/org/apache/celeborn/common/meta/DeviceInfo.scala](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2172?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vY29tbW9uL21ldGEvRGV2aWNlSW5mby5zY2FsYQ==) | 56.25% | [5 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2172?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...a/org/apache/celeborn/common/meta/WorkerInfo.scala](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2172?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vY29tbW9uL21ldGEvV29ya2VySW5mby5zY2FsYQ==) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2172?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 @@
## main #2172 +/- ##
==========================================
- Coverage 47.26% 47.10% -0.15%
==========================================
Files 185 185
Lines 11673 11693 +20
Branches 1039 1043 +4
==========================================
- Hits 5516 5507 -9
- Misses 5795 5816 +21
- Partials 362 370 +8
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2172?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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org