ledger: move accountdb to its own package #4428

ghost · 2022-08-18T20:42:30Z

Summary

This PR moves accountdb, tests, LRU caches and all related stuff into its own package.
Some data types were exported as interfaces, others were hidden.
There are three types of DB access functions:

Prepared queries for lookup
Commit time functions
- old account state loaders (AccountLoadOld and similar)
- Rounds update
- Accounts/Resources/Online commit functions
Catchpoint state management, DB reader MT committer

Future work possible as separate PR:

make compact deltas more self-contained and get rid of exporting AccountLoadOld and Co
make accounts commit function (old/existing state vs new state comparison) part of business logic, not storage logic

Test Plan

Rely on existing tests

… not compile yet

algorandskiy

First pass

remove ledger/TestFirstStagePersistence.7233885710903406427 binary files
maybe call accountdb package trackerdb ?

algorandskiy · 2022-08-29T22:40:28Z

cmd/catchpointdump/file.go

 				acctCount++
 			}
-			_, err = ledger.LoadAllFullAccounts(context.Background(), tx, balancesTable, resourcesTable, acctCb)
+			_, err = accountdb.LoadAllFullAccounts(context.Background(), tx, balancesTable, resourcesTable, acctCb)


suggestion: rewrite the function as iterator class with Next() and Close()

algorandskiy · 2022-08-29T22:44:12Z

ledger/accountdb/accounts_test.go

+	"github.com/algorand/go-algorand/protocol"
+)
+
+func accountsAll(tx *sql.Tx) (bals map[basics.Address]basics.AccountData, err error) {


this looks like can be replaced by the iterator from the suggestion from above.

algorandskiy · 2022-08-29T22:47:41Z

ledger/accountdb/catchpoints.go

+	"github.com/algorand/msgp/msgp"
+)
+
+type EncodedBalanceRecordV5 struct {


maybe this file should go into catchpoint package.
Rationale: it does not to touch db, only local files

algorandskiy · 2022-08-29T22:50:35Z

ledger/accountdb/lruaccts.go

 	// and the ones on the back are the oldest.
 	accountsList *persistedAccountDataList
-	// accounts provides fast access to the various elements in the list by using the account address
+	// accounts provides fast access to the various elements in the list by using the account Address


Address -> address

Thanks. Something annoying that happened was that when using goland refactor, it modified comments that are unrelated to changed variables. Please let me know if you spot any more weird capitalization in the comments and I will fix them.

algorandskiy · 2022-08-29T22:57:30Z

ledger/accountdb/lruaccts.go

 	m.accountsList = newPersistedAccountList().allocateFreeNodes(pendingWrites)
 	m.accounts = make(map[basics.Address]*persistedAccountDataListNode, pendingWrites)
-	m.pendingAccounts = make(chan persistedAccountData, pendingWrites)
+	m.pendingAccounts = make(chan PersistedAccountData, pendingWrites)


a big win would be if we can keep PersistedAccountData internal.

Usage in tracker/dcc : updatedPersistedAccounts []persistedAccountData could be an interface with a single method Len() and lruAccounts might have method writeAccounts taking this interface and casting to []persistedAccountData before iterating.

persistedData, err = au.accountsq.lookup(addr) could also be an interface with
Round()
Valid()
GetLedgerCoreAccountData()

Put these interfaces in acctupdates etc, name them UpdatedAccounts, etc

algorandskiy · 2022-08-30T16:59:10Z

ledger/acctupdates.go

+	// the updates of the actual account data is done last since the AccountsNewRound would modify the compactDeltas old values
 	// so that we can update the base account back.
-	dcc.updatedPersistedAccounts, dcc.updatedPersistedResources, err = accountsNewRound(tx, dcc.compactAccountDeltas, dcc.compactResourcesDeltas, dcc.compactCreatableDeltas, dcc.genesisProto, dbRound+basics.Round(offset))
+	dcc.updatedPersistedAccounts, dcc.updatedPersistedResources, err = accountdb.AccountsNewRound(tx, dcc.compactAccountDeltas, dcc.CompactResourcesDeltas, dcc.compactCreatableDeltas, dcc.genesisProto, dbRound+basics.Round(offset))


type UpdatedAccounts struct { Data interface{} Count uint64 }

codecov · 2022-09-02T18:01:50Z

Codecov Report

Merging #4428 (3292fb7) into master (00b37f2) will decrease coverage by 1.56%.
The diff coverage is 59.84%.

@@            Coverage Diff             @@
##           master    #4428      +/-   ##
==========================================
- Coverage   55.28%   53.71%   -1.57%     
==========================================
  Files         398      398              
  Lines       50336    50288      -48     
==========================================
- Hits        27829    27013     -816     
- Misses      20178    21072     +894     
+ Partials     2329     2203     -126

Impacted Files	Coverage Δ
ledger/accountdb/accountdb.go	`49.66% <ø> (ø)`
ledger/accountdb/persistedaccts_list.go	`92.00% <ø> (ø)`
ledger/accountdb/persistedonlineaccts_list.go	`92.00% <ø> (ø)`
ledger/accountdb/persistedresources_list.go	`92.00% <ø> (ø)`
ledger/accountdb/trackerdb.go	`0.00% <0.00%> (ø)`
ledger/ledgercore/accountdata.go	`0.00% <ø> (ø)`
ledger/ledgercore/catchpointlabel.go	`100.00% <ø> (ø)`
ledger/ledgercore/statedelta.go	`6.92% <0.00%> (-0.41%)`	⬇️
ledger/voters.go	`68.65% <0.00%> (-4.48%)`	⬇️
ledger/accountdb/catchpoints.go	`16.90% <16.90%> (ø)`
... and 29 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

…gorand/go-algorand into nguo/accountdb-pkg

cce · 2022-09-14T20:35:10Z

ledger/accountdb/accountdb.go

 	// want to ensure that the "real" written value isn't being overridden by the value from the pending accounts.
 	round basics.Round
 }

