Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VoteLastValid Counting Cache #5704

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

AlgoAxel
Copy link
Contributor

@AlgoAxel AlgoAxel commented Aug 23, 2023

WIP

Still WIP, large chunks and explanations may change.

What

Establishes a new caching mechanism in the Online Accounts Tracker, named ExpiringStakeCache

Why

Presently, the calls to ExpiredOnlineCirculation in the Online Account tracker will return a basics.MicroAlgos representing "how much stake is expiring on round votingRnd, from the perspective of the chain at round rnd". Under the hood, this call does a query which scans a large portion of the Online Accounts table.

We are interested in calling ExpiredOnlineCirculation more often, especially during ledger apply. From this, there is a concern about the performance of running to the database on every evaluation.

Therefore, a cache has been put in place to serve some of the calls which would otherwise hit the database. This should improve performance when getting expiring stake in these new scenarios

How

Two new interface/structures are in votelastvalidcache.go:

Double Index Map

A double index map is used to store V using a primary and secondary key (Ka and Kb).Data about the account's expiring stake is stored at [round][address]->V . An additional map stores [address]->round so that callers only need to know the address of the account, or the round they want to retrieve and they get an O(1) lookup.

ExpiringStakeCache

Expiring Stake Cache uses a double index map and a trimBehind value in order to keep record of all the expiring stake per account per round. This map is updatable using AccountDeltas from the deferred commiit context in CommitRound. It intentionally does not use account deltas, as the Online Account Tracker handles the play-forward of that data.

In Online Account Tracker,

At request time: onlineAcctsExpiredByRound

When getting the Online Accounts Expired By Round, a new parameter noCache is included. This is intended for testing purposes, to confirm that the behavior of this function is the same whether the cache is used or not.

When onlineAcctsExpiredByRound collects the DBRound via snapshot, if the DB Round is the requested rnd from the caller, then we know we can serve the the cache results. Otherwise, we use the existing SQL Query.

** Why not use the cache always?** : The OAER function takes rnd as a parameter representing the temporal perspective of the call. Take for example the following data:

round = 1, account X will expire on Round 400
round = 5, account X will expire on Round 399
round =10, account X will expire on Round 500

In this situation, the most up to date truth is that X will expire on Round 500. However, the existing utility of this call is to say "When will X expire from the perspective of round rnd?" From the perspective of round 8, X expired on Round 399.

To calculate this result, we need to look at all account updates which are not newer than the perspective round rnd. To achieve this today, we use a table scan which finds the most up-to-date update which is not newer than the perspective. In the cache, however, we would have to store all these updates in some journaled fashion in order to know which updates are applicable to which request. At that point we are storing essentially an entire database view in memory.

So instead, the cache only knows the expiring stake per round and account from the perspective of the current database round. This data is easy to keep, and since callers like eval will be looking for the most up to date information, this cache works for their calls.

At commit time: postCommit

When the dbTracker commits a round to the database, once complete it updates its trackers via a call to postCommit, under a lock.
When post commit runs, all updates from the deferred commit context are fed to the cache. In this way, if the database has been updated, so has the cache :)

Tests

Existing unit tests

Ongoing

  • Unit tests - need tests for the new structures, and for the new tracking functionality of OnlineAccount Tracker
  • Finish Wiring - need to expose the collected data to the ledger-apply functionality
  • Limits in Consensus.Params - we need a maximum amount of stake to compare against, and also a maximum distance to the supplied VoteLastValid. Limiting the distance of VLV keeps this cache's memory usage bounded, where it otherwise would not be.

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #5704 (cc2839f) into master (b212f94) will decrease coverage by 0.01%.
Report is 4 commits behind head on master.
The diff coverage is 67.67%.

@@            Coverage Diff             @@
##           master    #5704      +/-   ##
==========================================
- Coverage   55.60%   55.59%   -0.01%     
==========================================
  Files         474      475       +1     
  Lines       66619    66714      +95     
==========================================
+ Hits        37042    37089      +47     
- Misses      27058    27098      +40     
- Partials     2519     2527       +8     
Files Changed Coverage Δ
ledger/store/trackerdb/data.go 80.94% <ø> (ø)
ledger/votelastvalidcache.go 60.52% <60.52%> (ø)
ledger/acctonline.go 79.06% <91.30%> (-0.19%) ⬇️

... and 15 files with indirect coverage changes

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

Copy link
Contributor

@algorandskiy algorandskiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline, this cache without per-address info would not work correctly.

Copy link

@ASISBusiness ASISBusiness left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants