-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Increase Cursor Window Size to 20mb to load larger posts #20911
Conversation
Generated by 🚫 Danger |
Quality Gate passedIssues Measures |
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## trunk #20911 +/- ##
==========================================
- Coverage 40.95% 40.95% -0.01%
==========================================
Files 1516 1517 +1
Lines 69519 69523 +4
Branches 11467 11467
==========================================
Hits 28474 28474
- Misses 38459 38463 +4
Partials 2586 2586 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this. LGTM 🚀
Fixes #18727
We had previously bumped the cursor size to 5Mb in FluxC (#18774 and wordpress-mobile/WordPress-FluxC-Android#2776) and have been using that default in release builds ever since, but it seems like that wasn't enough as we still see some users crashing due to Cursor Window Size.
Ideally our data should not be stored with such a big size per row, but taking care of that requires a deep analysis and refactor of a huge part of the app and data storage within it. So as a smaller effort for now, let's bump the Cursor Window Size to 20mb (which we already use for
debug
builds) and monitor if we see fewer crashes inrelease
.Note: it's possible that increasing the cursor size could lead to OOM issues, but 20mb shouldn't be enough to cause that. We should monitor OOMs as well.
To Test:
I wasn't able to find a way of reproducing this issue as it requires having a big post, though I was not able to generate a fake large post easily.
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR Submission Checklist:
RELEASE-NOTES.txt
if necessary.Testing Checklist (strike-out the not-applying and unnecessary ones):