[GitHub] [flink] pnowojski commented on a change in pull request #14573: [FLINK-20868][task][metrics] Pause the idle/back pressure timers during processing mailbox actions

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

pnowojski commented on a change in pull request #14573:
URL: https://github.com/apache/flink/pull/14573#discussion_r553241937



##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/mailbox/MailboxDefaultAction.java
##########
@@ -59,6 +60,16 @@
          * Calling this method signals that the mailbox-thread should (temporarily) stop invoking
          * the default action, e.g. because there is currently no input available. This method must
          * be invoked from the mailbox-thread only!
+         *
+         * @param suspensionIdleTimer started (ticking) {@link TimerGauge} that measures how long
+         *     the default action was suspended/idling. If mailbox loop is busy processing mails,
+         *     this timer should be paused for the time required to process the mails.
+         */
+        Suspension suspendDefaultAction(TimerGauge suspensionIdleTimer);
+
+        /**
+         * Same as {@link #suspendDefaultAction(TimerGauge)} but without any associated timer
+         * measuring the idle time.
          */
         Suspension suspendDefaultAction();

Review comment:
       I don't like `null`s. I can tolerate them, if they are private, hidden and encapsulated in one file, but `suspendDefaultAction` is already an API. That's why I decided to overload the method here, and only use `@Nullable` in private methods one level deeper.




----------------------------------------------------------------
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:
users@infra.apache.org