gyfora commented on code in PR #617:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/617#discussion_r1224696456


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/JobAutoScalerImpl.java:
##########
@@ -148,4 +162,22 @@ private AutoscalerFlinkMetrics 
getOrInitAutoscalerFlinkMetrics(
                         new AutoscalerFlinkMetrics(
                                 
ctx.getResourceMetricGroup().addGroup("AutoScaler")));
     }
+
+    private void initRecommendedParallelism(
+            Map<JobVertexID, Map<ScalingMetric, EvaluatedScalingMetric>> 
evaluatedMetrics) {
+        evaluatedMetrics.forEach(
+                (jobVertexID, evaluatedScalingMetricMap) ->
+                        evaluatedScalingMetricMap.put(
+                                RECOMMENDED_PARALLELISM,
+                                evaluatedScalingMetricMap.get(PARALLELISM)));
+        LOG.debug("Recommended parallelisms where initialized  {}", 
evaluatedMetrics);

Review Comment:
   I think the initialized / reset debug logs are an overkill. They don't 
provide any valuable info and are called in every iteration



##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/JobAutoScalerImpl.java:
##########
@@ -79,6 +85,8 @@ public boolean scale(FlinkResourceContext<? extends 
AbstractFlinkResource<?, ?>>
         var conf = ctx.getObserveConfig();
         var resource = ctx.getResource();
         var resourceId = ResourceID.fromResource(resource);
+        var flinkMetrics = getOrInitAutoscalerFlinkMetrics(ctx, resourceId);

Review Comment:
   we could now use the `flinkMetrics` in the catch block instead of 
reinitializing it



##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/JobAutoScalerImpl.java:
##########
@@ -137,6 +145,12 @@ public boolean scale(FlinkResourceContext<? extends 
AbstractFlinkResource<?, ?>>
                     EventRecorder.Component.Operator,
                     e.getMessage());
             return false;
+        } finally {
+            if (evaluatedMetrics != null) {
+                LOG.debug("Storing evaluated metrics {}", evaluatedMetrics);

Review Comment:
   I think either this or the logging of the evaluated metrics at line 120 
should be removed, otherwise this is going to be crazy verbose



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