mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 10:04:18 +00:00
perf(rules): clear the EntityManager between apply-rule batches
walk() kept every message of every batch managed, so memory grew with the mailbox and each flush re-checked all earlier batches. Clear after each batch and re-find the rule, like RunBulkStatusHandler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
67935f0043
commit
77ed36117f
2 files changed
+61
-6
No files matched your search
@@ -5,6 +5,7 @@ declare(strict_types=1);
|
||||
namespace App\Infrastructure\Messaging\Handler;
|
||||
|
||||
use App\Domain\Enum\Mail\RuleRunState;
|
||||
use App\Entity\Rule\MailRule;
|
||||
use App\Infrastructure\Messaging\Message\ApplyMailRuleMessage;
|
||||
use App\Jmap\Query\EmailFilterCompiler;
|
||||
use App\Repository\Rule\MailRuleRepository;
|
||||
@@ -63,8 +64,10 @@ final readonly class ApplyMailRuleHandler
|
||||
$this->em->flush();
|
||||
$this->notifier->publish($rule);
|
||||
|
||||
$ruleId = (int) $rule->id;
|
||||
|
||||
try {
|
||||
$this->walk($rule);
|
||||
$rule = $this->walk($rule);
|
||||
} catch (Throwable $e) {
|
||||
$this->logger->error('ApplyMailRule: run failed', [
|
||||
'ruleId' => $rule->id,
|
||||
@@ -73,7 +76,21 @@ final readonly class ApplyMailRuleHandler
|
||||
]);
|
||||
|
||||
// Recorded rather than swallowed: a run that stopped halfway is
|
||||
// something the user has to be able to see and retry.
|
||||
// something the user has to be able to see and retry. Re-read,
|
||||
// because walk() clears the EntityManager between batches and the
|
||||
// $rule in hand may no longer be managed. A failed flush also
|
||||
// closes the EntityManager — then there is nothing to record into.
|
||||
if (false === $this->em->isOpen()) {
|
||||
return;
|
||||
}
|
||||
|
||||
$this->em->clear();
|
||||
$rule = $this->ruleRepository->find($ruleId);
|
||||
|
||||
if (null === $rule) {
|
||||
return;
|
||||
}
|
||||
|
||||
$rule->runState = RuleRunState::Failed;
|
||||
$rule->runFinishedAt = new DateTimeImmutable();
|
||||
$this->em->flush();
|
||||
@@ -88,21 +105,34 @@ final readonly class ApplyMailRuleHandler
|
||||
$this->notifier->publish($rule);
|
||||
}
|
||||
|
||||
private function walk(\App\Entity\Rule\MailRule $rule): void
|
||||
/**
|
||||
* @return MailRule the rule as it is managed NOW — the one passed in is
|
||||
* detached by the first batch's clear()
|
||||
*/
|
||||
private function walk(MailRule $rule): MailRule
|
||||
{
|
||||
$ruleId = (int) $rule->id;
|
||||
$filter = $this->compiler->compile($rule->conditions);
|
||||
$afterId = 0;
|
||||
|
||||
while (true) {
|
||||
// The user can be null only if it was deleted mid-run; the rule
|
||||
// row goes with it (cascade), so there is nothing left to apply.
|
||||
$user = $rule->usr;
|
||||
|
||||
if (null === $user) {
|
||||
return $rule;
|
||||
}
|
||||
|
||||
$ids = $this->messageRepository->findIdsMatchingForUser(
|
||||
$rule->usr,
|
||||
$user,
|
||||
$filter,
|
||||
$afterId,
|
||||
self::BATCH_SIZE,
|
||||
);
|
||||
|
||||
if (0 === count($ids)) {
|
||||
return;
|
||||
return $rule;
|
||||
}
|
||||
|
||||
foreach ($this->messageRepository->findByIds($ids) as $entity) {
|
||||
@@ -121,6 +151,19 @@ final readonly class ApplyMailRuleHandler
|
||||
|
||||
$this->em->flush();
|
||||
$this->notifier->publish($rule);
|
||||
|
||||
// Without this every message the run has touched stays managed:
|
||||
// memory grows with the mailbox, and each flush re-checks every
|
||||
// entity of every earlier batch, so the run slows down as it goes.
|
||||
// Same discipline as RunBulkStatusHandler and ResortMailboxHandler.
|
||||
$this->em->clear();
|
||||
|
||||
$rule = $this->ruleRepository->find($ruleId);
|
||||
|
||||
if (null === $rule) {
|
||||
// Deleted while running. Nothing to report progress on.
|
||||
throw new \RuntimeException(sprintf('Rule %d disappeared mid-run.', $ruleId));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -108,6 +108,18 @@ final class ApplyMailRuleHandlerTest extends KernelTestCase
|
||||
|
||||
$handler(new ApplyMailRuleMessage((int) $rule->id));
|
||||
|
||||
// The walk clears the EntityManager after every batch, or a large
|
||||
// mailbox holds every message it touched until the worker dies.
|
||||
self::assertCount(
|
||||
0,
|
||||
$this->em->getUnitOfWork()->getIdentityMap()[Message::class] ?? [],
|
||||
'The walk kept processed messages managed.',
|
||||
);
|
||||
|
||||
// Cleared, so the rule in hand is a stale copy: read the row back.
|
||||
$rule = $this->em->getRepository(MailRule::class)->find($rule->id);
|
||||
self::assertNotNull($rule);
|
||||
|
||||
self::assertSame(RuleRunState::Completed, $rule->runState);
|
||||
self::assertSame(7, $rule->runProcessed, 'Every matching message should be counted.');
|
||||
self::assertNotNull($rule->runStartedAt);
|
||||
@@ -117,7 +129,7 @@ final class ApplyMailRuleHandlerTest extends KernelTestCase
|
||||
$labelled = 0;
|
||||
|
||||
foreach ($this->messages->findByIds($this->matchingIds()) as $message) {
|
||||
if (true === $message->labels->contains($this->label)) {
|
||||
if (true === $message->labels->exists(fn ($i, Label $l): bool => $l->id === $this->label->id)) {
|
||||
$labelled++;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user