This repository has been archived by the owner on Jun 4, 2024. It is now read-only.
Reduce limit of buffered messages from 1GB to 600 MB #7264
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
snosenzo
approved these changes
Dec 19, 2023
defunctzombie
approved these changes
Dec 19, 2023
There was a problem hiding this comment.
I think its worth adding some shorter version of your PR description as a code comment above the change maxTotalSizeBytes variable so we can remember some justification on 600MB.
I am 👍 to this change in the short term to allow us time to explore better caching strategies - primarily for streaming data where we want to avoid re-fetching if we can.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User-Facing Changes
Reduce limit of buffered messages to mitigate OOM crashes
Description
This PR reduces the limit of buffered messages from 1GB to 600 MB. Previously, with the cache size for preloaded and buffered messages both being limited to 1GB, the app could go well beyond the 2GB JS heap limit at which the app eventually gets killed by the browser (or electron). By reducing the cache size of buffered messages, we make these OOM crashes less likely. The downside of this approach is that less messages can be buffered which can be noticed mainly by the amount of messages buffered behind the current playback cursor.