Re: [PR] [CELEBORN-1744] Cleanup old ResourceConsumption metrics [celeborn]

2024-11-26 Thread via GitHub


turboFei commented on PR #2946:
URL: https://github.com/apache/celeborn/pull/2946#issuecomment-2502096877

   it should be resolved by https://github.com/apache/celeborn/pull/2954, 
closing this one.


-- 
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-unsubscr...@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] [CELEBORN-1744] Cleanup old ResourceConsumption metrics [celeborn]

2024-11-26 Thread via GitHub


turboFei closed pull request #2946: [CELEBORN-1744] Cleanup old 
ResourceConsumption metrics
URL: https://github.com/apache/celeborn/pull/2946


-- 
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-unsubscr...@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] [CELEBORN-1744] Cleanup old ResourceConsumption metrics [celeborn]

2024-11-26 Thread via GitHub


turboFei commented on PR #2946:
URL: https://github.com/apache/celeborn/pull/2946#issuecomment-2501715514

   Raised https://github.com/apache/celeborn/pull/2954 to fix the incorrect app 
metrics.


-- 
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-unsubscr...@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] [CELEBORN-1744] Cleanup old ResourceConsumption metrics [celeborn]

2024-11-26 Thread via GitHub


turboFei commented on PR #2946:
URL: https://github.com/apache/celeborn/pull/2946#issuecomment-2501578302

   Will address the applicationId label metrics in another pr.


-- 
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-unsubscr...@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] [CELEBORN-1744] Cleanup old ResourceConsumption metrics [celeborn]

2024-11-25 Thread via GitHub


FMX commented on PR #2946:
URL: https://github.com/apache/celeborn/pull/2946#issuecomment-2499825364

   Roger that. Will review it tonight.
   
   > 2024年11月26日 15:07,Fei Wang ***@***.***> 写道:
   > 
   > 
   > gentle ping @SteNicholas  @FMX 

   > —
   > Reply to this email directly, view it on GitHub 
, or 
unsubscribe 
.
   > You are receiving this because you were mentioned.
   > 
   
   


-- 
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-unsubscr...@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] [CELEBORN-1744] Cleanup old ResourceConsumption metrics [celeborn]

2024-11-25 Thread via GitHub


turboFei commented on code in PR #2946:
URL: https://github.com/apache/celeborn/pull/2946#discussion_r1857925024


##
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/Worker.scala:
##
@@ -677,8 +677,6 @@ private[celeborn] class Worker(
   private def handleTopResourceConsumption(userResourceConsumptions: util.Map[
 UserIdentifier,
 ResourceConsumption]): Unit = {
-// Remove application top resource consumption gauges to refresh top 
resource consumption metrics.
-
removeAppResourceConsumption(topApplicationUserIdentifiers.keySet().asScala)

Review Comment:
   I wonder the metrics with appId label lost is related with 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-unsubscr...@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] [CELEBORN-1744] Cleanup old ResourceConsumption metrics [celeborn]

2024-11-25 Thread via GitHub


turboFei commented on PR #2946:
URL: https://github.com/apache/celeborn/pull/2946#issuecomment-2499822879

   gentle ping @SteNicholas @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-unsubscr...@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] [CELEBORN-1744] Cleanup old ResourceConsumption metrics [celeborn]

2024-11-25 Thread via GitHub


turboFei commented on code in PR #2946:
URL: https://github.com/apache/celeborn/pull/2946#discussion_r1857919158


##
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/Worker.scala:
##
@@ -677,8 +677,6 @@ private[celeborn] class Worker(
   private def handleTopResourceConsumption(userResourceConsumptions: util.Map[
 UserIdentifier,
 ResourceConsumption]): Unit = {
-// Remove application top resource consumption gauges to refresh top 
resource consumption metrics.
-
removeAppResourceConsumption(topApplicationUserIdentifiers.keySet().asScala)

Review Comment:
   we can leave it be cleaned by the cleaner thread?



-- 
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-unsubscr...@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] [CELEBORN-1744] Cleanup old ResourceConsumption metrics [celeborn]

2024-11-25 Thread via GitHub


turboFei commented on code in PR #2946:
URL: https://github.com/apache/celeborn/pull/2946#discussion_r1857916909


##
common/src/main/scala/org/apache/celeborn/common/meta/WorkerInfo.scala:
##
@@ -239,10 +239,7 @@ class WorkerInfo(
   def updateThenGetUserResourceConsumption(resourceConsumptions: util.Map[
 UserIdentifier,
 ResourceConsumption]): util.Map[UserIdentifier, ResourceConsumption] = {
-
userResourceConsumption.keys().asScala.filterNot(resourceConsumptions.containsKey).foreach
 {
-  identifier =>
-userResourceConsumption.put(identifier, ResourceConsumption(0, 0, 0, 
0))
-}
+userResourceConsumption.clear()

Review Comment:
   if we always reserve the ResourceConsumption with 0, seems it would not be 
cleaned. 



-- 
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-unsubscr...@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org