Skip to content

Commit 798a36a

Browse files
committed
bug symfony#52307 [Scheduler] Save checkpoint in a finally block (FrancoisPog)
This PR was squashed before being merged into the 6.3 branch. Discussion ---------- [Scheduler] Save checkpoint in a finally block | Q | A | ------------- | --- | Branch? | 6.3 <!-- see below --> | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Issues | Fix symfony#52288 <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead --> | License | MIT As described in the linked issue : > The message generator appears to store stateful data by loading and saving the checkpoint with the cache. > > But if the worker is configured to stop after X message(s), once the last message is handled, the (messenger) worker will break its loop and stop. In this case, the execution stop and the stateful data of the message (time of last execution for example) will not be saved. > > So when the consumer restarts it will re-handle the message because the last execution was not stored. > So the solution here is to put the code that save the checkpoint in a `finally` block. Thanks [maelanleborgne](https://github.com/maelanleborgne). <!-- Replace this notice by a description of your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the latest branch. - For new features, provide some code snippets to help understand usage. - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> Commits ------- 598d5bd [Scheduler] Save checkpoint in a finally block
2 parents a9b9e4e + 598d5bd commit 798a36a

File tree

2 files changed

+33
-2
lines changed

2 files changed

+33
-2
lines changed

src/Symfony/Component/Scheduler/Generator/MessageGenerator.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,11 @@ public function getMessages(): \Generator
7272
}
7373

7474
if ($yield) {
75-
yield (new MessageContext($this->name, $id, $trigger, $time, $nextTime)) => $message;
76-
$checkpoint->save($time, $index);
75+
try {
76+
yield (new MessageContext($this->name, $id, $trigger, $time, $nextTime)) => $message;
77+
} finally {
78+
$checkpoint->save($time, $index);
79+
}
7780
}
7881
}
7982

src/Symfony/Component/Scheduler/Tests/Generator/MessageGeneratorTest.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Cache\Adapter\ArrayAdapter;
1616
use Symfony\Component\Clock\ClockInterface;
17+
use Symfony\Component\Clock\MockClock;
18+
use Symfony\Component\Scheduler\Generator\Checkpoint;
1719
use Symfony\Component\Scheduler\Generator\MessageContext;
1820
use Symfony\Component\Scheduler\Generator\MessageGenerator;
1921
use Symfony\Component\Scheduler\RecurringMessage;
@@ -128,6 +130,32 @@ public function testYieldedContext()
128130
$this->assertEquals(self::makeDateTime('22:16:00'), $context->nextTriggerAt);
129131
}
130132

133+
public function testCheckpointSavedInBrokenLoop()
134+
{
135+
$clock = new MockClock(self::makeDateTime('22:12:00'));
136+
137+
$message = $this->createMessage((object) ['id' => 'message'], '22:13:00', '22:14:00', '22:16:00');
138+
$schedule = (new Schedule())->add($message);
139+
140+
$cache = new ArrayAdapter();
141+
$schedule->stateful($cache);
142+
$checkpoint = new Checkpoint('dummy', cache: $cache);
143+
144+
$scheduler = new MessageGenerator($schedule, 'dummy', clock: $clock, checkpoint: $checkpoint);
145+
146+
// Warmup. The first run is always returns nothing.
147+
$this->assertSame([], iterator_to_array($scheduler->getMessages(), false));
148+
149+
$clock->sleep(60 + 10); // 22:13:10
150+
151+
foreach ($scheduler->getMessages() as $message) {
152+
// Message is handled but loop is broken just after
153+
break;
154+
}
155+
156+
$this->assertEquals(self::makeDateTime('22:13:00'), $checkpoint->time());
157+
}
158+
131159
public static function messagesProvider(): \Generator
132160
{
133161
$first = (object) ['id' => 'first'];

0 commit comments

Comments
 (0)