-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat: enable innodb page compression #24536
feat: enable innodb page compression #24536
Conversation
Signed-off-by: Akhil Narang <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop frappe/frappe#24536 +/- ##
===========================================
- Coverage 62.02% 62.00% -0.03%
===========================================
Files 786 786
Lines 75135 75135
Branches 6436 6436
===========================================
- Hits 46603 46585 -18
- Misses 24849 24851 +2
- Partials 3683 3699 +16
Flags with carried forward coverage won't be shown. Click here to find out more. |
@@ -23,6 +23,7 @@ if [ "$DB" == "mariadb" ] | |||
then | |||
mariadb --host 127.0.0.1 --port 3306 -u root -ptravis -e "SET GLOBAL character_set_server = 'utf8mb4'"; | |||
mariadb --host 127.0.0.1 --port 3306 -u root -ptravis -e "SET GLOBAL collation_server = 'utf8mb4_unicode_ci'"; | |||
mariadb --host 127.0.0.1 --port 3306 -u root -ptravis -e "SET GLOBAL innodb_compression_default = ON"; |
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.
Some notes:
- We can't enable this globally
- We already have a lot of sites where we might want to enable this.
This should be done via transform-database
command to optionally enable/disable on existing sites.
For new one if this works we can try it on per table basis.
Also this feature requires file per table option, so if it's not enabled enabling this won't do anything.
Also when I tried this I noticed no difference, lets first verify on reasonably large DB with DYNAMIC
row type to see if we do get some benefit. Most likely I missed SOME configuration step which prevented me from seeing any difference.
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.
Hmm based on my understanding, with the current defaults + enabling this, all new tables should use it by default atleast.
Will test out a bit here as well and see if I notice any differences in storage used.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing. |
Resolves frappe/caffeine#40