Skip to content
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

Removes old accounts hash cache dir #32604

Merged

Conversation

brooksprumo
Copy link
Contributor

Problem

The directory that caches the result from accounts hash calculation was renamed in #29734. This required manual intervention on the part of node operators to remove the old cache directory. While it was harmless, the old directory did take up disk space. It should be cleaned up automatically.

Summary of Changes

Remove the old cache dir, if it exists.

@brooksprumo brooksprumo self-assigned this Jul 24, 2023
@brooksprumo brooksprumo added the v1.16 PRs that should be backported to v1.16 label Jul 24, 2023
@brooksprumo brooksprumo marked this pull request as ready for review July 24, 2023 14:38
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #32604 (9fab977) into master (62b9fcf) will decrease coverage by 0.1%.
The diff coverage is 90.0%.

@@            Coverage Diff            @@
##           master   #32604     +/-   ##
=========================================
- Coverage    82.0%    82.0%   -0.1%     
=========================================
  Files         781      781             
  Lines      210801   210813     +12     
=========================================
- Hits       172959   172947     -12     
- Misses      37842    37866     +24     

@brooksprumo brooksprumo requested a review from t-nelson July 24, 2023 18:05
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

// The accounts hash cache dir was renamed, so cleanup the old dir if it exists.
let old_accounts_hash_cache_dir = ledger_path.join("calculate_accounts_hash_cache");
if old_accounts_hash_cache_dir.exists() {
snapshot_utils::move_and_async_delete_path(old_accounts_hash_cache_dir);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated: logic in here looks a little suspect 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@brooksprumo brooksprumo merged commit 36b3722 into solana-labs:master Jul 24, 2023
@brooksprumo brooksprumo deleted the cleanup-old-accounts-hash-cache branch July 24, 2023 21:35
mergify bot pushed a commit that referenced this pull request Jul 24, 2023
brooksprumo added a commit that referenced this pull request Jul 25, 2023
brooksprumo added a commit that referenced this pull request Jul 25, 2023
Removes old accounts hash cache dir (#32604)

(cherry picked from commit 36b3722)

Co-authored-by: Brooks <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.16 PRs that should be backported to v1.16
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants