-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Clean up calculations of pcoinsTip memory usage #10133
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
Conversation
|
utack 99b814f. |
src/validation.cpp
Outdated
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.
The changes in the rollback test are never flushed, so there is no need for compensating for a peak?
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.
agreed. assumed coins was built on top of pcoinstip, missed that. will change.
Since we are more accurately measuring pcoinsTip peak usage at twice the current in dynamic usage, it makes sense to double the default (this will lead to the same effective usage and peak usage as previously). We should also double the buffer used to avoid flushing if above 90% but still sufficient space remaining.
Always leave a reasonable buffer of 50MB for usage from newly connected block (once over 50%) and increase the high water mark buffer to 200MB.
|
Fixed sipa's issue. After discussion I decided to remove the lowering of maxmempool until we are more confident that maxes sense. I also made the flushing buffer a bit smarter. |
|
utACK 1b55e07 We should tag this for 0.14.1 |
|
Benchmarking. |
1b55e07 Make threshold for flushing more conservative. (Alex Morcos) f33afd3 Lower default memory footprint slightly (Alex Morcos) 5b95a19 Make pcoinsTip memory calculations consistent (Alex Morcos) Tree-SHA512: d0061138596cf89008397b8729d9b25293938b1ad454cc99a6fe2f6210e94f76dfa78a8f0fce4c1ba3efec4e742a9c1a3ab26676a4a8346d3e7c3055d032669b
Since we are more accurately measuring pcoinsTip peak usage at twice the current in dynamic usage, it makes sense to double the default (this will lead to the same effective usage and peak usage as previously). We should also double the buffer used to avoid flushing if above 90% but still sufficient space remaining. Github-Pull: #10133 Rebased-From: 5b95a19
1b55e07 Make threshold for flushing more conservative. (Alex Morcos) f33afd3 Lower default memory footprint slightly (Alex Morcos) 5b95a19 Make pcoinsTip memory calculations consistent (Alex Morcos) Tree-SHA512: d0061138596cf89008397b8729d9b25293938b1ad454cc99a6fe2f6210e94f76dfa78a8f0fce4c1ba3efec4e742a9c1a3ab26676a4a8346d3e7c3055d032669b
In #10126 we changed the calculation of when to flush so that the requested dbcache memory limit would be respected even by the peak usage occurring at flush.
Without doubling the default dbcache limit, this would lead to an effective default of half the previous which I believe is too low.
First commit doubles the default and applies the DB_PEAK_USAGE_FACTOR in 2 other places.
The debug logging of the usage still reports the current usage.
The second commit lowers the defaults slightly to ensure 0.14.1 has a lower memory footprint than 0.13.2.
The third commit makes the delayed flushing on large dbcaches a bit less aggressive, so the limit is less likely to be violated in practice.