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


##########
flink-autoscaler/src/main/java/org/apache/flink/autoscaler/ScalingExecutor.java:
##########
@@ -264,4 +262,21 @@ private static Map<String, String> 
getVertexParallelismOverrides(
                 });
         return overrides;
     }
+
+    private boolean blockScalingExecution(
+            Context context,
+            Map<JobVertexID, ScalingSummary> scalingSummaries,
+            Configuration conf,
+            Instant now) {
+        var scaleEnabled = conf.get(SCALING_ENABLED);
+        var isExcluded = CalendarUtils.inExcludedPeriods(conf, now);
+        autoScalerEventHandler.handleScalingEvent(
+                context,
+                scalingSummaries,
+                scaleEnabled,
+                isExcluded,
+                conf.get(SCALING_EVENT_INTERVAL));
+
+      return !scaleEnabled || isExcluded;

Review Comment:
   In the current logic we send 2 types of events on a parallelism change:
    1. Scaling Recommendation (when disabled) -> `Recommended parallelism 
change: {parallelisms}`
    2. Scaling Event (when enabled) -> `Scaling Vertices: {parallelisms}`
    
   The PR basically adds a 3rd type of event which looks like a recommendation 
but it doesn't say recommendation it prints:
   
   `Scaling execution disabled by config: configkey:false {parallelism:map}`
   
   I don't think that this is fully consistent with the current logic. We 
intended to keep the recommendations, but we changed the code so that it's not 
a recommendation anymore but looks different.



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

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

Reply via email to