[GitHub] [beam] boyuanzz commented on a change in pull request #12836: [BEAM-10887] Expose clear() to FnApiTimer.

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

boyuanzz commented on a change in pull request #12836:
URL: https://github.com/apache/beam/pull/12836#discussion_r489074544



##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/state/Timer.java
##########
@@ -81,6 +81,9 @@
    */
   void setRelative();
 
+  /** Clears the timer. */

Review comment:
       There are 2 conditions that we know a cleared timer should never been fired:
   
   - Timer(key = A) is set first then cleared within the same bundle.
   
   - A never-set timer is cleared.
   
   One condition that we know a cleared timer will be fired:
   
   - Timer(key = A) is cleared first then set again.
   
   And the contract is vague under certain conditions(e.g, the runner may or may not clear the timer depends on the implementation details of the runner):
   
   - Timer(key = A)  is being scheduled by service meanwhile user code just clears Timer(key = A). This happens when set and clear calls happen on different bundles.
   
   I'll add ValidatesRunner tests to cover these. 
   




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