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

Implement Default SC #17227

Closed
Tracked by #17041
alexanderbez opened this issue Jul 31, 2023 · 10 comments
Closed
Tracked by #17041

Implement Default SC #17227

alexanderbez opened this issue Jul 31, 2023 · 10 comments
Assignees
Labels

Comments

@alexanderbez
Copy link
Contributor

alexanderbez commented Jul 31, 2023

See #17226 for implementation details and ideas. The current proposal is to borrow ideas from memIAVL and tweak things to make them more performant and ergonomic, which also fits the SC API

@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Jul 31, 2023
@alexanderbez alexanderbez added C:Store WG: Storage and removed needs-triage Issue that needs to be triaged labels Jul 31, 2023
@tac0turtle
Copy link
Member

are we pulling memiavl into a repo in the cosmos org or rewriting it?

@alexanderbez
Copy link
Contributor Author

I'd like for @kocubinski and @cool-develope to provide their thoughts here on that.

From my POV, I believe we should bring it into the cosmos org no matter what.

Now as for re-writing, my mental model is that we're probably better off porting the existing Cronos version of memIAVL with all our improvements, fixes, and benchmarks.

@cool-develope
Copy link
Contributor

I'd like to rewrite it since I can see it is far away from the current store v2 design.

@kocubinski
Copy link
Member

I think MemIAVL should continue to be what its README says it is: Alternative IAVL Implementation.

cosmos/iavl would be better off evolving to include the performance improvement strategies which MemIAVL uses without the tight coupling to its implementation. These strategies (common to many production-grade databases) are:

  • Push writes to a WAL, pin in cache, and flush at checkpoints. Implemented as periodic full snapshots in MemIAVL.
  • A in-memory page cache. MemIAVL splits this cache into pinned/dirty nodes, MemNode, and mmap snapshotted nodes PersistentNode
  • A performant, well-researched storage format. (MemIAVL uses a custom format to layout an AVL tree on disk)

The storage format/backend is not swappable in MemIAVL, but imo should be. For page cache, AFAIK in MemIAVL, it's not possible to limit the dirty cache size between snapshots, which could lead to unpredictable memory usage, and mmap at the OS level is an abstraction away, and memory caps are not easily controllable there either.

For a storage format I doubt we'll ever beat LSM for writes or sequential reads given the iavl-v1 node key format and the upper bound on SC size in store v2. If all we did was introduce a WAL, a memory bounded cache with second chance or Clock page replacement, and infrequent full checkpoints we'd have the same improvements.

I prototyped some of this in https://github.com/cosmos/iavl/tree/avlite and produced a benchmark with good results.

Maybe we should also wait for an answer in historic proof feasibility and performance against periodic full backups, since this requires replaying and rebuilding the tree in memory to obtain a proof.

@tac0turtle
Copy link
Member

@cool-develope is iavl v1 integrated?

@cool-develope
Copy link
Contributor

yes, for the current SC API (PoC)

@alexanderbez
Copy link
Contributor Author

Do you feel that is sufficient to close this issue then @cool-develope?

@cool-develope
Copy link
Contributor

yeah, if we agree with creating a new issue for memiavl integration and separating the pruning and snapshotting from the SC.

@alexanderbez
Copy link
Contributor Author

@cool-develope please close this when ready 👍

@cool-develope
Copy link
Contributor

cool-develope commented Sep 8, 2023

iavl V1 integration is ready

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