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 2024/12/24 08:28:16 UTC
[PR] [CELEBORN-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
SteNicholas opened a new pull request, #3026:
URL: https://github.com/apache/celeborn/pull/3026
### What changes were proposed in this pull request?
Introduce `ApplicationTotalCount` and `ApplicationFallbackCount` metric to record the total and fallback count of application.
### Why are the changes needed?
There is no any metric to record the total count of application running with celeborn shuffle and engine bulit-in shuffle and the fallback count of application. Meanwhile, the fallback of Flink shuffle is based on job granularity rather than shuffle granularity.
Follw up https://github.com/apache/celeborn/pull/3012#issuecomment-2553488532.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- `DefaultMetaSystemSuiteJ#testShuffleAndApplicationCountWithFallback`
- `RatisMasterStatusSystemSuiteJ#testShuffleAndApplicationCountWithFallback`
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1909766236
##########
client-spark/spark-2/src/main/java/org/apache/spark/shuffle/celeborn/SparkShuffleManager.java:
##########
@@ -118,10 +123,24 @@ public <K, V, C> ShuffleHandle registerShuffle(
String appId = SparkUtils.appUniqueId(dependency.rdd().context());
initializeLifecycleManager(appId);
+ if (!registeredApps.contains(appId)) {
Review Comment:
@RexXiong, from my knowledge, there is no case that SparkContext or ShuffleManager or lifecycleManager have multiple apps. Could you give some input or thoughts about this?
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1897015659
##########
master/src/main/java/org/apache/celeborn/service/deploy/master/clustermeta/ha/MetaHandler.java:
##########
@@ -150,8 +155,17 @@ public ResourceResponse handleWriteRequest(ResourceProtos.ResourceRequest reques
shuffleFallbackCounts.values().stream().mapToLong(v -> v).sum(),
appId);
}
+ Map<String, Long> applicationFallbackCounts =
+ request.getAppHeartbeatRequest().getApplicationFallbackCountsMap();
Review Comment:
Seems not necessary, due we have log the shuffle fallback info with appId.
```
if (CollectionUtils.isNotEmpty(shuffleFallbackCounts)) {
LOG.warn(
"{} shuffle fallbacks in app {}",
shuffleFallbackCounts.values().stream().mapToLong(v -> v).sum(),
appId);
}
```
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2668032049
@RexXiong, any comment for the updates?
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r2062365393
##########
client-spark/spark-3/src/main/java/org/apache/spark/shuffle/celeborn/SparkShuffleManager.java:
##########
@@ -139,6 +139,7 @@ private void initializeLifecycleManager(String appId) {
if (lifecycleManager == null) {
appUniqueId = celebornConf.appUniqueIdWithUUIDSuffix(appId);
lifecycleManager = new LifecycleManager(appUniqueId, celebornConf);
+ lifecycleManager.applicationCount().increment();
Review Comment:
This comments should has been resolved.
https://github.com/apache/celeborn/pull/3026#discussion_r1912595366
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "codenohup (via GitHub)" <gi...@apache.org>.
codenohup commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1897191750
##########
client-flink/common/src/main/java/org/apache/celeborn/plugin/flink/RemoteShuffleMaster.java:
##########
@@ -102,6 +102,7 @@ public void registerJob(JobShuffleContext context) {
celebornAppId = FlinkUtils.toCelebornAppId(lifecycleManagerTimestamp, jobID);
LOG.info("CelebornAppId: {}", celebornAppId);
lifecycleManager = new LifecycleManager(celebornAppId, conf);
+ lifecycleManager.applicationCount().increment();
Review Comment:
The application count should be recorded on a per-job basis rather than per cluster.
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1901429247
##########
client-spark/spark-2/src/main/java/org/apache/spark/shuffle/celeborn/SparkShuffleManager.java:
##########
@@ -118,10 +123,24 @@ public <K, V, C> ShuffleHandle registerShuffle(
String appId = SparkUtils.appUniqueId(dependency.rdd().context());
initializeLifecycleManager(appId);
+ if (!registeredApps.contains(appId)) {
Review Comment:
Does a SparkContext or ShuffleManager or lifecycleManager have multiple apps?
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r2062363219
##########
client-spark/spark-3/src/main/java/org/apache/spark/shuffle/celeborn/SparkShuffleManager.java:
##########
@@ -139,6 +139,7 @@ private void initializeLifecycleManager(String appId) {
if (lifecycleManager == null) {
appUniqueId = celebornConf.appUniqueIdWithUUIDSuffix(appId);
lifecycleManager = new LifecycleManager(appUniqueId, celebornConf);
+ lifecycleManager.applicationCount().increment();
Review Comment:
rebased the code, could you take a look? @RexXiong
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r2095471022
##########
assets/grafana/celeborn-dashboard.json:
##########
@@ -1710,6 +1710,186 @@
"x": 0,
"y": 90
},
+ "id": 239,
+ "options": {
+ "legend": {
+ "calcs": [],
+ "displayMode": "list",
+ "placement": "bottom",
+ "showLegend": true
+ },
+ "tooltip": {
+ "maxHeight": 600,
+ "mode": "single",
+ "sort": "none"
+ }
+ },
+ "targets": [
+ {
+ "datasource": {
+ "type": "prometheus",
+ "uid": "${DS_PROMETHEUS}"
+ },
+ "expr": "metrics_ApplicationTotalCount_Value{instance=~\"${instance}\"}",
+ "legendFormat": "${baseLegend}",
+ "range": true,
+ "refId": "A"
+ }
+ ],
+ "title": "metrics_ApplicationTotalCount_Value",
+ "type": "timeseries"
+ },
+ {
+ "datasource": {
+ "type": "prometheus",
+ "uid": "${DS_PROMETHEUS}"
+ },
+ "description": "The count of application fallbacks.",
+ "fieldConfig": {
+ "defaults": {
+ "color": {
+ "mode": "palette-classic"
+ },
+ "custom": {
+ "axisCenteredZero": false,
+ "axisColorMode": "text",
+ "axisLabel": "",
+ "axisPlacement": "auto",
+ "barAlignment": 0,
+ "drawStyle": "line",
+ "fillOpacity": 0,
+ "gradientMode": "none",
+ "hideFrom": {
+ "legend": false,
+ "tooltip": false,
+ "viz": false
+ },
+ "lineInterpolation": "linear",
+ "lineWidth": 1,
+ "pointSize": 5,
+ "scaleDistribution": {
+ "type": "linear"
+ },
+ "showPoints": "auto",
+ "spanNulls": false,
+ "stacking": {
+ "group": "A",
+ "mode": "none"
+ },
+ "thresholdsStyle": {
+ "mode": "off"
+ }
+ },
+ "mappings": [],
+ "thresholds": {
+ "mode": "absolute",
+ "steps": [
+ {
+ "color": "green"
+ }
+ ]
+ }
+ },
+ "overrides": []
+ },
+ "gridPos": {
+ "h": 8,
+ "w": 12,
+ "x": 12,
+ "y": 90
+ },
+ "id": 240,
+ "options": {
+ "legend": {
+ "calcs": [],
+ "displayMode": "list",
+ "placement": "bottom",
+ "showLegend": true
+ },
+ "tooltip": {
+ "maxHeight": 600,
+ "mode": "single",
+ "sort": "none"
+ }
+ },
+ "targets": [
+ {
+ "datasource": {
+ "type": "prometheus",
+ "uid": "${DS_PROMETHEUS}"
+ },
+ "expr": "metrics_ApplicationFallbackCount_Value{instance=~\"${instance}\"}",
Review Comment:
```suggestion
"expr": "metrics_ApplicationFallbackCount_Value{role=\"Master\", instance=~\"${instance}\"}",
```
##########
assets/grafana/celeborn-dashboard.json:
##########
@@ -1710,6 +1710,186 @@
"x": 0,
"y": 90
},
+ "id": 239,
+ "options": {
+ "legend": {
+ "calcs": [],
+ "displayMode": "list",
+ "placement": "bottom",
+ "showLegend": true
+ },
+ "tooltip": {
+ "maxHeight": 600,
+ "mode": "single",
+ "sort": "none"
+ }
+ },
+ "targets": [
+ {
+ "datasource": {
+ "type": "prometheus",
+ "uid": "${DS_PROMETHEUS}"
+ },
+ "expr": "metrics_ApplicationTotalCount_Value{instance=~\"${instance}\"}",
Review Comment:
```suggestion
"expr": "metrics_ApplicationTotalCount_Value{role=\"Master\", instance=~\"${instance}\"}",
```
##########
assets/grafana/celeborn-dashboard.json:
##########
@@ -1710,6 +1710,186 @@
"x": 0,
"y": 90
},
+ "id": 239,
+ "options": {
+ "legend": {
+ "calcs": [],
+ "displayMode": "list",
+ "placement": "bottom",
+ "showLegend": true
+ },
+ "tooltip": {
+ "maxHeight": 600,
+ "mode": "single",
+ "sort": "none"
+ }
+ },
+ "targets": [
+ {
+ "datasource": {
+ "type": "prometheus",
+ "uid": "${DS_PROMETHEUS}"
+ },
+ "expr": "metrics_ApplicationTotalCount_Value{instance=~\"${instance}\"}",
+ "legendFormat": "${baseLegend}",
+ "range": true,
+ "refId": "A"
+ }
+ ],
+ "title": "metrics_ApplicationTotalCount_Value",
+ "type": "timeseries"
+ },
+ {
+ "datasource": {
+ "type": "prometheus",
+ "uid": "${DS_PROMETHEUS}"
+ },
+ "description": "The count of application fallbacks.",
+ "fieldConfig": {
+ "defaults": {
+ "color": {
+ "mode": "palette-classic"
+ },
+ "custom": {
+ "axisCenteredZero": false,
+ "axisColorMode": "text",
+ "axisLabel": "",
+ "axisPlacement": "auto",
+ "barAlignment": 0,
+ "drawStyle": "line",
+ "fillOpacity": 0,
+ "gradientMode": "none",
+ "hideFrom": {
+ "legend": false,
+ "tooltip": false,
+ "viz": false
+ },
+ "lineInterpolation": "linear",
+ "lineWidth": 1,
+ "pointSize": 5,
+ "scaleDistribution": {
+ "type": "linear"
+ },
+ "showPoints": "auto",
+ "spanNulls": false,
+ "stacking": {
+ "group": "A",
+ "mode": "none"
+ },
+ "thresholdsStyle": {
+ "mode": "off"
+ }
+ },
+ "mappings": [],
+ "thresholds": {
+ "mode": "absolute",
+ "steps": [
+ {
+ "color": "green"
+ }
+ ]
+ }
+ },
+ "overrides": []
+ },
+ "gridPos": {
+ "h": 8,
+ "w": 12,
+ "x": 12,
+ "y": 90
+ },
+ "id": 240,
+ "options": {
+ "legend": {
+ "calcs": [],
+ "displayMode": "list",
+ "placement": "bottom",
+ "showLegend": true
+ },
+ "tooltip": {
+ "maxHeight": 600,
+ "mode": "single",
+ "sort": "none"
+ }
+ },
+ "targets": [
+ {
+ "datasource": {
+ "type": "prometheus",
+ "uid": "${DS_PROMETHEUS}"
+ },
+ "expr": "metrics_ApplicationFallbackCount_Value{instance=~\"${instance}\"}",
Review Comment:
```suggestion
"expr": "metrics_ApplicationFallbackCount_Value{role=\"Master\", instance=~\"${instance}\"}",
```
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2764749907
cc @SteNicholas
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2601605643
@RexXiong
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2561121803
Ping @turboFei, @codenohup, @reswqa, @FMX.
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1896906010
##########
common/src/main/proto/TransportMessages.proto:
##########
@@ -680,7 +682,9 @@ message PbSnapshotMetaInfo {
map<string, PbApplicationMeta> applicationMetas = 16;
repeated PbWorkerInfo decommissionWorkers = 17;
int64 shuffleTotalCount = 18;
- map<string, int64> shuffleFallbackCounts = 19;
+ int64 applicationTotalCount = 19;
+ map<string, int64> shuffleFallbackCounts = 20;
Review Comment:
ditto
```
map<string, int64> shuffleFallbackCounts = 19;
int64 applicationTotalCount = 20;
```
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2567760793
Ping @pan3793, @FMX, @RexXiong.
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1896906419
##########
common/src/main/proto/TransportMessages.proto:
##########
@@ -444,7 +444,9 @@ message PbHeartbeatFromApplication {
repeated PbWorkerInfo needCheckedWorkerList = 5;
bool shouldResponse = 6;
int64 shuffleCount = 7;
- map<string, int64> shuffleFallbackCounts = 8;
+ int64 applicationCount = 8;
+ map<string, int64> shuffleFallbackCounts = 9;
Review Comment:
And the order looks fine.
```
shuffleCount
shuffleFallbackCounts
applicationCount
applicationFallbackCounts
```
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2817063436
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2717052560
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2871462275
cc @RexXiong
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r2095469972
##########
assets/grafana/celeborn-dashboard.json:
##########
@@ -1710,6 +1710,186 @@
"x": 0,
"y": 90
},
+ "id": 239,
+ "options": {
+ "legend": {
+ "calcs": [],
+ "displayMode": "list",
+ "placement": "bottom",
+ "showLegend": true
+ },
+ "tooltip": {
+ "maxHeight": 600,
+ "mode": "single",
+ "sort": "none"
+ }
+ },
+ "targets": [
+ {
+ "datasource": {
+ "type": "prometheus",
+ "uid": "${DS_PROMETHEUS}"
+ },
+ "expr": "metrics_ApplicationTotalCount_Value{instance=~\"${instance}\"}",
Review Comment:
```suggestion
"expr": "metrics_ApplicationTotalCount_Value{role=\"Master\", instance=~\"${instance}\"}",
```
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1896905893
##########
common/src/main/proto/TransportMessages.proto:
##########
@@ -444,7 +444,9 @@ message PbHeartbeatFromApplication {
repeated PbWorkerInfo needCheckedWorkerList = 5;
bool shouldResponse = 6;
int64 shuffleCount = 7;
- map<string, int64> shuffleFallbackCounts = 8;
+ int64 applicationCount = 8;
+ map<string, int64> shuffleFallbackCounts = 9;
Review Comment:
Sorry, I am using the main branch for deployment, it is not necessary to change the order here.
```
map<string, int64> shuffleFallbackCounts = 8;
int64 applicationCount = 9;
```
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2560893359
## [Codecov](https://app.codecov.io/gh/apache/celeborn/pull/3026?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 `0%` with `10 lines` in your changes missing coverage. Please review.
> Project coverage is 32.66%. Comparing base [(`2eb4c23`)](https://app.codecov.io/gh/apache/celeborn/commit/2eb4c23eb8c87788daea66caa8e331e024d160b5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`95503c3`)](https://app.codecov.io/gh/apache/celeborn/commit/95503c36f9c269efc67a9cfd63bb663866367baa?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 4 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/celeborn/pull/3026?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...born/common/protocol/message/ControlMessages.scala](https://app.codecov.io/gh/apache/celeborn/pull/3026?src=pr&el=tree&filepath=common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fceleborn%2Fcommon%2Fprotocol%2Fmessage%2FControlMessages.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vY29tbW9uL3Byb3RvY29sL21lc3NhZ2UvQ29udHJvbE1lc3NhZ2VzLnNjYWxh) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/celeborn/pull/3026?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...org/apache/celeborn/common/util/PbSerDeUtils.scala](https://app.codecov.io/gh/apache/celeborn/pull/3026?src=pr&el=tree&filepath=common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fceleborn%2Fcommon%2Futil%2FPbSerDeUtils.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vY29tbW9uL3V0aWwvUGJTZXJEZVV0aWxzLnNjYWxh) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/celeborn/pull/3026?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 #3026 +/- ##
==========================================
- Coverage 32.99% 32.66% -0.33%
==========================================
Files 331 336 +5
Lines 19842 20042 +200
Branches 1787 1792 +5
==========================================
- Hits 6545 6544 -1
- Misses 12933 13134 +201
Partials 364 364
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/celeborn/pull/3026?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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei closed pull request #3026: [CELEBORN-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application
URL: https://github.com/apache/celeborn/pull/3026
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2891216794
Let us move forward, merging to 0.6.0
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#issuecomment-2889014351
anymore comments? @RexXiong
This feature is helpful for the ESS migration.
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1896906010
##########
common/src/main/proto/TransportMessages.proto:
##########
@@ -680,7 +682,9 @@ message PbSnapshotMetaInfo {
map<string, PbApplicationMeta> applicationMetas = 16;
repeated PbWorkerInfo decommissionWorkers = 17;
int64 shuffleTotalCount = 18;
- map<string, int64> shuffleFallbackCounts = 19;
+ int64 applicationTotalCount = 19;
+ map<string, int64> shuffleFallbackCounts = 20;
Review Comment:
```
map<string, int64> shuffleFallbackCounts = 19;
int64 applicationTotalCount = 20;
```
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1896915309
##########
master/src/main/java/org/apache/celeborn/service/deploy/master/clustermeta/ha/MetaHandler.java:
##########
@@ -150,8 +155,17 @@ public ResourceResponse handleWriteRequest(ResourceProtos.ResourceRequest reques
shuffleFallbackCounts.values().stream().mapToLong(v -> v).sum(),
appId);
}
+ Map<String, Long> applicationFallbackCounts =
+ request.getAppHeartbeatRequest().getApplicationFallbackCountsMap();
Review Comment:
```
if (CollectionUtils.isNotEmpty(applicationFallbackCounts)) {
LOG.warn(...);
}
```
##########
master/src/main/proto/Resource.proto:
##########
@@ -121,7 +121,9 @@ message AppHeartbeatRequest {
required int64 totalWritten = 3;
required int64 fileCount = 4;
optional int64 shuffleCount = 5;
- map<string, int64> shuffleFallbackCounts = 6;
+ optional int64 applicationCount = 6;
+ map<string, int64> shuffleFallbackCounts = 7;
+ map<string, int64> applicationFallbackCounts = 8;
Review Comment:
ditto
```
map<string, int64> shuffleFallbackCounts = 6;
optional int64 applicationCount = 7;
map<string, int64> applicationFallbackCounts = 8;
```
--
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-1800] Introduce ApplicationTotalCount and ApplicationFallbackCount metric to record the total and fallback count of application [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on code in PR #3026:
URL: https://github.com/apache/celeborn/pull/3026#discussion_r1912597824
##########
client-spark/spark-2/src/main/java/org/apache/spark/shuffle/celeborn/SparkShuffleManager.java:
##########
@@ -118,10 +123,24 @@ public <K, V, C> ShuffleHandle registerShuffle(
String appId = SparkUtils.appUniqueId(dependency.rdd().context());
initializeLifecycleManager(appId);
+ if (!registeredApps.contains(appId)) {
+ registeredApps.add(appId);
+ lifecycleManager.applicationCount().increment();
Review Comment:
`lifecycleManager.applicationCount().increment()` can move to `initializeLifecycleManager`
##########
client-spark/spark-2/src/main/java/org/apache/spark/shuffle/celeborn/SparkShuffleManager.java:
##########
@@ -118,10 +123,24 @@ public <K, V, C> ShuffleHandle registerShuffle(
String appId = SparkUtils.appUniqueId(dependency.rdd().context());
initializeLifecycleManager(appId);
+ if (!registeredApps.contains(appId)) {
Review Comment:
So why do we need registeredApps and fallbackApps sets? For Spark, I think there should only be a fallback for shuffle.
--
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