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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
