Fix infinite loop and unbounded batches in persistent quote cleanup cron

[TuVanDev]

Summary

The persistent_clear_expired cron job (Magento\Persistent\Observer\ClearExpiredCronJobObserver → Magento\Persistent\Model\CleanExpiredPersistentQuotes) cleans up expired persistent quotes in batches (default batchSize 500, set in Magento_Persistent/etc/di.xml). It has three related defects, all present unchanged in this branch's target.

Root cause

1. Unbounded loop. In processStoreQuotes(), $lastProcessedId only advances when quoteRepository->delete($quote) succeeds:

   $this->quoteRepository->delete($quote);
   $lastProcessedId = (int)$quote->getId();

   getExpiredPersistentQuotes() selects main_table.entity_id > $lastProcessedId. If every delete in a batch throws (for example, a foreign key from another table blocking the delete), the cursor never advances, the next iteration re-selects the identical batch, and while (true) never terminates.

2. batchSize has no effect on the query. In ExpiredPersistentQuotesCollection::getExpiredPersistentQuotes(), setOrder() and setPageSize() are applied to $additionalQuotes, but that collection's Select is only cloned to build a sub-select of matching entity IDs. The collection actually returned and iterated ($quotes) receives only a WHERE ... IN (...) and neither an ORDER BY nor a LIMIT:

   $quotes->getSelect()->where('main_table.entity_id IN (' . $selectQuoteIds . ')');

   Every batch loads the store's entire expired-quote backlog in one query, regardless of $batchSize.

3. Log volume. The catch block logs (string)$e, stringifying the whole exception including its stack trace, once per failed row.

Fix

• Move $lastProcessedId = (int)$quote->getId(); above the try, unconditional, so the cursor advances past a row once it has been visited, whether or not the delete succeeded.
• Add the order and limit to the collection that is returned:

  $quotes->getSelect()
      ->where('main_table.entity_id IN (' . $selectQuoteIds . ')')
      ->order('main_table.entity_id ' . Select::SQL_ASC)
      ->limit($batchSize);

  (The LIMIT can't live inside the IN (...) union sub-select on MySQL, which is why it sits on the outer collection.)
• Change the catch block to log $e->getMessage() instead of (string)$e.
• Update ExpiredPersistentQuotesCollectionTest to stub the new order()/limit() calls on the returned collection's Select mock (harmless before this change, since the original code never used that call's return value).

D1 (unconditional cursor advance) and D2 (the real LIMIT) must ship together, and the ORDER BY is not optional. With a real LIMIT and an unconditional cursor but no deterministic order, $lastProcessedId becomes the last row visited in an arbitrary batch rather than the true maximum, and rows in between are skipped permanently. Today that cannot happen only because the missing LIMIT makes every batch the whole remaining set.

Testing

This reproduces with core code only. The general condition is anything that makes QuoteRepository::delete() throw for the highest-id expired persistent quote in a store — for example a foreign key from a custom table referencing quote.entity_id with ON DELETE NO ACTION.

1. Enable persistence and get a quote the cron will select as expired: is_persistent = 1, updated_at older than the configured persistence lifetime, matching the customer_log conditions in getExpiredPersistentQuotes(). Note its entity_id, e.g. 123.
2. Block its deletion:

   CREATE TABLE quote_reference_test (
       entity_id INT UNSIGNED NOT NULL AUTO_INCREMENT,
       quote_id INT UNSIGNED NOT NULL,
       PRIMARY KEY (entity_id),
       CONSTRAINT FK_QUOTE_REFERENCE_TEST_QUOTE
           FOREIGN KEY (quote_id) REFERENCES quote (entity_id)
           ON DELETE NO ACTION ON UPDATE CASCADE
   ) ENGINE=InnoDB;
   
   INSERT INTO quote_reference_test (quote_id) VALUES (123);

3. Run the cron job (bin/magento cron:run --group default, or invoke CleanExpiredPersistentQuotes::execute($websiteId) directly).
   • Before: the delete on quote 123 throws every pass; $lastProcessedId never advances past it, so the same batch is re-selected forever. The job never terminates and the log fills with a full stack trace per iteration.
   • After: the cursor advances past quote 123 regardless, the loop finishes once each store's expired quotes are exhausted, quote 123 is the only one left behind (by design, since it's genuinely undeletable), and the log records one single-line message for it.
4. Clean up: DROP TABLE quote_reference_test;

Related

Same fix submitted upstream to magento/magento2: magento/magento2#41173

[rhoerr]

Thank you, looks good to me