feat(state): delete old database directories #4586

oxarbitrage · 2022-06-08T20:43:18Z

Motivation

When the database format change, a new full blockchain will be downloaded when zebra starts. The old blockchain will remain in the file system occupying a lot of hard drive space.

We want to delete this old versions of the blockchain by default when zebra starts, but we also want a config option to not do it if the user want to keep the different versions for whatever reason.

This PR will close #1213 if it gets merged.

Solution

When zebra start and if the delete_old_database config option is set to true, the code added here will check the database directories and delete the ones that are considered all. Directory will be deleted will all the content inside them.

Review

I think anyone can review, open for improvements, probably some code in the check_and_delete_old_databases can be simplified a bit.

No tests were made, i do tested manually. check_and_delete_old_databases function could be tested with different scenarios but i am unsure if it will worth to do it.

zebrad/src/commands/start.rs

codecov · 2022-06-09T21:22:33Z

Codecov Report

Merging #4586 (c95f468) into main (e5c9208) will decrease coverage by 0.05%.
The diff coverage is 3.89%.

@@            Coverage Diff             @@
##             main    #4586      +/-   ##
==========================================
- Coverage   79.07%   79.01%   -0.06%     
==========================================
  Files         304      304              
  Lines       37418    37499      +81     
==========================================
+ Hits        29589    29631      +42     
- Misses       7829     7868      +39

teor2345 · 2022-06-14T04:33:30Z

This looks good, and it looks like it should work.

If we wanted to test it, we'd need to pass the current state version as a function argument. And I'm not sure it's worth it. Our ephemeral database code is very similar, and it works fine.

I have some suggestions about edge cases, simplifications, and module structure.

