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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
