[dillardblom]

Summary

When a mailbox isn't cached yet, the client syncs it with init: true and no known ids (initializeCache() in Mailbox.vue), and then loads the first page from the message list endpoint. The initial sync returned every message of the mailbox as new anyway: getDatabaseSyncChanges() treated the cache as unknown to the client and ran each message through the PreviewEnhancer.

On an INBOX with 33,000 messages that request ran out of PHP memory (1 GB) in the MIME parser and charset converter, and the client retried it, so the log filled with Allowed memory size … exhausted errors. The same mailbox syncs fine from occ mail:account:sync (peak 79 MB), because that path doesn't run the database diff.

The initial sync now only fills the cache and returns the mailbox stats. Partial syncs are unchanged.

Thanks to @stiller for tracing the request to initializeCache().

Test plan

• Unit test: the initial sync returns no messages and doesn't read the cache. It fails without the change.
• A partial sync still reads the cache.
• SyncServiceTest and MailboxesControllerTest green, php-cs-fixer clean on the changed files.
• CI

[stiller]

Since this PR overlaps with my own goals, I tested this on my local stack:

Nextcloud 33.0.9, PHP 8.4 with memory_limit=2G, Postgres 16, Stalwart 0.16 (IMAP), a 33k INBOX cached by occ with no rows structure-analyzed (occ peak 72 MB), and a 95k Archive.

Sync without known ids

• Without the PR, POST …/sync with ids: [] runs out of memory (Allowed memory size of 2147483648 bytes exhausted) after 86–91 s, with no query, is:pi-important, is:pi-other and sortOrder: oldest. With the PR: 200 in 0.6–3.3 s, about 21 KB, 20 messages.
• Once every row is analyzed, the same request without the PR succeeds in 4.3 s (27.7 MB, all 33k envelopes), which confirms the cost is in the preview enhancer path.
• The 20 returned are the same ids as the list endpoint's first page for newest-first, is:pi-important, is:pi-other and oldest-first, so the oldest-first fix works. (For oldest-first they arrive in sent_at DESC order; the client re-sorts them.)

Sync with known ids (unchanged)

• A known-ids sync after the first page still delivers new mail (+5 delivered → new: 5).
• A burst of 30 new messages into a folder the client showed as empty displays 20; scrolling loads the other 10.

Trigger

On current main's frontend with a cached INBOX, I could not get the priority inbox to send ids: [] (one and two accounts, with new mail arriving). The empty-ids requests I did see came from the first open of an uncached mailbox, which this PR fixes as well:

First open	Without the PR	With the PR
33k INBOX	OOM after 92 s, no list in 10 min	list at 35 s
95k Archive	OOM ×4, no list in 10 min	list at 96 s
Priority inbox	no list in 10 min	Important section at 68 s
All inboxes	no list	list at 40 s

Two small points on the change

• The new test covers newest-first without a query only. There's no test for the oldest-first fix or for the query + no-known-ids branch.
• The sync endpoint has no view parameter, so the page is always the threaded one; in singleton view it can differ from the list's first page.

Hope this helps.

[dillardblom]

Thanks a lot for testing this so thoroughly, on mailboxes that size, and for the comparison table. That is more than I could reproduce here.

Both points:

• Tests: added in f90aef7. They cover the first page in oldest-first order, the first page with a filter (is:pi-important, at most 20, with the parsed query), and the known-ids path with a filter in oldest-first order (new ids and known ids are both matched in the requested order).
• view: right, the sync endpoint has no view, so the first page is the threaded one. In singleton view it can leave out messages that are not the newest of their thread, until the list loads them. Before this PR the same request returned every message of the mailbox, so nothing was missing there, but that is also what ran out of memory. The known-ids path with a filter has the same gap already today. Passing the view from the frontend to the sync request would close it for both. I would rather do that in a small follow-up, to keep this fix focused, unless the maintainers prefer it here.

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[ChristophWurst]

Thanks for the patch

I don't have the time for a deep dive but from what I remember about the sync logic is that there are initial and partial syncs. And the OOM only happens if an incorrect partial sync is performed.
Limiting to 20 is only curing the symptom. It doesn't address that the request should not happen that way. Try to find out why the frontend sends it like this, and if there is some kind of unmet precondition or race condition.

[stiller]

In my tests the empty ids request was always the initial sync, not a partial one. It comes from initializeCache() in Mailbox.vue when an uncached mailbox is opened for the first time.

With init: true the server syncs and then returns every message in the mailbox as new, running all of them through the preview enhancer. That's where it ran out of memory on a 33k inbox. The client doesn't need that response, because initializeCache() loads the first page from the list endpoint right after.

So maybe the initial sync just shouldn't return new messages. I couldn't reproduce the priority inbox case from the PR description, so that one may be different.

[ChristophWurst]

So maybe the initial sync just shouldn't return new messages

Yes, agree. Initial sync should not be handled like a partial one.

[dillardblom]

Thanks both. @stiller is right, and I can't find the priority inbox case from my original description in the code either: it was most likely the same initial sync being retried. Sorry for the noise there.

I've reworked the PR as suggested: the initial sync no longer returns new messages, only the mailbox stats, and the 20-message limit is gone. The client already loads the first page from the message list right after initializeCache(). The description is updated.