clarax commented on code in PR #685:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/685#discussion_r1368209456


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/KubernetesAutoScalerEventHandler.java:
##########
@@ -43,26 +52,71 @@ public void handleEvent(
             String reason,
             String message,
             @Nullable String messageKey,
-            @Nullable Duration interval) {
-        if (interval == null) {
-            eventRecorder.triggerEvent(
-                    context.getResource(),
-                    EventRecorder.Type.valueOf(type.name()),
-                    reason,
-                    message,
-                    EventRecorder.Component.Operator,
-                    messageKey,
-                    context.getKubernetesClient());
+            @Nonnull Duration interval) {
+        eventRecorder.triggerEvent(
+                context.getResource(),
+                EventRecorder.Type.valueOf(type.name()),
+                reason,
+                message,
+                EventRecorder.Component.Operator,
+                messageKey,
+                context.getKubernetesClient());
+    }
+
+    @Override
+    public void handleScalingEvent(
+            KubernetesJobAutoScalerContext context,
+            Map<JobVertexID, ScalingSummary> scalingSummaries,
+            boolean scaled,
+            @Nonnull Duration interval) {
+        if (scaled) {
+            AutoScalerEventHandler.super.handleScalingEvent(
+                    context, scalingSummaries, scaled, interval);
         } else {
-            eventRecorder.triggerEventByInterval(
+            var conf = context.getConfiguration();
+            var scalingReport = 
AutoScalerEventHandler.scalingReport(scalingSummaries, scaled);
+            var labels = Map.of(PARALLELISM_MAP_KEY, 
getParallelismHashCode(scalingSummaries));

Review Comment:
   Hi @1996fanrui, Thanks for the proposed alternative, just adding to @gyfora 
's comment, 
   "To give a bit more background on the messageKey , we use that to define the 
specific event when triggering in Kubernetes such that we keep the history 
(last triggered, etc) without always creating new event objects. This is 
completely independent of the interval / parallelism based triggering that are 
trying to do here.
   "
   
   Overriding messageKey will totally break the design of using messageKey to 
keep history of certain type of events at minimal cost. We keep using the same 
Kubernetes event object and just need to increment the count of the object and 
updating the messsages. The dedupe logic by interval is independent where we 
don't even update the kubenetes event object or trigger any operator events. 
The behaviors of your POC is different from this PR.



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