pokt-network / pocket

Official implementation of the Pocket Network Protocol v1
https://pokt.network
MIT License
63 stars 33 forks source link

[Persistence][Core][Savepoints/Rollbacks] Implement KISS SavePoints - Serialize WorldState from Persistence #327

Open jessicadaugherty opened 1 year ago

jessicadaugherty commented 1 year ago

Objective

Revisit the TODOs used as placeholders throughout the persistence module codebase related to save points.

Make sure that the SavePoint is created when the UtilityContext (soon to be renamed to UtilityUnitOfWork) is instantiated.

NOTE: This ticket is dependent on #563

Origin Document

Goals

Deliverable

Non-goals / Non-deliverables

General issue deliverables

Testing Methodology


Creator: @jessicadaugherty - rescope: @deblasis Co-creator: @Olshansk

Olshansk commented 1 year ago

@jessicadaugherty Updated the description. Please review

h5law commented 1 year ago

@jessicadaugherty Is it worth looking into using pgx's builtin subtransaction savepoints and rollbacks? As far as I know, the tx.Begin() command will automatically create a savepoint and then you can call tx.Rollback() at any point before tx.Commit() to revert to the most recent savepoint.

See: pgx rollback implementation And: pgx Tx docs

This way we could have NewSavepoint(pgx.Tx) (pgx.Tx, error) and use this functionality across the board for subtransaction rollbacks calling RollbackToSavepoint(pgx.Tx) error

I am not super clear on how another form of rollbacks could be implemented as Postgres doesn't support them outside of transactions (I believe). Unless it were to be a DB incremental backup and restore scenario?

deblasis commented 1 year ago

Hi @h5law ! I have received a notification since the issue is assigned to me so I'll try to answer your question on @jessicadaugherty's behalf :)

You might be on the right path on the Postgres side of things but I think it's worth clarifying that savepoints and rollbacks have to also consider blockStore, txIndexer and stateTrees:

type persistenceModule struct {
    bus          modules.Bus
    config       *configs.PersistenceConfig
    genesisState *genesis.GenesisState

    blockStore kvstore.KVStore
    txIndexer  indexer.TxIndexer
    stateTrees *stateTrees

    // TECHDEBT: Need to implement context pooling (for writes), timeouts (for read & writes), etc...
    writeContext *PostgresContext // only one write context is allowed at a time
}

Essentially, we need to implement whatever is necessary so that everything we persist can have Savepoints and Rollback functionality. The whole thing has to work in unison across all the datastores.

Any ideas or pointers from your point of view are more than welcome.

Please let me know if this makes sense, happy to answer any further questions you might have.

h5law commented 1 year ago

@deblasis

Any ideas or pointers from your point of view are more than welcome.

My knowledge of SMTs is very very limited but from a little reading

func (s *SMT) Revert(toOldRoot []byte) error {

"When Revert is called, the trees to rollback (between the current tree and toOldRoot) are deleted from the database. This process can be slow so for a quick rollback, manually set the Root to the state you want to revert to and reload the cache."

Resetting the root seems to be a fast implementation.

BlockStore and txIndexer are both using kvstore.KVStore for their internal db, again I will keep looking into it as I am not super familiar with this yet but it seems the savepoint/rollback functionality extends as far as the KVStore and SparseMerkleTree types as well as PostgreSQL.

If this is correct I'll try to find out more on each of these and see what comes up.

Olshansk commented 1 year ago

To add to what @deblasis said, you're definitely on the write part w.r.t the Postgres piece.

In shared/modules/persistence_module.go, the first few functions of PersistenceWriteContext are:

type PersistenceWriteContext interface {
    // Context Operations
    NewSavePoint([]byte) error
    RollbackToSavePoint([]byte) error
    Release() error

    // Commits the current context (height, hash, transactions, etc...) to disk (i.e. finality).
    Commit(proposerAddr, quorumCert []byte) error

    // ...
}

And the last few lines current implementation of Commit are:

func (p PostgresContext) Commit(proposerAddr, quorumCert []byte) error {
    //...

    // Commit the SQL transaction
    ctx := context.TODO()
    if err := p.getTx().Commit(ctx); err != nil {
        return err
    }
    if err := p.conn.Close(ctx); err != nil {
        log.Println("[TODO][ERROR] Implement connection pooling. Error when closing DB connecting...", err)
    }

    return nil
}

There's still some work to make the postgres part work correctly (if you navigate the code you'll understand what I mean), but most of the piece are setup to enable that. It's "easy" because we can leverage rollbacks in a SQL db, which we use for state storage.

For state commitment (i.e. computing the hash), the hard part is that we have to make the change (i.e. update the key-value stores) after validating the transactions, and verify that the root is what we expect it to be.

In essence, here's what we need to do (pseudo-code):

for tx in transactions:
   // Start a postgres transaction
   // Create "ephemeral" version of our key-value store (which are the one's backing our trees)
   if tx is valid:
      pgx.Tx.Apply(stateChange)
      tree.Apply(stateChange) // update key value || delete existing key || insert new key
   else: // exception, error, invalid tx, etc...
      pgx.Tx.rollback()
      tree.Revert() // revert updated keys to old value || re-insert deleted keys || delete new keys

if new_state_hash != proposed_state_hash:
    // same rollback logic as above            

The hardest part of what I described above is // revert updated keys to old value || re-insert deleted keys || delete new keys because it either requires one of the following:

  1. looking for functionality in the KV stores that enables this OR
  2. Maintaining some sort of in-memory cache that allows rolling these OR
  3. Having logic that loops through the transactions in the block being reverted and determines the proper logic to revert

I haven't thought through the details here deeply so the above is just where I would start thinking/looking if I were to implement it myself. There are still some open questions and we'll need to properly design this piece.

jessicadaugherty commented 1 year ago

Likely need to split into multiple issues:

jessicadaugherty commented 1 year ago

@deblasis @Olshansk please rescope this ticket based on outcome of review of https://github.com/pokt-network/pocket/issues/493