mxm commented on code in PR #493:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/493#discussion_r1057660487
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/ScalingMetricCollector.java:
##########
@@ -122,7 +130,15 @@ public CollectedMetrics getMetricsHistory(
// Add scaling metrics to history if they were computed successfully
scalingMetricHistory.put(clock.instant(), scalingMetrics);
- scalingInformation.updateMetricHistory(currentJobStartTs,
scalingMetricHistory);
+ scalingInformation.updateMetricHistory(currentJobUpdateTs,
scalingMetricHistory);
+
+ if (currentJobUpdateTs
+ .plus(stabilizationDuration)
+ .isAfter(clock.instant().minus(metricsWindowDuration))) {
Review Comment:
After thinking about this change more, it might actually be tricky for users
to set the minimum metric window size correctly. There could be unexpected side
effects if the delta between the two is too high, e.g. oscillation where the
minimum window scales up, the maximum window scales down, and so on. It seems
we only want to permit the minimum window for the initial scaling but not for
subsequent scaling decisions. We could keep `metrics.window` and introduce
`metrics.window.initial` which is only respected as long as the last scaling
decision didn't come from the regular window.
--
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]