Did you want to catch up to talk through them, or I could make a PR, or I could write some comments?
(I'm happy to just do edge cases to start with, if that would help.)

oxarbitrage · 2022-06-14T10:44:38Z

Comments or PR are welcome (whatever is easier for you). Thanks.

teor2345

Filesystem error and path handling is a bit tricky in Rust, I made some suggestions to avoid panics.

I have some other suggestions about performance and module structure, but they are optional.

zebrad/src/commands/start.rs

teor2345

The premature returns and panics are all fixed, so we can merge this if you want.

(The other changes can be made in other PRs, but they are all optional.)

zebra-state/src/config.rs

…ectory (#4631) * Add function comments, tweak log * Simplify version parsing * Use spawn_blocking to launch the task on a separate thread, do the cleanup last * Abort the cleanup task when Zebra exits * Split directory deletion into its own function, handle ownership * Rename cache_dir to state_dir * If an outdated state directory is outside the cache directory, don't delete it * Minimise diffs

teor2345 · 2022-06-20T02:16:40Z

I'm going to merge main into this PR, to check if the CI failures are actually in this PR.
(I thought we had fixed the macOS failure with PR #4613.)

teor2345 · 2022-06-20T02:16:50Z

@Mergifyio update

mergify · 2022-06-20T02:17:14Z

update

✅ Branch has been successfully updated

teor2345

This looks good, we just need to fix a non-canonical path on macOS.

zebrad/tests/acceptance.rs

Co-authored-by: teor <teor@riseup.net>

teor2345

Thanks, this looks great, and thanks for the tests!

* ZIPs were updated to remove ambiguity, this was tracked in #1267. * #2105 was fixed by #3039 and #2379 was closed by #3069 * #2230 was a duplicate of #2231 which was closed by #2511 * #3235 was obsoleted by #2156 which was fixed by #3505 * #1850 was fixed by #2944, #1851 was fixed by #2961 and #2902 was fixed by #2969 * We migrated to Rust 2021 edition in Jan 2022 with #3332 * #1631 was closed as not needed * #338 was fixed by #3040 and #1162 was fixed by #3067 * #2079 was fixed by #2445 * #4794 was fixed by #6122 * #1678 stopped being an issue * #3151 was fixed by #3934 * #3204 was closed as not needed * #1213 was fixed by #4586 * #1774 was closed as not needed * #4633 was closed as not needed * Clarify behaviour of difficulty spacing Co-authored-by: teor <teor@riseup.net> * Update comment to reflect implemented behaviour Co-authored-by: teor <teor@riseup.net> * Update comment to reflect implemented behaviour when retrying block downloads Co-authored-by: teor <teor@riseup.net> * Update `TODO` to remove closed issue and clarify when we might want to fix Co-authored-by: teor <teor@riseup.net> * Update `TODO` to remove closed issue and clarify what we might want to change in future Co-authored-by: teor <teor@riseup.net> * Clarify benefits of how we do block verification Co-authored-by: teor <teor@riseup.net> * Fix rustfmt errors --------- Co-authored-by: teor <teor@riseup.net>

delete old database directories

6fb2cb9

oxarbitrage requested a review from a team as a code owner June 8, 2022 20:43

oxarbitrage requested review from dconnolly and removed request for a team June 8, 2022 20:43

oxarbitrage commented Jun 8, 2022

View reviewed changes

zebrad/src/commands/start.rs Outdated Show resolved Hide resolved

oxarbitrage commented Jun 8, 2022

View reviewed changes

zebrad/src/commands/start.rs Outdated Show resolved Hide resolved

check if state directory exists

3453b54

teor2345 assigned oxarbitrage Jun 14, 2022

teor2345 reviewed Jun 14, 2022

View reviewed changes

ftm1000 added the P-Medium ⚡ label Jun 14, 2022

oxarbitrage added 3 commits June 15, 2022 11:11

skip deleting when ephemeral

fd02f55

split check_and_delete_old_databases

3cd8ab6

move check_and_delete_old_databases to state

189970a

teor2345 previously approved these changes Jun 15, 2022

View reviewed changes

spawn check_and_delete_old_databases

7a57981

oxarbitrage dismissed teor2345’s stale review via 7a57981 June 15, 2022 21:57

teor2345 reviewed Jun 15, 2022

View reviewed changes

zebra-state/src/config.rs Outdated Show resolved Hide resolved

simplity a bit

5c3595a

teor2345 reviewed Jun 15, 2022

View reviewed changes

zebra-state/src/config.rs Outdated Show resolved Hide resolved

teor2345 changed the title ~~feat(cleanup): delete old database directories~~ feat(state): delete old database directories Jun 15, 2022

teor2345 mentioned this pull request Jun 16, 2022

fix(state): only delete old database directories inside the cache directory #4631

Merged

3 tasks

teor2345 and others added 3 commits June 16, 2022 09:11

Merge remote-tracking branch 'origin/main' into issue1213

e2f96b5

add test

5a707e1

teor2345 mentioned this pull request Jun 20, 2022

fix(state): Update column family names to match Zebra's database design #4639

Merged

2 tasks

Merge branch 'main' into issue1213

154f0b0

teor2345 reviewed Jun 20, 2022

View reviewed changes

zebrad/tests/acceptance.rs Outdated Show resolved Hide resolved

zebrad/tests/acceptance.rs Show resolved Hide resolved

zebrad/tests/acceptance.rs Show resolved Hide resolved

oxarbitrage and others added 3 commits June 20, 2022 10:25

fix typos

8a537b5

Co-authored-by: teor <teor@riseup.net>

add canonicalize to test regex

6cd8f57

add another match to test

c95f468

teor2345 approved these changes Jun 20, 2022

View reviewed changes

mergify bot added a commit that referenced this pull request Jun 20, 2022

Merge of #4586

1291854

mergify bot mentioned this pull request Jun 20, 2022

merge-queue: embarking main (b7536c7) and #4586 together #4653

Closed

92 tasks

mergify bot added a commit that referenced this pull request Jun 20, 2022

Merge of #4586

0ece36b

mergify bot mentioned this pull request Jun 20, 2022

merge-queue: embarking main (961fcb6) and #4586 together #4655

Closed

92 tasks

mergify bot merged commit 769d069 into main Jun 21, 2022

mergify bot deleted the issue1213 branch June 21, 2022 00:59

mpguerra added a commit that referenced this pull request May 19, 2023

#1213 was fixed by #4586

d3456b8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(state): delete old database directories #4586

feat(state): delete old database directories #4586

oxarbitrage commented Jun 8, 2022

codecov bot commented Jun 9, 2022 •

edited

Loading

teor2345 commented Jun 14, 2022

oxarbitrage commented Jun 14, 2022

teor2345 left a comment

teor2345 left a comment

teor2345 commented Jun 20, 2022

teor2345 commented Jun 20, 2022

mergify bot commented Jun 20, 2022

teor2345 left a comment

teor2345 left a comment

feat(state): delete old database directories #4586

feat(state): delete old database directories #4586

Conversation

oxarbitrage commented Jun 8, 2022

Motivation

Solution

Review

codecov bot commented Jun 9, 2022 • edited Loading

Codecov Report

teor2345 commented Jun 14, 2022

oxarbitrage commented Jun 14, 2022

teor2345 left a comment

Choose a reason for hiding this comment

teor2345 left a comment

Choose a reason for hiding this comment

teor2345 commented Jun 20, 2022

teor2345 commented Jun 20, 2022

mergify bot commented Jun 20, 2022

✅ Branch has been successfully updated

teor2345 left a comment

Choose a reason for hiding this comment

teor2345 left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 9, 2022 •

edited

Loading