enh(metadata): Introduce a memory limit for metadata generation#42800
enh(metadata): Introduce a memory limit for metadata generation#42800
Conversation
It's the ToDo point on the PR summary 😉 |
artonge
left a comment
artonge
left a comment
There was a problem hiding this comment.
Sorry for the delay, I forgot to submit the review
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Thank you, good idea 👍 I think the decision "can this file be processed" should live in the actual event listeners. But the event listeners I checked all read the file, and therefore it's a practical approach. The naming is a bit misleading because it's not a memory limit but a file size limit. |
If |
That's good, thank you 👍 |
|
Please don't forget to update the config.sample.php I'm not sure how useful the debug log is, but without you have no idea why the metadata generation was skipped for a given file. |
solracsf
force-pushed
the
metaGenMemLimit
branch
from
c5adc87 to
a88d4e0
Compare
solracsf
added
4. to release
|
/backport to stable29 |
st3iny
left a comment
st3iny
left a comment
There was a problem hiding this comment.
Amended to fix the commit message.
|
Psalm is failing because the latest rebase brought in #46450 that changed IConfig to IAppConfig. |
Signed-off-by: Git'Fellow <[email protected]> Co-Authored-By: Louis <[email protected]> Signed-off-by: Richard Steinmetz <[email protected]>
7 participants
Summary
Fix #42793
By introducing a (configurable) memory limit, one should not reach OOM on very big files that the server can't support.
TODO
metadata_max_filesizeparamSimilar to:
server/config/config.sample.php
Lines 1241 to 1249 in 265e906
Checklist