Cleanup blocks from db (incl. forked blocks) #2815
Conversation
…db directly. Signed-off-by: Mike Tang <[email protected]>
chain/src/chain.rs
Outdated
| @@ -1065,6 +1065,49 @@ impl Chain { | |||
| count, tail.height | |||
| ); | |||
|
|
|||
| // Remove short live fork blocks | |||
| // Get all block headers in db | |||
| let all_block_headers = self.store.all_block_headers()?; | |||
There was a problem hiding this comment.
You'll want to iterate over all blocks, not block headers. We never compact/delete the block headers, so using your approach would end up iterating over 100s of thousands of headers. Blocks are compacted frequently, which means we never have more than a few weeks worth of them. Just iterate over each block in the store, check is_on_current_chain and check block.header.height >= tail.height. If either of those is false, call delete_block.
There was a problem hiding this comment.
Hi, updated with new algorithm u advised :)
… cleaup. Signed-off-by: Mike Tang <[email protected]>
chain/src/store.rs
Outdated
| @@ -378,6 +378,13 @@ impl<'a> Batch<'a> { | |||
| db: self.db.child()?, | |||
| }) | |||
| } | |||
|
|
|||
| /// An iterator to all lived block now | |||
| pub fn iter_lived_blocks(&self) -> Result<SerIterator<Block>, Error> { | |||
There was a problem hiding this comment.
Maybe rename this to blocks_iter() for consistency with difficulty_iter() (and others)?
chain/src/chain.rs
Outdated
| Err(NotFoundErr(_)) => break, | ||
| Err(e) => return Err(From::from(e)), | ||
| // Remove old blocks and short live fork blocks | ||
| // here b is a lived block |
There was a problem hiding this comment.
I'm not sure what you mean by "lived" block - maybe reword these comments?
chain/src/chain.rs
Outdated
| Err(e) => return Err(From::from(e)), | ||
| // Remove old blocks and short live fork blocks | ||
| // here b is a lived block | ||
| let _ = batch.iter_lived_blocks()?.map(|(_, b)| { |
There was a problem hiding this comment.
This can be simplified to -
for (_, b) in batch.iter_lived_blocks()? {
...
}
|
Looks like lmdb is locking up internally with this change - our "batch" is very susceptible to deadlocking with multiple accesses. |
Wow, this is my first attempt to contribute to grin, so lack experiences on it. :( |
|
Or should I use store_instance.iter() rather than batch.iter() in exported iterator in chain/src/store.rs? |
2. comments and iterator calling optimiztions. Signed-off-by: Mike Tang <[email protected]>
|
Ha - It's the call to |
|
In my opinion, maybe we don't need batch lock in the procedure of iterating over blocks in db, this is not necessary for the whole block cleanup, only necessary when delete one block. So, we can add an iterator to ChainStore implementation, rather than Batch. |
|
I think we actually got the logic wrong here. Now that we are iterating over all blocks in the db, all we really care about is cleaning up blocks older than the threshold. So we do not need to actually check for And this should resolve the deadlocking issue as well I think. |
|
The height of a fork block MUST be smaller than tail.height? Surely? |
Yes - we want to delete block older (i.e. lower height) than tail.height |
…er just by removing it. Because "we want to delete block older (i.e. lower height) than tail.height". Signed-off-by: Mike Tang <[email protected]>
antiochp
added
the
release notes
antiochp
changed the title
fix: try to fix issue #2585 by adding block cleanup from db directly.