cosmos-sdk icon indicating copy to clipboard operation
cosmos-sdk copied to clipboard

Clarify/Change merkelized DB expected delete API

Open ValarDragon opened this issue 2 years ago • 3 comments

Summary

We historically have had very unclarified API expectations of the merkelized DB store, especially around deletes and replay. I suggest we clarify this, and pair down the functionality needed. This makes implementation of the database easier, gives us clear properties we can test for in the SDK for any DB, and will significantly improve maintenance of all of this.

We should make the API for deletion that:

  • Delete data needed for versions older than height {H}. DeleteHeightsUpTo(height uint64)
  • Delete/rollback data needed for versions at height {H} or newer. DeleteAndRollbackToHeight(height uint64)

Importantly, this is having no deleting of single versions, or allowing random states in the middle ("keep-every"). We just need to delay the prune versions older than {H} based upon pruning settings + whats pruned.

For replay, we need the following guarantees:

  • Due to non-atomic commits (and this being a property that should actually be preserved, as we shouldn't force different logical DB backends to share a physical db for a number of reasons), if there is a failure during block commit, then we should be able to replay that block and ignore old data entries for that version or at most check equality of result. Its unclear to me, why we'd not just overwrite the result at any time.
  • We want to be able to rollback to a prior state.

From this I think we derive the needs:

  • Delete/rollback data needed for versions at height {H} or newer.
  • Delete latest version
    • This can one day be optimized to allow overriding latest version, but that definitely seems like a premature optimization right now.

This is likely going to be the main place mental effort has to go in swapping the key format for IAVL to make sense (with the root cause being SDK expectations being unclear), and is something that will have to be tackled for any subsequent merkelized tree as well. (A nice feature is that with IAVL key format updates, rolling back becomes trivial)

ValarDragon avatar Aug 22 '22 11:08 ValarDragon

Ref: https://github.com/cosmos/cosmos-sdk/issues/12986

Would prefer if we triage effectively (accept or reject proposals) and have all items/issues linked and referenced in the core EPIC above.

Just only glanced at this, but it seems reasonable I think.

alexanderbez avatar Aug 22 '22 14:08 alexanderbez

@ValarDragon, Should we add the new API to the IAVL tree implementation in iavl repo or only in the iavl store in sdk repo

catShaark avatar Oct 14 '22 09:10 catShaark

This makes sense to me. the sort of more granular pruning shouldn't be handled by the sdk but an external process that you stream data to.

tac0turtle avatar Nov 30 '22 17:11 tac0turtle

this is done in the latest version of iavl. Closing this issue

tac0turtle avatar May 03 '23 08:05 tac0turtle