validators always skip clean/shrink on startup

[jeffwashington]

Problem

See
discord discussion

When starting a validator, we currently skip startup clean and shrink if the snapshot was generated locally. As of 1.13 or 1.14?, we now start accounts background service much earlier in the startup process. Accounts background service does a clean and shrink concurrently with tx processing.

Note that clean is probably slower with a disk index than without. This synchronous clean thus has a bigger penalty on startup when disk index is enabled.

Summary of Changes

Always skip the synchronous initial clean/shrink on validator startup. Make the cli arg obsolete.

Fixes #

[brooksprumo]

Looks like cargo-fmt borked here. Can be fixed here in this PR or in a subsequent one. Minimally, can it be fixed in the backport?

[jeffwashington]

yep, fixed it. I was relying on fmt to fix this and it did not...

[bw-solana]

LGTM!

[t-nelson]

thanks!

[codecov]

Codecov Report

Merging #30710 (9a914f0) into master (89d5efa) will decrease coverage by 0.1%.
The diff coverage is 0.0%.

@@            Coverage Diff            @@
##           master   #30710     +/-   ##
=========================================
- Coverage    81.7%    81.6%   -0.1%     
=========================================
  Files         723      723             
  Lines      201656   201655      -1     
=========================================
- Hits       164761   164749     -12     
- Misses      36895    36906     +11     

[steviez]

Should we do the same (change default behavior / maybe deprecate arg) in ledger-tool as well ?

[jeffwashington]

Should we do the same (change default behavior / maybe deprecate arg) in ledger-tool as well ?

we could. We'd need to be sure it is what we want in all cases. I imagine create-snapshot, for example, does a clean/shrink before it creates a snapshot? Otherwise, we'd want this first one probably?

[steviez]

Should we do the same (change default behavior / maybe deprecate arg) in ledger-tool as well ?

we could. We'd need to be sure it is what we want in all cases. I imagine create-snapshot, for example, does a clean/shrink before it creates a snapshot? Otherwise, we'd want this first one probably?

Hmm, good point on different behavior for different command. If create-snapshot does a clean before snapshot creation, that seems sufficient and can skip. For all other commands (such as verify), do we even need clean/shrink at all since that AccountsDb will only live as long as the command? Would skipping the clean/shrink have perf impact on replay speed of a command like verify ?

[jeffwashington]

not cleaning does slow down operations somewhat since there are more index entries to filter out. I expect this is negligible. Clean will catch up in the back ground.

We don't need it for verify. It does serve to verify the clean/shrink algorithm is correct. It is just another can of worms to get rid of it in ledger-tool. I wanted to keep the 1.14 backport pr simple and just get rid of it for validator. Ripping the code and all args out from ledger-tool and verifying bank at startup is something we could do in master if we're comfortable with the ledger-tool side effects.