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


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/ScalingExecutor.java:
##########
@@ -121,13 +122,22 @@ public boolean scaleResource(
 
     private boolean stabilizationPeriodPassed(
             AbstractFlinkResource<?, ?> resource, Configuration conf) {
-        var now = clock.instant();
+        var jobStatus = resource.getStatus().getJobStatus();
+
+        if (!JobStatus.RUNNING.name().equals(jobStatus.getState())) {

Review Comment:
   I noticed while implementing that there are various invariants enforced in 
different layers. This per se is not a problem if they weren't leaky, i.e. we 
wouldn't pass the same data structures with different invariants through the 
layers. This becomes apparent in unit tests where implied invariants have to be 
modeled. One way to solve this would be to pass a different data structure, 
e.g. not pass JobStatus to lower levels but only the update time / running time.
   
   I think the check here doesn't hurt as it asserts an important invariant.



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