mxm commented on code in PR #945: URL: https://github.com/apache/flink-kubernetes-operator/pull/945#discussion_r1963310240
########## flink-autoscaler/src/main/java/org/apache/flink/autoscaler/ScalingMetricCollector.java: ########## @@ -149,19 +149,31 @@ public CollectedMetricHistory updateMetrics( // Add scaling metrics to history if they were computed successfully metricHistory.put(now, scalingMetrics); - if (isStabilizing) { - LOG.info("Stabilizing until {}", readable(stableTime)); - stateStore.storeCollectedMetrics(ctx, metricHistory); - return new CollectedMetricHistory(topology, Collections.emptySortedMap(), jobRunningTs); - } - var collectedMetrics = new CollectedMetricHistory(topology, metricHistory, jobRunningTs); if (now.isBefore(windowFullTime)) { - LOG.info("Metric window not full until {}", readable(windowFullTime)); + if (isStabilizing) { + LOG.info( + "Stabilizing... until {}. {} samples collected", Review Comment: True, we return the metrics from the stabilization period once the stabilization period is over. They are used to calculate the observed true processing rate. When the window is full, we clear the metrics from the stabilization period and mark the window as fully collected. I still think that returning this logging message ("XX samples collected") to the user during stabilization is confusing. It suggests that these metrics will be evaluated for scaling decisions, which they are not. We are merely measuring the processing capacity of the sources. ########## flink-autoscaler/src/main/java/org/apache/flink/autoscaler/utils/DateTimeUtils.java: ########## @@ -26,7 +26,7 @@ public class DateTimeUtils { private static final DateTimeFormatter DEFAULT_FORMATTER = - DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); + DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSS"); Review Comment: AFAIK we don't use any milliseconds in the tests, but advance only by full seconds. I think this may have unexpected consequences for the storage and logging layer. If this is a test-only change, then it should go into test. -- 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