From 15e1a0367450083d1d6f8f216583d3e6f017bb0d Mon Sep 17 00:00:00 2001 From: Cool Developer Date: Tue, 18 Jul 2023 15:17:44 -0400 Subject: [PATCH] away store v2 --- docs/architecture/adr-002-api-cleanup.md | 54 ++++++++++++------------ 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/docs/architecture/adr-002-api-cleanup.md b/docs/architecture/adr-002-api-cleanup.md index 9d63c4774..9a23344c8 100644 --- a/docs/architecture/adr-002-api-cleanup.md +++ b/docs/architecture/adr-002-api-cleanup.md @@ -1,4 +1,4 @@ -# ADR ADR-002: API Cleanup to Support Store V2 +# ADR ADR-002: API Cleanup to Improve Commit Performance ## Changelog @@ -10,18 +10,17 @@ DRAFT ## Abstract -This ADR proposes a cleanup of the API to support [Store V2](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-065-store-v2.md). - -There are some proposals for the speedup of the `Commit` by the async writes. See the [Discussion](https://github.com/cosmos/cosmos-sdk/issues/16173) for more details. +This ADR proposes a cleanup of the API to make more understandable and maintainable the codebase of `iavl`. There is a lot of legacy code in the SDK that is not used anymore and can be removed. See the [Discussion](https://github.com/cosmos/iavl/issues/737) for more details. +There are some proposals for the speedup of the `Commit` by the async writes. See the [Discussion](https://github.com/cosmos/cosmos-sdk/issues/16173) for more details. + ## Context -The introduction of `Store V2` separates SS (State Storage) and SC (State Commitment), where `iavl` is used for SC. The index of the `fast node` is not needed in `iavl` since SS allows key-value queries for a specific version. -Additionally, `Store V2` introduces a new approach to `SaveVersion` that accepts a batch object for the atomicity of the commit and removes the usage of `dbm.Batch` in `nodeDB`. +The current implementation of `iavl` suffers from performance issues due to synchronous writes during the `Commit` process. To address this, the proposed changes aim to finalize the current version in memory, parallelize hash calculations in `WorkingHash`, and introduce asynchronous writes in the background. -The current implementation of `iavl` suffers from performance issues due to synchronous writes during the `Commit` process. To address this, the proposed changes aim to finalize the current version in memory, parallelize hash calculations in `WorkingHash`, and introduce asynchronous writes. +It is also necessary to refactor the `Set` function to accept a batch object for the atomic commitments. Moreover, the existing architecture of `iavl` lacks modularity and code organization: @@ -30,12 +29,11 @@ Moreover, the existing architecture of `iavl` lacks modularity and code organiza ## Decision -To support `Store V2` and improve `Commit` performance, we propose the following changes: +To make the codebase more clear and improve `Commit` performance, we propose the following changes: -### Support Store V2 +### Batch Set -- Refactor the `SaveVersion` function to accept a batch object. -- Remove the index of the `fast node` from `iavl`. +- Refactor the `Set` function to accept a batch object. - Eliminate the usage of `dbm.Batch` in `nodeDB`. ### Async Commit @@ -46,27 +44,30 @@ To support `Store V2` and improve `Commit` performance, we propose the following ### API Cleanup -- Merge `ImmutableTree` and `MutableTree` into a single `Tree` structure. -- Make `nodeDB` methods private. +- Make `nodeDB` methods private and remove all unused methods. +- Follow the naming convention of the `store` module. The modified API will have the following structure: ```go - Tree interface { - Has(key []byte) (bool, error) - Get(key []byte) ([]byte, error) - SaveVersion(cs *ChangeSet) ([]byte, int64, error) - Version() int64 - Hash() []byte + MutableTree interface { + SaveChangeSet(cs *ChangeSet) ([]byte, error) // this is for batch set WorkingHash() []byte - VersionExists(version int64) bool + Commit() ([]byte, int64, error) // SaveVersion -> Commit + WaitForCommit() error // this is to make sure the async write is finished when shutdown DeleteVersionsTo(version int64) error - GetVersioned(key []byte, version int64) ([]byte, *cmtprotocrypto.ProofOps, error) - GetTree(version int64) (*iavl.Tree, error) SetInitialVersion(version uint64) - Iterator(start, end []byte, ascending bool) (types.Iterator, error) LoadVersionForOverwriting(targetVersion int64) error - WaitForCommit() error // this is for when gracefully shutdown + GetImmutableTree(version int64) (*ImmutableTree, error) + } + + ImmutableTree interface { + Has(key []byte) (bool, error) + Get(key []byte) ([]byte, error) + Hash() []byte + Iterator(start, end []byte, ascending bool) (types.Iterator, error) + VersionExists(version int64) bool + GetVersionedProof(key []byte, version int64) ([]byte, *cmtprotocrypto.ProofOps, error) } ``` ## Consequences @@ -75,11 +76,11 @@ We expect the proposed changes to improve the performance of Commit through asyn ### Backwards Compatibility -While this ADR will break the external API of `iavl`, it will not affect the internal state of nodeDB. Compatibility measures and migration steps will be necessary during the `Store V2` migration to handle the breaking changes. +While this ADR will break the external API of `iavl`, it will not affect the internal state of nodeDB. Compatibility measures and migration steps will be necessary during the API cleanup to handle the breaking changes. ### Positive -- Atomicity of the `Commit`. +- Atomicity of the `Commitments`. - Improved Commit performance through async writes and parallelized hash calculations. - Increased flexibility and ease of modification and refactoring for iavl. @@ -89,6 +90,5 @@ While this ADR will break the external API of `iavl`, it will not affect the int ## References -- [Store V2](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-065-store-v2.md) - https://github.com/cosmos/cosmos-sdk/issues/16173 - https://github.com/cosmos/iavl/issues/737 \ No newline at end of file