-
Notifications
You must be signed in to change notification settings - Fork 1.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
Update SQLite3 max_page_count to match current defaults #5114
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #5114 +/- ##
=======================================
Coverage 76.2% 76.2%
=======================================
Files 760 760
Lines 61489 61489
Branches 8115 8110 -5
=======================================
+ Hits 46832 46843 +11
+ Misses 14657 14646 -11 |
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.
looks good to me
I'm not sure I understand that future task section. We are very unlikely to downgrade our current version. Why do we need to check if older version work with this new default?
These settings should be configurable imo. Also, updating the max_page_count might fix the issue today but we will not be able to update this again. We should be increasing the page_size for FH nodes. |
We had a discussion like this a couple of days ago. We thought there's no point to make max_page_count configurable. But regarding page_size - yes, there's a plan to increase a default value and make it configurable. |
@dangell7 this PR sets max_page_count to the highest value currently supported in SQLite3. I don't know if there would be a reason for someone to desire a lower value. We could also increase page_size or make that configurable. AFAIK, vacuuming isn't necessary if page count is also increased, since the larger page size will be used moving forward. Vacuuming is still ideal to transition historic DB pages to the larger page size. Sadly, I don't seem to have the resources to try vacuuming the DB. |
Sqlite vacuuming requires an equal amount of unallocated space in order to cleanup a database file. For example, if an 8TB transaction.db file needs to be vacuumed, at least 8TB (probably more) of free space would be required to process the file. For this reason, if there comes a time when developers need to vacuum any sqlite database files, it is advantageous to have a handful of smaller files compared to one large file. Did we consider a solution that breaks up the transaction.db into smaller chucks? This would alleviate the max_page_count threshold and improve |
65ded6b
to
d66235d
Compare
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.
All CI checks passed on 65ded6b. Merging to master
for 2.2.3 hotfix.
High Level Overview of Change
When rippled initiates a connection to SQLite3, rippled sends a "PRAGMA" statement defining the maximum number of pages allowed in the database. This commit updates the max_page_count so it is consistent with the default for newer versions of SQLite3. Increasing max_page_count is critical for keeping full history servers online.
Context of Change
Fixes #5102
Type of Change
Future Tasks
Older versions of SQLite3 should be tested to ensure they can successfully use the larger max_page_count