mxm commented on code in PR #711:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/711#discussion_r1400366983
##########
flink-autoscaler/src/main/java/org/apache/flink/autoscaler/config/AutoScalerOptions.java:
##########
@@ -154,6 +154,15 @@ private static ConfigOptions.OptionBuilder
autoScalerConfig(String key) {
+ RESTART_TIME.key()
+ "' will act as an upper bound.");
+ public static final ConfigOption<Duration> MAX_RESTART_TIME =
+ autoScalerConfig("restart.time.max")
+ .durationType()
+ .defaultValue(Duration.ofMinutes(30))
Review Comment:
We have internal overrides for all configuration defaults, but I don't think
this overly pessimistic value is a good default for the general public. I would
set this to not more than 15 minutes, even that is still conservative.
##########
flink-autoscaler/src/main/java/org/apache/flink/autoscaler/ScalingTracking.java:
##########
@@ -142,9 +142,11 @@ public double
getMaxRestartTimeSecondsOrDefault(Configuration conf) {
}
}
}
+ long restartTimeFromConfig =
conf.get(AutoScalerOptions.RESTART_TIME).toSeconds();
+ long maxRestartTimeFromConfig =
conf.get(AutoScalerOptions.MAX_RESTART_TIME).toSeconds();
return maxRestartTime == -1
? restartTimeFromConfig
Review Comment:
I think we should always cap the RESTART_TIME by the configured
MAX_RESTART_TIME (if configured).
--
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]
