vocdoni / vocdoni-node

A set of libraries and tools for the Vocdoni decentralized backend infrastructure, the main ground of our universally verifiable, privacy-centric and scalable digital voting protocol
GNU Affero General Public License v3.0
86 stars 17 forks source link

api briefly returns outdated account balances during commit of a block #890

Open altergui opened 1 year ago

altergui commented 1 year ago

right now the indexer Commit():

step 1 might take significant time (1 second, seen in CI), during this interval any queries to accounts return the outdated balances.

that indexing maybe can be optimized to take milliseconds instead of seconds, but that will simply push the problem forward, to 10000 votes per block instead of 100 votes per block.

one very simple solution would be to hold a Lock, so that all reads are blocked until Commit finishes. right now this is not the case, only RLock is held.

func (idx *Indexer) Commit(height uint32) error {
    idx.lockPool.RLock()
    defer idx.lockPool.RUnlock()

problem is, Commit is called synchronously

    // Notify listeners about the commit state
    for _, l := range v.eventListeners {
        if err := l.Commit(height); err != nil {
            if _, fatal := err.(ErrHaltVochain); fatal {
                return nil, err
            }
            log.Warnf("event callback error on commit: %v", err)
        }
    }

so the refactor should include turning that into a goroutine. this might bring other consecuences, tho. careful redesign is needed.

mvdan commented 1 year ago

FYI, Indexer.Commit will get dramatically faster on the sqlite side over the next week or so, so I would say wait until that's finished - it might be enough for the foreseeable future. Adding more goroutines is very risky, because more often than not, that adds all kinds of races and correctness issues unless we're very, very careful.

mvdan commented 1 year ago

For reference, I mean faster via changes like https://github.com/vocdoni/vocdoni-node/pull/906. In short, doing more work upfront so that Commit doesn't need to execute queries, and using one transaction for all changes to be committed.

p4u commented 1 year ago

@altergui let's see if Mvdan's optimizations fix this issue.