mxm commented on code in PR #533:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/533#discussion_r1107115459


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/config/AutoScalerOptions.java:
##########
@@ -153,4 +153,10 @@ private static ConfigOptions.OptionBuilder 
autoScalerConfig(String key) {
                     .defaultValue(MetricAggregator.MAX)
                     .withDescription(
                             "Metric aggregator to use for busyTime metrics. 
This affects how true processing/output rate will be computed. Using max allows 
us to handle jobs with data skew more robustly, while avg may provide better 
stability when we know that the load distribution is even.");
+
+    public static final ConfigOption<String> EXCLUDE_VERTEX_IDS =
+            autoScalerConfig("exclude.vertex.ids")
+                    .stringType()

Review Comment:
   This should be of list type.
   
   ```suggestion
                       .asList()
   ```



##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/config/AutoScalerOptions.java:
##########
@@ -153,4 +153,10 @@ private static ConfigOptions.OptionBuilder 
autoScalerConfig(String key) {
                     .defaultValue(MetricAggregator.MAX)
                     .withDescription(
                             "Metric aggregator to use for busyTime metrics. 
This affects how true processing/output rate will be computed. Using max allows 
us to handle jobs with data skew more robustly, while avg may provide better 
stability when we know that the load distribution is even.");
+
+    public static final ConfigOption<String> EXCLUDE_VERTEX_IDS =
+            autoScalerConfig("exclude.vertex.ids")
+                    .stringType()
+                    .defaultValue("")
+                    .withDescription("comma separated list of vertex ids in 
hexstring to disable for autoscaler to ignore for evaluating and scaling.");

Review Comment:
   This would be a semicolon-separated list in case of ListType.



##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/ScalingMetricEvaluator.java:
##########
@@ -66,12 +67,15 @@ public Map<JobVertexID, Map<ScalingMetric, 
EvaluatedScalingMetric>> evaluate(
         var scalingOutput = new HashMap<JobVertexID, Map<ScalingMetric, 
EvaluatedScalingMetric>>();
         var metricsHistory = collectedMetrics.getMetricHistory();
         var topology = collectedMetrics.getJobTopology();
+        var excludeVertexIdList = 
Arrays.asList(conf.get(AutoScalerOptions.EXCLUDE_VERTEX_IDS).split(","));

Review Comment:
   No need to do this here if we use a list type like other options, e.g.  see 
`classloader.parent-first-patterns.default`.



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