anza-xyz / agave

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://www.anza.xyz/
Apache License 2.0
178 stars 67 forks source link

Do not purge old snapshot archives inside of archive_snapshot_package() #1417

Closed brooksprumo closed 1 week ago

brooksprumo commented 2 weeks ago

Problem

Old snapshot archives are purged inside of archive_snapshot_package(). I find this surprising. This also causes the function (and all its callers) to pass in four extra parameters. I'd argue that purging should be done explicitly. Additionally, I'm working on removing the reserialization of bank snapshots, and part of that will be removing unnecessary fields from the AccountsPackage/SnapshotPackage. That will include the snapshot archive dirs, which are duplicates of the SnapshotConfig that AccountsHashVerifier and SnapshotPackagerService already have a copy of.

Summary of Changes

Do not purge old snapshot archives inside of archive_snapshot_package(). Instead, purge explicitly, after archiving.

Note: Ignoring whitespace in the diff may make it easier.

codecov-commenter commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 7.69231% with 36 lines in your changes are missing coverage. Please review.

Project coverage is 82.7%. Comparing base (83be26a) to head (19b7744). Report is 4 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1417 +/- ## ========================================= - Coverage 82.7% 82.7% -0.1% ========================================= Files 871 871 Lines 370276 370250 -26 ========================================= - Hits 306463 306390 -73 - Misses 63813 63860 +47 ```