Skip to content

Summary mail exhausts memory: complete pagevisit relation loaded per lead #81

Description

@t3-vfm

Summary mail exhausts memory: complete pagevisit relation loaded per lead

Environment: EXT:lux 44.3.0, TYPO3 13.4, PHP 8.2, CLI. Reproduced on 44.3.0; identical code
in 43.1.0.

What happens

lux:leadSendSummary runs out of memory. Measured on 44.3.0 with memory_limit = 1024M:

Lead set Page visits owned by those leads Result
67 leads 218,787 Allowed memory size of 1073741824 bytes exhausted in ObjectStorage.php:151
137 leads same, in AbstractDomainObject.php:147
full 24h window (capped at 750) ~872,000 same, in Session.php:126

The installation that surfaced this originally failed at memory_limit = 512M in
DataMapper.php:178. 67 leads are already enough to exhaust 1 GB.

Why

Several getters in Domain\Model\Visitor materialise a whole relation in order to return a single
row:

public function getPagevisitLast(): ?Pagevisit
{
    $pagevisits = $this->getPagevisits();      // loads every pagevisit of this visitor
    foreach ($pagevisits as $pagevisit) {
        return $pagevisit;                     // ...to take the first one
    }
    return null;
}

public function getPagevisitFirst(): ?Pagevisit
{
    $pagevisits = $this->getPagevisits();
    ksort($pagevisits);                        // ...and sorts all of them
    foreach ($pagevisits as $pagevisit) {
        return $pagevisit;
    }
    return null;
}

getHottestCategoryscoring() follows the same pattern via getCategoryscorings()->toArray() plus
uasort(), and getDateOfLastVisit() reaches getPagevisits() through getLastPagevisit().

Resources/Private/Templates/Mail/SummaryMail.html touches pagevisitFirst, pagevisitLast,
dateOfLastVisit and hottestCategoryscoring for every row, while
LuxLeadSendSummaryCommand caps the result set at a hardcoded $filter->setLimit(750).

On an installation whose active leads average a few thousand page visits each, that adds up to
hundreds of thousands of Pagevisit objects plus their related page records per run.

Reproduction

  1. An installation with some dozens of active leads that have long visit histories.
  2. vendor/bin/typo3 lux:leadSendSummary -- mail@example.org 86400 -1 0

Since setLimit(750) is hardcoded and nothing reduces the per-lead cost, the only workaround is
shrinking the result set via minimumScoring — which changes what the mail is supposed to report,
and as the table above shows it does not help much: even 67 high-scoring leads exceed 1 GB. On
shared hosting, where memory_limit often cannot be raised at all, the command is unusable.

Suggested fix

The repository already contains the right building blocks:

  • PagevisitRepository::findLatestDateByVisitor() does order by crdate desc limit 1 and is a
    drop-in replacement for getDateOfLastVisit().
  • PagevisitRepository::findOneByCompany() is an existing template for a per-visitor equivalent.
    findFirstByVisitor() / findLatestByVisitor() following the same shape would let
    getPagevisitFirst() / getPagevisitLast() avoid loading the relation entirely.
  • getHottestCategoryscoring() could use order by scoring desc limit 1.

Two caveats we could not resolve from the outside, which is why this report only suggests a
direction instead of shipping a patch:

  1. Authorization. getPagevisits() does not return the raw relation — it runs every row
    through Pagevisit::canBeRead(), which restricts non-admin backend users to the sites they are
    allowed to see. A plain ORDER BY crdate LIMIT 1 would bypass that check and could show an
    editor the last page visit of a site they have no permission for. Replicating the check in SQL
    means duplicating authorization logic — your call whether that is acceptable.
    (Note that the existing findLatestDateByVisitor() already queries without the site filter, so
    the inconsistency exists internally today.)

  2. In-memory state. The current getters also work for visitors that are not yet persisted,
    where page visits exist only in the in-memory ObjectStorage — for example during tracking,
    before persistAll(). Moving them to SQL changes that behaviour.

An alternative that avoids both problems would be to leave the model untouched and instead resolve
the four values per lead inside SendSummaryService before handing them to the view. That confines
the change to the path where the problem actually occurs, at the price of a different data
structure in SummaryMail.html.

Related: getLastPagevisit() additionally returns the same Pagevisit for every visitor because
of a static variable — fixed in #78.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions