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

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

Open
altergui opened this issue Apr 18, 2023 · 3 comments
Open
Labels
bug Something isn't working

Comments

@altergui
Copy link
Contributor

altergui commented Apr 18, 2023

right now the indexer Commit():

  • indexes new votes found the freshly committed block
  • updates account balances

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.

@altergui altergui added the bug Something isn't working label Apr 18, 2023
@altergui altergui changed the title reported account balances are misleading during commit of a block with votes to be indexed api returns outdated account balances during commit of a block with votes to be indexed Apr 18, 2023
@altergui altergui changed the title api returns outdated account balances during commit of a block with votes to be indexed api returns outdated account balances during commit of a block Apr 18, 2023
@altergui altergui changed the title api returns outdated account balances during commit of a block api briefly returns outdated account balances during commit of a block Apr 18, 2023
@mvdan
Copy link
Contributor

mvdan commented May 4, 2023

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
Copy link
Contributor

mvdan commented May 4, 2023

For reference, I mean faster via changes like #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
Copy link
Member

p4u commented May 18, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants