huyuanfeng2018 commented on code in PR #921:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/921#discussion_r1880095470


##########
flink-autoscaler/src/main/java/org/apache/flink/autoscaler/ScalingMetricEvaluator.java:
##########
@@ -296,8 +297,16 @@ protected static void computeProcessingRateThresholds(
             upperUtilization = 1.0;
             lowerUtilization = 0.0;
         } else {
-            upperUtilization = targetUtilization + utilizationBoundary;
-            lowerUtilization = targetUtilization - utilizationBoundary;
+            if (conf.getOptional(UTILIZATION_MAX).isPresent()
+                    || conf.getOptional(UTILIZATION_MIN).isPresent()
+                    || 
conf.getOptional(TARGET_UTILIZATION_BOUNDARY).isEmpty()) {
+                upperUtilization = conf.get(UTILIZATION_MAX);
+                lowerUtilization = conf.get(UTILIZATION_MIN);

Review Comment:
   I think as long as one of UTILIZATION_MAX and UTILIZATION_MIN is set, it 
means that the user is already using the new config.
   
   At the same time, if the user does not set TARGET_UTILIZATION_BOUNDARY, we 
still use the new config
   
   



##########
flink-autoscaler/src/main/java/org/apache/flink/autoscaler/config/AutoScalerOptions.java:
##########
@@ -89,13 +89,17 @@ private static ConfigOptions.OptionBuilder 
autoScalerConfig(String key) {
                                     + "seconds suffix, daily expression's 
formation is startTime-endTime, such as 9:30:30-10:50:20, when exclude from 
9:30:30-10:50:20 in Monday and Thursday "
                                     + "we can express it as 9:30:30-10:50:20 
&& * * * ? * 2,5");
 
-    public static final ConfigOption<Double> TARGET_UTILIZATION =
-            autoScalerConfig("target.utilization")
+    public static final ConfigOption<Double> UTILIZATION_TARGET =
+            autoScalerConfig("utilization.target")

Review Comment:
   
   > I understand its cleaner, but perhaps we can simply add the new keys and 
leave this unchanged?
   > Do we have to change this well-established key? This might confuse users, 
even though we have a fallback.
   
   I think methed `withFallbackKeys` good enough
   WDYT?
   



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