ledger: increase locks granularity in lookupWithoutRewards

[algorandskiy]

Summary

This PR is a rework of #5527 (later revered in #5620 because of test failure).

1. Take a state snapshot for a single iteration (the first part of the for {} loop in lookupWithoutRewards) - dbRound, deltas, offset, rewards, inDeltas check.
2. Make the nested loop over deltas lock free.
3. If not found in deltas check baseAccounts and ensure the ledger did not advance while we were checking deltas.

Why does it work:

say au.deltas has 10 elements that is backed by some array underneath. e make local deltas = au.deltas[0:len(au.deltas)], pointing to the same backing array. Then there are two options:

1. when receiving a new block => add element
   • if backing array has space, it is not reallocated, just appended, our deltas keeps pointing on it
   • if not enough space, then realloc for au.deltas but our deltas keeps pointing to the old backing array
2. when committing => remove elements
   • removal op is slicing, so au.deltas keeps pointing to the same backing array but to the right subset, our deltas is fine as well

The change would benefit from @icorderi work on making lru caches concurrency-safe and possible switch to a bound circular buffer of deltas.

Why is it safe

lookupWithoutRewards is used by block evaluator in txpool, ensureBlock, catchup to add a new block into the ledger, i.e. always has rnd=latest. Having the MaxAcctLookback > 1 (deltas size, actual value is 4) guarantees the requested rnd is greater than the flushed rounds, and for rnd=latest the invariant "value is in deltas, or is in baseAccounts if flushed recently, or in DB" still holds.

Test Plan

Existing tests should pass

[codecov]

Codecov Report

Merging #5637 (80b24d5) into master (5acab71) will increase coverage by 0.00%.
The diff coverage is 81.81%.

@@           Coverage Diff           @@
##           master    #5637   +/-   ##
=======================================
  Coverage   54.94%   54.94%           
=======================================
  Files         463      463           
  Lines       64476    64485    +9     
=======================================
+ Hits        35427    35433    +6     
+ Misses      26668    26667    -1     
- Partials     2381     2385    +4     

Files Changed	Coverage Δ
ledger/acctupdates.go	70.91% <81.81%> (-0.16%)	⬇️

... and 6 files with indirect coverage changes

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

[algonautshant]

Why do you think it is worth taking an optimistic (instead of preventing an error here with locks), and returning an error when the database advances?
There are operation that need to get canceled because an error is returned from here.

[algorandskiy]

This is only actual for rest api since txpool and block validator need the latest state of account to validate.

[zeldovich]

Looks reasonable to me. The only suggestion is, consider moving the two au.baseAccounts reads (read() and readNotFound()) to before the lock release. They seem cheap enough (just a map lookup in each one), and it seems safe to do the au.baseAccounts.writePending(macct) later without holding the lock, if we decide to go down that control flow path. Avoids an extraneous lock release and re-acquire in the case that we fall through to au.accountsq.LookupAccount etc.