Introduce AccountsFileProvider

[yhchiang-sol]

Problem

Currently, all existing code paths for creating a new writable AccountsFile
are hard-coded to create an AppendVec. As we introduce a new AccountsFile
format, we will need a more programmatic way to create writable AccountsFiles
using a different format.

Summary of Changes

This PR introduces AccountsFileProvider trait, which allows both AppendVec
and TieredStorage to provide their implementations to create a new AccountsFile.
This will later allow tests and the production code-path to switch between different
formats programmatically.

For existing places where AppendVec is created, this PR simply replaces them by
AppendVecProvider. Will create follow-up PRs that change them to use generic.

[codecov-commenter]

Codecov Report

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

Project coverage is 81.9%. Comparing base (7c8a287) to head (6ebec49).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #334   +/-   ##
=======================================
  Coverage    81.8%    81.9%           
=======================================
  Files         841      841           
  Lines      228267   228272    +5     
=======================================
+ Hits       186924   186984   +60     
+ Misses      41343    41288   -55     

[brooksprumo]

Hopefully the generic AccountsFileProvider doesn't get too unruly. I can't think of a better way to handle this though.

[yhchiang-sol]

Rebased on top of #434

[yhchiang-sol]

Rebased on top of master that includes #434

[brooksprumo]

Code looks good to me. Adding @jeffwashington to review as well. Jeff, wdyt of this solution?

[jeffwashington]

I'm struggling with the language feature of traits applied to this. I'm not immediately thinking, "Yes, this is the right way to do it." I would mr. language, @brooksprumo to have ideas.

[yhchiang-sol]

Code looks good to me. Adding @jeffwashington to review as well. Jeff, wdyt of this solution?

Here're some exam examples of how this PR is used:

• Apply to unit-tests (PR344).
• Apply to some accounts-db functions (PR439)

[jeffwashington]

I'm struggling with the language feature of traits applied to this. I'm not immediately thinking, "Yes, this is the right way to do it." I would mr. language, @brooksprumo to have ideas.

I guess alternatives are:

1. an enum passed to a creation fn
2. 2 different creation fns
3. a creation fn that uses state from AccountsDb to know which type of new storage to create?
   Are there other alternatives?

[yhchiang-sol]

I start feeling enum might be the right way to go.

My initial thought was that constructor functions usually don't take &self, but the enum-based approach requires an instance to determine its type. So that's why I started with a trait approach. Here's what I am currently thinking. Please feel free to correct me if I am wrong here:

Let's try to imagine how this would apply to the entire code base. In production, AccountsDb instance is created via AccountsDb::new_with_config. And I can imagine we will have an enum in AccountsDbConfig for the accounts-file-format, possibly derived from the cli argument.

#[derive(Debug, Default, Clone)]
pub struct AccountsDbConfig {
    ... // existing fields
    pub accounts_file_type: SomeAccountsFileTypeWeWillDiscussHere,
}

enum-based approach

For enum-based approach, the above accounts_file_type can be replaced by the provider enum. So it will be:

#[derive(Debug, Default, Clone)]
pub struct AccountsDbConfig {
    ... // existing fields
    pub accounts_file_provider: AccountsFileProviderEnum,
}

And we will likely copy this field to AccountsDb.

Then, for all functions that need to create a new accounts-file, we will need to pass-around the accounts_file_provider from the AccountsDb.

trait-based approach

For trait-based approach, I feel we will need to match the enum at some point as we can't dynamically determine the type:

   match(accounts_file_type) {
       AppendVecType => some_function::<AppendVecProvider>(...),
       HotStorageType => some_function::<HotStorageProvider>(...),
   }

A slight up-side for this is that, after the top-level call. Subsequent call will directly obtain the correct provider type.
Like PR439

The other way is to have dyn field inside AccountsDbConfig or AccountsDb, but I feel this might not be the right way to do.

Another way to achieve the trait-based approach is to have AccountsDb take a generic like this:

struct AccountsDb<AP: AccountsFileProvider> {
}

But again I feel this might not be the right way.

2 different creation fns

Assuming two creation fns share the same API, then I think AccountsDbConfig will have one field that is the instance of the creation function:

pub struct AccountsDbConfig {
    ... // existing fields
    pub accounts_file_fn: AccountsFileCreationFn,
}

I feel this approach works similarly to the enum-based approach, but the enum-based approach might better organize things than this.

a creation fn that uses state from AccountsDb to know which type of new storage to create?

I feel this is a mixed approach of enum + trait. As it requires passing an argument, it also performs a match. But I feel enum-based and trait-based organize better.

wdyt @jeffwashington, @brooksprumo

[yhchiang-sol]

To cast my own vote, here're the two solutions that I prefer:

Pass-around ProviderEnum from AccountsDbConfig

#[derive(Debug, Default, Clone)]
pub struct AccountsDbConfig {
    ... // existing fields
    pub accounts_file_provider: AccountsFileProviderEnum,
}

So all functions that create a writable-accounts-file will take an enum instance of accounts-file-provider.

Use match on top-level calls w/ generic for creating accounts-file

Only on top-level calls. Subsequent calls should already have the right provider type

   match(accounts_file_type) {
       AppendVecType => some_function::<AppendVecProvider>(...),
       HotStorageType => some_function::<HotStorageProvider>(...),
   }

[yhchiang-sol]

Created an enum-based PR #457:

[jeffwashington]

i think we will go with enum