search

beyondcode / laravel-mailbox

Catch incoming emails in your Laravel application
https://beyondco.de/docs/laravel-mailbox/getting-started/introduction
MIT License
1.04k stars 127 forks source link

Fix memory limit issues when running mailbox:cleanup #96

Closed emargareten closed 8 months ago

emargareten commented 2 years ago

I sometimes get a memory limit error when running mailbox:cleanup since the command fetches all columns of the table which includes entire emails + raw attachments, this PR optimizes the command to run the delete statement right on the database level.

LucaRed commented 2 years ago

This prevents model events from being fired upon deletion, though.

emargareten commented 2 years ago

If we need the event I can revert to select the id then delete each.

yehuda-hybiz commented 2 years ago

Can we get this fix merged? 👀

joelharkes commented 2 years ago

@emargareten fixed in my fork: joelharkes/laravel-mailbox version 3.0.0

I fixed it by paging per 1000. although im not sure how big the mails can get, if we have 1mb mails it can still lead to 1gb of memory.

emargareten commented 2 years ago

@joelharkes thanks, I think 1000 is still too much since the emails contain raw attachments (which can make each email itself use a large amount of memory), why not use the solution of this PR? (or https://github.com/beyondcode/laravel-mailbox/pull/96/commits/de8a6c4fd7e7aa85904df054c206a55f30aea18c if the deleted event is needed)

joelharkes commented 2 years ago

@emargareten deleted event might be used by others, wouldn't want to break it. I considered making a custom config variable to make it use the mass delete query, but might be easier just to make a second command that does that instead with another signature, its less cluttered.

Ill try to add that next.