[GitHub] tillrohrmann commented on a change in pull request #6702: [FLINK-10135] The JobManager does not report the cluster-level metrics

[GitBox <gi...@apache.org>]

tillrohrmann commented on a change in pull request #6702: [FLINK-10135] The JobManager does not report the cluster-level metrics
URL: https://github.com/apache/flink/pull/6702#discussion_r223690671
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java
 ##########
 @@ -734,6 +744,18 @@ public void requestHeartbeat(ResourceID resourceID, Void payload) {
 		}
 	}
 
+	private void registerSlotAndTaskExecutorMetrics() {
+		jobManagerMetricGroup.gauge(
+			TASK_SLOTS_AVAILABLE_METRIC_NAME,
+			() -> (long) slotManager.getNumberFreeSlots());
 
 Review comment:
   I think it should be okish to call these methods from a different thread. All of them simply return the size of a `HashMap`. In the worst case, we would report a little bit stale data. The main danger I see is that the implementations of `SlotManager` might change, which makes this no longer hold true.
   
   If we wanted to make it "thread-safe", then we would need to make the access call inside of the `MainThreadExecutor`:
   ```
   jobManagerMetricGroup.gauge(
   	MetricNames.TASK_SLOTS_AVAILABLE,
   	() -> callAsync(() -> slotManager.getNumberFreeSlots(), timeout).get());
   ```
   
   However, this might be an overkill at the moment.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services