[Merged by Bors] - Refine compaction #1916
Closed
Conversation
This file contains hidden or 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
michaelsproul
added
A0
consensus
paulhauner
requested changes
Nov 16, 2020
Member
paulhauner
left a comment
paulhauner
left a comment
There was a problem hiding this comment.
Looks good, only two comments/suggestions.
paulhauner
added
waiting-on-author
michaelsproul
removed
the
consensus
michaelsproul
added
ready-for-review
Member
Author
|
I'm going to test sync on an HDD to see if this resolves issues, but I'm also happy to merge this in the meantime, as I think all these improvements are generally helpful, even if they don't 100% solve the OOM issue. |
paulhauner
added
ready-for-merge
paulhauner
approved these changes
Nov 17, 2020
Member
Author
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Nov 17, 2020
## Proposed Changes In an attempt to fix OOM issues and database consistency issues observed by some users after the introduction of compaction in v0.3.4, this PR makes the following changes: * Run compaction less often: roughly every 1024 epochs, including after long periods of non-finality. I think the division check proposed by Paul is pretty solid, and ensures we don't miss any events where we should be compacting. LevelDB lacks an easy way to check the size of the DB, which would be another good trigger. * Make it possible to disable the compaction on finalization using `--auto-compact-db=false` * Make it possible to trigger a manual, single-threaded foreground compaction on start-up using `--compact-db` * Downgrade the pruning log to `DEBUG`, as it's particularly noisy during sync I would like to ship these changes to affected users ASAP, and will document them further in the Advanced Database section of the book if they prove effective.
Member
Author
|
on second thoughts... bors r- I want to do some more testing. I suspect based on Dankrad's reports that the compaction is still too frequent during sync (23 compactions during Medalla sync is still a lot): #1917 (comment) |
|
Canceled. |
paulhauner
approved these changes
Nov 17, 2020
Member
paulhauner
left a comment
paulhauner
left a comment
There was a problem hiding this comment.
Looks good! We spent a fair bit of time chatting about this out-of-band and I think the present compaction timing is good for now.
Merge it!
Member
Author
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Nov 17, 2020
## Proposed Changes In an attempt to fix OOM issues and database consistency issues observed by some users after the introduction of compaction in v0.3.4, this PR makes the following changes: * Run compaction less often: roughly every 1024 epochs, including after long periods of non-finality. I think the division check proposed by Paul is pretty solid, and ensures we don't miss any events where we should be compacting. LevelDB lacks an easy way to check the size of the DB, which would be another good trigger. * Make it possible to disable the compaction on finalization using `--auto-compact-db=false` * Make it possible to trigger a manual, single-threaded foreground compaction on start-up using `--compact-db` * Downgrade the pruning log to `DEBUG`, as it's particularly noisy during sync I would like to ship these changes to affected users ASAP, and will document them further in the Advanced Database section of the book if they prove effective.
|
Pull request successfully merged into master. Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Proposed Changes
In an attempt to fix OOM issues and database consistency issues observed by some users after the introduction of compaction in v0.3.4, this PR makes the following changes:
--auto-compact-db=false--compact-dbDEBUG, as it's particularly noisy during syncI would like to ship these changes to affected users ASAP, and will document them further in the Advanced Database section of the book if they prove effective.