Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/videoplayer oom #2418

Open
wants to merge 2 commits into
base: feature/chat-multiple-doc-and-videoplayer-big-files
Choose a base branch
from

Conversation

panasetskaya
Copy link
Contributor

@panasetskaya panasetskaya commented Feb 10, 2025

As far as I understand, what happens here is that if we try to upload a too big file that exceeds the whole app JVM memory (maybe we even should use getRuntime().totalMemory() instead of .maxMemory() here - not sure),
then (despite that I tried as you see to buffer writing in the fileService - it does not help, I'm not sure if I should revert it as it was or is it still better than writing it all in the sink in one go?) the system starts urgently shutdown everything, and we can't even catch this OOM error bc there's nothing left that can catch it, so to speak :)
So here I'm making this file size check before we even start.

Combined with this user-friendly file size limit PR I think we're probably good. That linked PR has temporary hard-coded limitation according to our current BE requirements + user-friendly error handling.

@panasetskaya panasetskaya requested a review from a team as a code owner February 10, 2025 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant