Account files remove

[xiangzhu70]

Problem

The validator logs show that clean_accounts_paths takes too long time. This moves files deleting work to an async background task, unblocking validator new() to move forward without waiting for the deletion to be done.

INFO solana_core::validator] done. clean_accounts_paths took 14.5s

Summary of Changes

Rename the files first, which should take much less time than deleting them.
Call an async task to remove the files in async context, let the validator init to go forward without blocking on it.

Before the validator process starts, there is 105GB files in the directory, which would take 14s before the change.
sol@dev-server-da11:~$ du /mnt/nvme1n1/ledger/accounts
98832856 /mnt/nvme1n1/ledger/accounts/accounts
105338348 /mnt/nvme1n1/ledger/accounts

With the change, there is no more 14s waiting.
1984890 [2022-08-17T21:22:08.428526819Z INFO solana_core::validator] Cleaning accounts paths..
1984891 [2022-08-17T21:22:08.428580241Z INFO solana_core::validator] done. clean_accounts_paths took 43us

Fixes #

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[jeffwashington]

I suggest just:

1. calculate path_delete
2. remove directory path_delete
3. rename path to path_delete

This simplifies things. And it keeps this process from growing out of control if we fail at startup quickly somehow.
Step 2 will take 0 time for any normal case.

[brooksprumo]

I like this idea, should definitely help speed up startup. Note, I didn't fully review the code!

With that said, here's the general approach I would use, in pseudo-code:

in a thread or async fn
    create a temp dir [^1]
    move all the account paths into temp dir
    close/drop the temp dir, which will remove it and everything inside

[^1] temp dir: https://docs.rs/tempfile/latest/tempfile/fn.tempdir.html

This way I don't think you'll need to handle calling any remove/delete functions directly.

[jeffwashington]

I like this idea, should definitely help speed up startup. Note, I didn't fully review the code!

With that said, here's the general approach I would use, in pseudo-code:

in a thread or async fn
    create a temp dir [^1]
    move all the account paths into temp dir
    close/drop the temp dir, which will remove it and everything inside

[^1] temp dir: https://docs.rs/tempfile/latest/tempfile/fn.tempdir.html

This way I don't think you'll need to handle calling any remove/delete functions directly.

An issue here is that the temp dir could be on a different drive than the accounts folder.
I think we should stick with the current method atm.

[jeffwashington]

just change up the path and then looks good to me.

Pull request has been modified.

[xiangzhu70]

I like this idea, should definitely help speed up startup. Note, I didn't fully review the code!

With that said, here's the general approach I would use, in pseudo-code:

in a thread or async fn
    create a temp dir [^1]
    move all the account paths into temp dir
    close/drop the temp dir, which will remove it and everything inside

[^1] temp dir: https://docs.rs/tempfile/latest/tempfile/fn.tempdir.html

This way I don't think you'll need to handle calling any remove/delete functions directly.

I think this approach will make the rm_dir call inside the drop call, at some scope closing time. It is effectively the same as making the rm_dir call directly.

[brooksprumo]

Lgtm, with the nits noted. I'll defer to @jeffwashington for additional/final approval.

[brooksprumo]

nit: For Paths, (and PathBuf), they have a .display() method for this purpose. I'd recommend that here, and all user-facing logs.

Suggested change

	info!("{path_delete:?} exists, delete it first.");
	info!("{} exists, delete it first.", path.display());

[brooksprumo]

Same nit w.r.t. path.display().

Probably can also dial this log level down to debug!(). If we're trying to delete a file/directory and it doesn't exist, then the end result is the same :)

[brooksprumo]

I think this approach will make the rm_dir call inside the drop call, at some scope closing time. It is effectively the same as making the rm_dir call directly.

Yah, the remove_dir_all() part is the same. The nice part is that TempDir handles the rename/collision part so we don't have to. But yeah, not needed for this PR.

Pull request has been modified.

[jeffwashington]

I think get rid of this info. It is always true that this will be done in bg now.

[jeffwashington]

I think get rid of this. only useful for your debugging. nobody keeps debug logs turned on.