aljoscha commented on a change in pull request #13969:
URL: https://github.com/apache/flink/pull/13969#discussion_r518767382



##########
File path: 
flink-core/src/main/java/org/apache/flink/api/connector/sink/SinkWriter.java
##########
@@ -57,6 +59,14 @@
         */
        List<WriterStateT> snapshotState();
 
+       /**
+        * Called when a timer fired.
+        *
+        * @param time The timestamp of the firing timer.
+        * @param context The context that user can use to register a 
processing timer when calling {@link #onTimer(long, OnTimerContext)}.
+        */
+       default void onTimer(long time, OnTimerContext context) {

Review comment:
       I believe this one and the interface below are unused because a timer is 
registered with a `ProcessingTimerCallback`.

##########
File path: 
flink-core/src/main/java/org/apache/flink/api/connector/sink/Sink.java
##########
@@ -92,4 +98,37 @@
                 */
                MetricGroup metricGroup();
        }
+
+       /**
+        * This service is responsible for executing user's given callback at 
given timestamp.
+        */
+       interface ProcessingTimerService {

Review comment:
       I think this should be `ProcessingTimeService`

##########
File path: 
flink-core/src/main/java/org/apache/flink/api/connector/sink/Sink.java
##########
@@ -82,6 +83,11 @@
         */
        interface InitContext {
 
+               /**
+                * @return A processing timer service.
+                */

Review comment:
       ```suggestion
                /**
                 * Returns a {@link ProcessingTimeService} that can be used to 
get the current time and register timers.
                 */
   ```

##########
File path: 
flink-core/src/main/java/org/apache/flink/api/connector/sink/Sink.java
##########
@@ -92,4 +98,37 @@
                 */
                MetricGroup metricGroup();
        }
+
+       /**
+        * This service is responsible for executing user's given callback at 
given timestamp.
+        */
+       interface ProcessingTimerService {
+
+               /**
+                * @return Current process time.
+                */
+               long getCurrentProcessingTime();
+
+               /**
+                * Invoking the given callback at the given timestamp.
+                *
+                * @param time Time when the callback is invoked at
+                * @param processingTimerCallback The callback to be invoked.
+                */
+               void registerProcessingTimer(long time, ProcessingTimerCallback 
processingTimerCallback);
+
+               /**
+                * The callback that could be register at {@link 
ProcessingTimerService}.
+                */
+               interface ProcessingTimerCallback {

Review comment:
       Should be `ProcessingTimeCallback` to conform to the rest of the code 
base.




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to