[GitHub] [flink] Aitozi commented on a change in pull request #16207: [FLINK-23031][table-runtime]Make early processing time trigger d'ont repeat itself when no new element come in

[GitBox <gi...@apache.org>]

Aitozi commented on a change in pull request #16207:
URL: https://github.com/apache/flink/pull/16207#discussion_r781717534



##########
File path: flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/operators/window/triggers/ProcessingTimeTriggers.java
##########
@@ -59,28 +59,29 @@ private ProcessingTimeTriggers() {}
     }
 
     /**
-     * Creates a trigger that fires by a certain interval after reception of the first element.
+     * Creates a trigger that fires by a certain interval after reception of the element. If no new
+     * element comes, it will not repeat itself.
      *
      * @param time the certain interval
      */
-    public static <W extends Window> AfterFirstElementPeriodic<W> every(Duration time) {
-        return new AfterFirstElementPeriodic<>(time.toMillis());
+    public static <W extends Window> AfterElementNonPeriodic<W> every(Duration time) {
+        return new AfterElementNonPeriodic<>(time.toMillis());
     }
 
     /**
-     * Trigger every a given interval, the first trigger time is interval after the first element in
-     * the pane.
+     * Trigger fires by a certain interval after reception of the element. If no new element comes,
+     * it will not repeat itself.
      *
      * @param <W> type of window
      */
-    public static final class AfterFirstElementPeriodic<W extends Window> extends WindowTrigger<W> {
+    public static final class AfterElementNonPeriodic<W extends Window> extends WindowTrigger<W> {

Review comment:
       @wenlong88 Thanks for your quickly review. I refactor the method name and `toString()`. I also change the state descriptor to keep same. I think it will not break the compatibility.
   
   > I think the timer registered should not larger than end of window
   
   The trigger is not always used in the early trigger, It can also be used in late trigger. So I think we do not have to assume that the trigger time should not larger than end of window.
   




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