zentol commented on code in PR #27257:
URL: https://github.com/apache/flink/pull/27257#discussion_r2609918919


##########
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/slotmanager/FineGrainedSlotManager.java:
##########
@@ -227,10 +238,26 @@ public void start(
     }
 
     private void registerSlotManagerMetrics() {
-        slotManagerMetricGroup.gauge(

Review Comment:
   The original plan was indeed to just use a concurrent map as it is the 
simplest option.
   
   The proposal _is_ indeed more complex but it isn't overly so, while also 
being more general and easier to extend in the future, and can be a reasonable 
blueprint for other areas.
   That said, it does violate the general stance on metrics not incurring 
additional overhead, as we now update the metrics all the time no matter 
whether they are read though.
   But I think I still prefer that over pushing odd concurrency requirements 
down into the data structures that technically don't really need it. The 
tracker has no clue that metrics exist, and it'd just be arbitrary for _some_ 
maps to be concurrent, but on the other hand overkill to make the entire thing 
synchronized.



-- 
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