Skip to content

Conversation

@morcos
Copy link
Contributor

@morcos morcos commented Mar 31, 2017

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.

@JeremyRubin
Copy link
Contributor

utack 99b814f.

Copy link
Member

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?

Copy link
Contributor Author

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.

morcos added 3 commits March 31, 2017 13:15
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.
@morcos
Copy link
Contributor Author

morcos commented Mar 31, 2017

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.

@sdaftuar
Copy link
Member

utACK 1b55e07

We should tag this for 0.14.1

@sipa
Copy link
Member

sipa commented Mar 31, 2017

Benchmarking.

@fanquake fanquake added this to the 0.14.1 milestone Mar 31, 2017
@laanwj laanwj merged commit 1b55e07 into bitcoin:master Apr 5, 2017
laanwj added a commit that referenced this pull request Apr 5, 2017
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
laanwj pushed a commit that referenced this pull request Apr 5, 2017
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
laanwj pushed a commit that referenced this pull request Apr 5, 2017
laanwj pushed a commit that referenced this pull request Apr 5, 2017
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.

Github-Pull: #10133
Rebased-From: 1b55e07
@laanwj
Copy link
Member

laanwj commented Apr 5, 2017

utACK 1b55e07

BTW @fanquake if you tag things for backport please also add the "needs backport" tag, this allows for filtering what has not been backported yet.

codablock pushed a commit to codablock/dash that referenced this pull request Oct 24, 2017
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants