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]