+func (pad persistedAccountData) Addr() basics.Address {


@algorandskiy possible opportunity for fewer copies / performance — should we make these pointer receivers and then switch to returning references to persistedAccountData as PersistedAccountData implementations?

yes. But when I tried it there is a problem with writing pointers into a cache. Having shared copies break some tests and possibly some non-test functionality

icorderi

LGTM

A few suggestions for followup PRs:

move TrackerDB to its own package, similar to what was done with BlockDB
create a catchpoint package
create a cache package

algorandskiy · 2022-11-09T00:46:53Z

This looks extremely hard to merge after boxes, closing.

cce and others added 2 commits August 17, 2022 12:51

WIP of accountdb that builds in separate package, but ledger tests do…

8942f0f

… not compile yet

make code compile

68c8902

ghost added Enhancement Team Carbon-11 labels Aug 18, 2022

ghost self-assigned this Aug 18, 2022

ghost marked this pull request as draft August 18, 2022 20:42

nicholasguo added 2 commits August 19, 2022 11:55

fix tests low hanging fruit

692f82f

make tests run

1a6ed60

algorandskiy mentioned this pull request Aug 23, 2022

do not merge: Accountdb pkg #4424

Closed

nicholasguo added 5 commits August 24, 2022 12:35

make fields private

0c169a3

make get private

6d478bb

fix comments

9db5053

fix comments

049ea5e

Merge branch 'master' into nguo/accountdb-pkg

f5a909e

algorandskiy reviewed Aug 29, 2022

View reviewed changes

algorandskiy reviewed Aug 30, 2022

View reviewed changes

nicholasguo and others added 2 commits August 30, 2022 17:01

privitize persistedaccountdata

a7107e3

Fix accountdb tests

ffa5af2

algorandskiy and others added 11 commits September 2, 2022 19:39

Rename RowId to ID

7c0320e

Make AccountDelta.oldAcct private

9382825

Make AccountDelta.newAcct private

2206ed2

Fix blockdb linter

78731eb

Make AccountDelta.address private

2541bb3

Fix some linter warnings

639abe7

persistedonlineaccountdata

fb42587

Merge branch 'master' into nguo/accountdb-pkg

0f45740

fix tests

ae96ce0

Make CompactAccountDeltas.insert private

4a57e90

More private fields in OnlineAccountDelta

5b5631f

nicholasguo added 3 commits September 7, 2022 10:48

persistedResourceData

e4c5b53

Merge branch 'nguo/accountdb-pkg' of https://github.com/nicholasguoal…

ef20561

…gorand/go-algorand into nguo/accountdb-pkg

fix test

22cde16

algorandskiy requested a review from algonautshant September 7, 2022 20:52

algorandskiy and others added 5 commits September 9, 2022 10:46

Move vacuumDatabase from account updates

54bae2a

add some comments to exported functions

52f1d1f

finish fixing lint

3158eb6

fix

8677628

Fix more linter

3292fb7

algorandskiy marked this pull request as ready for review September 13, 2022 16:16

algorandskiy requested a review from icorderi September 13, 2022 20:08

cce reviewed Sep 14, 2022

View reviewed changes

icorderi approved these changes Sep 15, 2022

View reviewed changes

algorandskiy closed this Nov 9, 2022

icorderi mentioned this pull request Nov 9, 2022

ledger: move pieces of accountdb.go into a storage package #4776

Merged

ledger: move accountdb to its own package #4428

ledger: move accountdb to its own package #4428

Uh oh!

Conversation

ghost commented Aug 18, 2022 • edited by algorandskiy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

algorandskiy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cce Sep 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

icorderi left a comment

Choose a reason for hiding this comment

Uh oh!

algorandskiy commented Nov 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ghost commented Aug 18, 2022 •

edited by algorandskiy

Loading

codecov bot commented Sep 2, 2022 •

edited

Loading

cce Sep 14, 2022 •

edited

Loading