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

Clarify/Change merkelized DB expected delete API #12989

Closed
ValarDragon opened this issue Aug 22, 2022 · 4 comments
Closed

Clarify/Change merkelized DB expected delete API #12989

ValarDragon opened this issue Aug 22, 2022 · 4 comments
Labels

Comments

@ValarDragon
Copy link
Contributor

ValarDragon commented Aug 22, 2022

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)

@alexanderbez
Copy link
Contributor

Ref: #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.

@catShaark
Copy link
Contributor

catShaark commented Oct 14, 2022

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

@tac0turtle
Copy link
Member

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants