[PR] [FLINK-39108][checkpoint] Respect disabled interval-during-backlog for the first checkpoint [flink]

["NestDream (via GitHub)" <gi...@apache.org>]

NestDream opened a new pull request, #29110:
URL: https://github.com/apache/flink/pull/29110

   ## What is the purpose of the change
   
   Fixes FLINK-39108. `execution.checkpointing.interval-during-backlog` is honored for steady-state scheduling but not for the first checkpoint. Two things combine to let it through:
   
   - `ScheduledTrigger#run` parks itself when it finds the effective interval disabled, but still triggers the current run. This is the path the disabled ITCase hits: the source reports backlog before its task is running, the scheduler is (re)started at task RUNNING with a zero initial delay, and that first run fires a checkpoint before parking.
   - `CheckpointCoordinator#setIsProcessingBacklog` only reschedules the periodic trigger when the new effective interval would fire earlier and skips the disabled case, so a trigger that is already armed when the backlog is reported stays armed and wakes up only to park.
   
   Net effect: at least one checkpoint is triggered during backlog even when checkpointing during backlog is disabled, which is what the ticket and the disabled `CheckpointIntervalDuringBacklogITCase#testNoCheckpointDuringBacklog` describe.
   
   ## Brief change log
   
   - `ScheduledTrigger#run` no longer triggers a checkpoint when it finds the effective interval disabled; it only parks
   - `setIsProcessingBacklog` cancels an already armed periodic trigger when the effective interval becomes disabled instead of letting it wake up and park; when the backlog ends, the existing reschedule-if-earlier path re-arms it (the parked trigger's next fire time is `Long.MAX_VALUE`), so the first checkpoint after a backlog comes one full interval after the backlog ends
   - re-enabled `CheckpointIntervalDuringBacklogITCase#testNoCheckpointDuringBacklog`; the `@Disabled` annotation referenced FLINK-39018, a typo for FLINK-39108
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   
   - `CheckpointIntervalDuringBacklogITCase#testNoCheckpointDuringBacklog` (the test disabled in #27630) reproduces the reported behavior against unmodified master: it fails with `Expecting AtomicInteger(2) to have value: 0`, i.e. two checkpoints ran while the first source was still in backlog. With the change the whole class passes, 4/4.
   - `CheckpointCoordinatorTriggeringTest#testFirstScheduledCheckpointNotTriggeredWhenBacklogCheckpointingDisabled`: new unit test for a periodic trigger that is already armed when the backlog is reported. It checks that the trigger is cancelled right away, that no checkpoint is triggered during the backlog, and that exactly one checkpoint is triggered once the backlog ends. Fails on master at the first assertion (the trigger stays armed), passes with the change.
   - `CheckpointCoordinatorTriggeringTest#testSchedulerStartedDuringBacklogDoesNotTriggerWhenBacklogCheckpointingDisabled`: new unit test for the ordering the ITCase hits (backlog reported before the scheduler is started). It checks that the first run of the armed trigger does not trigger a checkpoint and that checkpointing resumes once the backlog ends. Fails on master (one checkpoint is triggered), passes with the change.
   - End to end on a local MiniCluster: a job reading a HybridSource of two NumberSequenceSources with interval 1s and interval-during-backlog 0. On master, checkpoints 1 and 2 complete while the first source is still in backlog; with this change no checkpoint runs during backlog and checkpointing resumes right after the switch to the second source (7 checkpoints). Same result in 4 runs each.
   
   ## 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 changed class annotated with `@Public(Evolving)`: no
     - The serializers: no
     - The runtime per-record code paths (performance sensitive): no
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: yes, Checkpointing (scheduling of the periodic trigger)
     - The S3 file system connector: no
   
   ## Documentation
   
     - Does this pull request introduce a new feature? no
     - If yes, how is the feature documented? not applicable
   


-- 
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-39108][checkpoint] Respect disabled interval-during-backlog for the first checkpoint [flink]

["flinkbot (via GitHub)" <gi...@apache.org>]

flinkbot commented on PR #29110:
URL: https://github.com/apache/flink/pull/29110#issuecomment-5547694937

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b6b93a63c49c7dbf56e817fcbbd7bfbd4bc0eec0",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b6b93a63c49c7dbf56e817fcbbd7bfbd4bc0eec0",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b6b93a63c49c7dbf56e817fcbbd7bfbd4bc0eec0 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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-39108][checkpoint] Respect disabled interval-during-backlog for the first checkpoint [flink]

["NestDream (via GitHub)" <gi...@apache.org>]

NestDream commented on PR #29110:
URL: https://github.com/apache/flink/pull/29110#issuecomment-5547647623

   @rkhachatryan this re-enables testNoCheckpointDuringBacklog, the test you disabled in #27630. Could you take a look when you get a chance?


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