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

Proposed new repo API #737

Open
ValarDragon opened this issue Apr 10, 2023 · 0 comments
Open

Proposed new repo API #737

ValarDragon opened this issue Apr 10, 2023 · 0 comments

Comments

@ValarDragon
Copy link
Contributor

Currently the IAVL interfaces here make no sense, and as a huge part of that, touching IAVL is always incredibly overly complex.

This issue proposes some first-pass ways that I think we should be able to get out with confidence, and significantly reduce complexity of working with this code base.

Lets recap what the existing parts of our code are rn:

  • MutableTree
    • This is the entry point for end-users, and is imo the bulk of why the code is so confusing. It serves the purpose of:
      • Managing versions. So the caller uses this to delete old versions, see what versions are availalbe
    • Managing WIP state being committed to and built up
      • (where your set API is)
    • Getting data at old versions, and latest versions
    • Getting Read-Only copies of old versions.
  • ImmutableTree
    • A read-only copy of a version. Most of the time old versions, but can also be taken out on latest version. Guarantees are at least unclear to me, for what happens when taking it for latest version while MutableTree has rights
  • NodeDB
    • an internal only object, thats responsible for: Reading/Writing nodes / branches to disk, along the tree structure
    • Responsible for saving/deleting versions
    • The object passed into "everything"

Phased proposals:

1. Make a DB version manager API as the entrypoint

So make a new struct, that first acts as something intended to manage the various versions.
TODO: Consider renaming version here to "Block Height"

TreeReader for now is just a type alias for ImmutableTree

NewIavlDB(database, options) IavlDB

func (*IavlDB) VersionExists(version uint64) bool
func (*IavlDB) GetLatestVersion) TreeReader
func (*IavlDB) GetVersion(version uint64) TreeReader
// do whatever delete APIs we want to use.
func (*IavlDB) DeleteVersion(version uint64) error
func (*IavlDB) Rollback() error

// Only one WriteBatch can be open at once. SaveVersion deletes that write batch.
func (*IavlDB) StartWriteBatch() WriteBatch
func (*IavlDB) SaveVersion(batch WriteBatch) error

// TODO: Consider deleting?
func (*IavlDB) SetInitialHeight(version uint64)

TreeReader in the above is just a name alias for immutable tree.
In phase 1, the DB version manager can maintain a mutable tree instance, and have write batch just be that.

2. Refactor WriteBatch to be its own type

Basically move all of the logic in the MutableTree.Set to be done on the new WriteBatch struct.
Move the SaveVersion logic from MutableTree onto IavlDB

Delete MutableTree

3. Other

Theres other cleanup we can/should do that will make the repo cleaner. (E.g. a lot of NodeDB code cleanup, making api's private, etc.)

But I think the overall flow will be far simpler to follow / feel confident about, with the above two in.

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

No branches or pull requests

1 participant