s0nskar commented on code in PR #3221:
URL: https://github.com/apache/celeborn/pull/3221#discussion_r2063360325


##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/Worker.scala:
##########
@@ -717,7 +717,7 @@ private[celeborn] class Worker(
       applicationId: String = null,
       resourceConsumption: ResourceConsumption = null): Unit = {
     var resourceConsumptionLabel = userIdentifier.toMap
-    if (applicationId != null)
+    if (applicationId != null && conf.metricsPushApplicationId)
       resourceConsumptionLabel += (resourceConsumptionSource.applicationLabel 
-> applicationId)

Review Comment:
   Replied here – 
https://github.com/apache/celeborn/pull/3221#discussion_r2063353314
   > if we remove this default behaviour will be send top app consumption with 
application label, which will break the resource consumption with 
userIdentifier as well.
   > IMO we should keep it or set the top resource consumption count default 
value to 0 + add a note about cardinality in the config documentation. WDYT?
   
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to