Fix infinite loop and inert batch size in the persistent expired-quote cleanup cron

[TuVanDev]

Description (*)

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:

1. Unbounded loop. In processStoreQuotes(), $lastProcessedId is only advanced 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. This PR moves the assignment above the try, unconditional, so the cursor always advances past a row once it has been visited, whether or not the delete succeeded.

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 therefore loads the store's entire expired-quote backlog in one query, regardless of $batchSize. This PR adds 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 has to sit on the outer collection.)

   These first two fixes 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 between the two are silently skipped forever. Today that can't happen only because the missing LIMIT makes every batch the whole remaining set.

3. Log volume. The catch block logs (string)$e, which stringifies the whole exception including its stack trace, once per failed row. Changed to $e->getMessage().

Related Pull Requests

None.

Fixed Issues (if relevant)

N/A — no existing GitHub issue; reproducible directly on 2.4-develop (see manual testing scenarios).

Manual testing scenarios (*)

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

1. Enable persistence (Stores → Configuration → Customers → Persistent Shopping Cart) and create a quote that the cron will select as expired: is_persistent = 1, updated_at older than the configured persistence lifetime, and matching the customer_log conditions in getExpiredPersistentQuotes() (logged in and out, or logged in with an expired session). Note its entity_id, e.g. 123.

2. Create a table with a blocking foreign key to that quote, and a row referencing it:

   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: quoteRepository->delete() throws an integrity-constraint violation for quote 123 on every pass. $lastProcessedId never advances past its value from before quote 123, so getExpiredPersistentQuotes() re-selects the same batch (quote 123 and everything after it) every iteration. The job never terminates — it has to be killed — and the log fills with a full stack trace per iteration.
   • After: $lastProcessedId advances past quote 123 regardless of the delete failing, the loop makes progress through the rest of the batch and terminates 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, not one stack trace per iteration.

4. To confirm the LIMIT/ORDER BY specifically: with several dozen expired persistent quotes present and a query logger (or Xdebug breakpoint) on getExpiredPersistentQuotes(), compare the emitted SQL before and after — before, there is no LIMIT clause and the full expired set is fetched in one query; after, the query includes ORDER BY main_table.entity_id ASC LIMIT <batchSize>.

5. Clean up: DROP TABLE quote_reference_test;

Questions or comments

ExpiredPersistentQuotesCollectionTest::testGetExpiredPersistentQuotes mocks the collection's Select object for the returned collection but doesn't stub where()/order()/limit() on it (harmless before this change, since the original code never used that return value). This PR adds the missing stubs so the existing test continues to pass against the new order()/limit() calls; happy to adjust if maintainers prefer a different test shape.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests — existing CleanExpiredPersistentQuotesTest and ExpiredPersistentQuotesCollectionTest cover the changed methods; the latter is updated for the new order()/limit() calls.
• All automated tests passed successfully — not run against the full suite in this environment (no local Magento install); verified with php -l on all changed files and by manual trace of the existing unit tests against the diff.

[m2-assistant]

Hi @TuVanDev. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.