You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "clarax (via GitHub)" <gi...@apache.org> on 2023/10/05 20:01:07 UTC

[PR] [FLINK-33187] Don't record duplicate event if no change [flink-kubernetes-operator]

clarax opened a new pull request, #683:
URL: https://github.com/apache/flink-kubernetes-operator/pull/683

   ## What is the purpose of the change
   
   https://issues.apache.org/jira/browse/FLINK-33187
   Suppress duplicate event for scaling report within the interval defined by a new operator config "scaling.report.interval" in second, defaulted to 1800.
   
   ## Brief change log
   
     - Add a new method in EventUtil to suppress duplicate event within an interval.
     - Suppress duplicate event for scaling report within the interval defined by a new operator config "scaling.report.interval" in second, defaulted to 1800.
   
   ## Verifying this change
   This change added tests and can be verified as follows:
   
     - manually verify by reading records
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): No
     - The public API, i.e., is any changes to the `CustomResourceDescriptors`: No
     - Core observer or reconciler logic that is regularly executed: No
   
   ## Documentation
   
     - Does this pull request introduce a new feature? No
     - If yes, how is the feature documented? docs
   


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33187] Don't record duplicate event if no change [flink-kubernetes-operator]

Posted by "clarax (via GitHub)" <gi...@apache.org>.
clarax commented on code in PR #683:
URL: https://github.com/apache/flink-kubernetes-operator/pull/683#discussion_r1349036555


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/config/AutoScalerOptions.java:
##########
@@ -168,4 +168,10 @@ private static ConfigOptions.OptionBuilder autoScalerConfig(String key) {
                     .defaultValues()
                     .withDescription(
                             "A (semicolon-separated) list of vertex ids in hexstring for which to disable scaling. Caution: For non-sink vertices this will still scale their downstream operators until https://issues.apache.org/jira/browse/FLINK-31215 is implemented.");
+
+    public static final ConfigOption<Long> SCALING_REPORT_INTERVAL =
+            autoScalerConfig("scaling.report.interval")
+                    .longType()

Review Comment:
   updated.



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33187] Don't record duplicate event if no change [flink-kubernetes-operator]

Posted by "gyfora (via GitHub)" <gi...@apache.org>.
gyfora commented on code in PR #683:
URL: https://github.com/apache/flink-kubernetes-operator/pull/683#discussion_r1348864871


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/ScalingExecutor.java:
##########
@@ -94,14 +94,15 @@ public boolean scaleResource(
         var scalingEnabled = conf.get(SCALING_ENABLED);
 
         var scalingReport = scalingReport(scalingSummaries, scalingEnabled);
-        eventRecorder.triggerEvent(
+        eventRecorder.triggerEventByInterval(

Review Comment:
   We should only control the events by interval if `scalingEnabled == false` (in advisor mode). Regular scaling should always trigger the event



##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/config/AutoScalerOptions.java:
##########
@@ -168,4 +168,10 @@ private static ConfigOptions.OptionBuilder autoScalerConfig(String key) {
                     .defaultValues()
                     .withDescription(
                             "A (semicolon-separated) list of vertex ids in hexstring for which to disable scaling. Caution: For non-sink vertices this will still scale their downstream operators until https://issues.apache.org/jira/browse/FLINK-31215 is implemented.");
+
+    public static final ConfigOption<Long> SCALING_REPORT_INTERVAL =
+            autoScalerConfig("scaling.report.interval")
+                    .longType()

Review Comment:
   This should be a `Duration` type config



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33187] Don't record duplicate event if no change [flink-kubernetes-operator]

Posted by "clarax (via GitHub)" <gi...@apache.org>.
clarax commented on code in PR #683:
URL: https://github.com/apache/flink-kubernetes-operator/pull/683#discussion_r1349038012


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/ScalingExecutor.java:
##########
@@ -94,14 +94,15 @@ public boolean scaleResource(
         var scalingEnabled = conf.get(SCALING_ENABLED);
 
         var scalingReport = scalingReport(scalingSummaries, scalingEnabled);
-        eventRecorder.triggerEvent(
+        eventRecorder.triggerEventByInterval(

Review Comment:
   updated.



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33187] Don't record duplicate event if no change [flink-kubernetes-operator]

Posted by "gyfora (via GitHub)" <gi...@apache.org>.
gyfora merged PR #683:
URL: https://github.com/apache/flink-kubernetes-operator/pull/683


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org