-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Conversation
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.
LGTM
#[cfg(test)] | ||
const MAX_CLEAN_BATCH_SIZE: u32 = 10; | ||
#[cfg(not(test))] | ||
const MAX_CLEAN_BATCH_SIZE: u32 = 300; |
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.
What is the impact of doing this many items on each earliest session update? (for nodes who have a lot of dangling storage items to clean)
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.
From what I have seen so far, it seems to be pretty fast - although that might have been mostly empty sessions. Worst thing that could happen is that a valdiator is heavily loaded at a session boundary and fails to do some work. I also don't have any good data yet about how much wasted storage we are actually talking about, if it is tiny we can go with smaller batch sizes as then it does not matter if it takes forever.
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.
burnin on Kusama will tell.
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.
How did burn-in on Kusama go? What are the expected savings?
Co-authored-by: Andronik <write@reusable.software>
Co-authored-by: Andronik <write@reusable.software>
burnin on Kusama was not done yet. On Rococo I don't have definite data, but it looks like around 10%. I will add a metric how long cleanup rounds take and will then burnin on Kusama. |
…o rk-fix-dispute-storage-leak
burnin looks fine so far. Given that cleanup takes a while, let's include this sooner than later. |
* Fix cleanup of old votes. * Cleanup. * Get rid of redundant import * Tests + logging * Fix db key name. * Add some reasoning to batch size. * Add dispute data to indexed columns * Fix fmt * Add helper function. * Fix typos. * Update node/core/dispute-coordinator/src/db/v1.rs Co-authored-by: Andronik <write@reusable.software> * Update node/core/dispute-coordinator/src/db/v1.rs Co-authored-by: Andronik <write@reusable.software> * Add metric for how long cleanup takes. Co-authored-by: Andronik <write@reusable.software>
this has not been resolved, since the release of v0.9.25 (and after a full cleanup of parachains/db folder) it has gradually increased with .sst files left behind since Jul 6 every day. At the moment on a validator that is active it has gotten to 376 .sst files total and 23GB in size of the parachains/db folder. du -hs .local/share/polkadot/chains/ksmcc3/db/full/parachains/db/ ls -ltr .local/share/polkadot/chains/ksmcc3/db/full/parachains/db/*.sst | wc -l |
Thanks @Generic-Chain ! Indeed there exists another leak, @vstakhov already identified that one as well and fixed it. |
It is not very likely that the leak I have fixed could lead to 23Gb of storage waste. One thing we could try is to examine the current database to figure out what's happening. |
I'm just reporting what I am seeing, I'm not familiar with what exactly was fixed and if there's some other storage leak unrelated to this one - the files are there since the cleanup and upgrade and the size and count keeps increasing 8 days after my post is up to 40GB and 576 .sst files (so +16GB and +200 .sst files) du -hs .local/share/polkadot/chains/ksmcc3/db/full/parachains/db/ ls -ltr .local/share/polkadot/chains/ksmcc3/db/full/parachains/db/*.sst | wc -l anyone with an active validator can confirm this - I have 2 and both are having the same issue, 2nd one has 40GB and 545 files in the parachains/db folder |
This PR makes sure old votes are pruned from the database in dispute-coordinator.
Should be tested on Versi and with paritydb.
Release Notes
Operators of nodes running paritydb will need to delete the parachains database before upgrading.