-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
docs(store/v2): update store v2 docs #19502
Conversation
WalkthroughThe recent updates involve a significant restructuring of the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 8
Configuration used: .coderabbit.yml
Files selected for processing (4)
- store/README.md (1 hunks)
- store/commitment/README.md (1 hunks)
- store/storage/README.md (1 hunks)
- store/storage/storage_bench_test.go (1 hunks)
Additional comments: 9
store/README.md (2)
- 3-5: The introduction provides a clear overview of the
store
package and its significance within the Cosmos SDK, linking to relevant design documents. This is well-structured and informative.- 13-15: The explanation of pruning responsibilities is clear and concise, effectively communicating that pruning is handled by the underlying layers rather than the
root.Store
itself.store/commitment/README.md (2)
- 3-6: The introduction succinctly outlines the purpose of the
commitment
package and its integration with the broader store/v2 architecture, providing a good starting point for understanding the SC implementation.- 8-18: The design goals and requirements for SC backends are clearly articulated, emphasizing flexibility and reduced scope. This section effectively communicates the shift in responsibilities, enhancing the clarity of the SC's role within the system.
store/storage/README.md (5)
- 3-18: The introduction and explanation of the state storage (SS) implementation's goals and responsibilities are clear and comprehensive. It effectively sets the stage for understanding the role of SS within the Cosmos SDK.
- 36-40: The PebbleDB section effectively highlights its role as an alternative to RocksDB and the implications of lacking native support for ColumnFamilies. The trade-offs between implementation complexity and the benefits of a pure Go solution are well articulated.
- 44-46: The SQLite implementation description is concise and informative, emphasizing its straightforward SQL-based approach. This section effectively communicates the benefits of using SQLite for SS.
- 55-65: The inclusion of benchmarks provides valuable performance insights for the different SS backends. However, ensuring that these benchmarks are up-to-date and reflect the latest performance characteristics is crucial.
- 70-75: The explanation of pruning functionality within SS backends is clear, detailing how pruning configuration and execution are handled. This section effectively communicates the flexibility and implementation-specific nature of pruning.
|
||
## Migration | ||
|
||
<!-- TODO --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Migration" section is marked with a "TODO" comment, indicating incomplete documentation. It's important to either complete this section or remove it if it's not applicable.
The `store` package contains a `root.Store` type which is intended to act as an | ||
abstraction layer around it's two primary constituent components - state storage (SS) | ||
and state commitment (SC). It acts as the main entry point into storage for an | ||
application to use in server/v2. Through `root.Store`, an application can query | ||
and iterate over both current and historical data, commit new state, perform state | ||
sync, and fetch commitment proofs. | ||
|
||
A `root.Store` is intended to be initialized with already constructed SS and SC | ||
backends (see relevant package documentation for instantiation details). Note, | ||
from the perspective of `root.Store`, there is no notion of multi or single tree/store, | ||
rather these are implementation details of SS and SC. For SS, we utilize store keys | ||
to namespace raw key/value pairs. For SC, we utilize an abstraction, `commitment.CommitStore`, | ||
to map store keys to a commitment trees. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Usage" section effectively describes the purpose and functionality of the root.Store
type. However, it's important to ensure that the explanation of how root.Store
interacts with SS and SC backends is clear to readers unfamiliar with these concepts.
Consider adding a brief explanation or example of what SS (state storage) and SC (state commitment) backends are, to make this section more accessible to newcomers.
An SC implementation may choose not to provide historical proofs past height `h - n` | ||
(`n` can be 0) due to the time and space constraints, but since store/v2 defines | ||
an API for historical proofs there should be at least one configuration of a | ||
given SC backend which supports this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The explanation regarding historical proofs and their optional nature based on backend configuration is informative. However, it might benefit from a brief example or further clarification on how this impacts the choice of SC backend.
Consider adding an example or further explanation to clarify the implications of the optional nature of historical proofs for different SC backends.
|
||
## Pruning | ||
|
||
<!-- TODO --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Pruning" section is marked with a "TODO" comment, indicating incomplete documentation. Completing this section or providing a brief explanation of pruning in the context of SC would be beneficial.
store/commitment/README.md
Outdated
|
||
## State Sync | ||
|
||
<!-- TODO --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "State Sync" section is also marked with a "TODO" comment. Given the importance of state sync in blockchain systems, completing this section with relevant information would enhance the documentation's comprehensiveness.
if err == nil && db != nil { | ||
db.SetSync(false) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting synchronization to false for the PebbleDB instance before returning it is a significant change aimed at improving performance in testing scenarios. While this can speed up tests, it's crucial to document the rationale behind this decision and its potential impact on test reliability.
Consider adding a comment explaining why synchronization is disabled and how it might affect the reliability of tests, especially in scenarios where write durability is critical.
store/storage/README.md
Outdated
|
||
## State Sync | ||
|
||
<!-- TODO --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "State Sync" section is marked with a "TODO" comment, indicating incomplete documentation. Completing this section with relevant information on how SS backends handle state sync would enhance the documentation's comprehensiveness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, very informative and easy to read 👌
I can't check for correctness tho, maybe a quick ACK from @kocubinski and/or @cool-develope
## Pruning | ||
|
||
The `root.Store` is NOT responsible for pruning. Rather, pruning is the responsibility | ||
of the underlying SS and SC layers. This means pruning can be implementation specific, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add a small key legend to explain what ss and sc are
…dk into bez/store-v2-doc-updates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 10
Configuration used: .coderabbit.yml
Files selected for processing (2)
- store/commitment/README.md (1 hunks)
- store/storage/README.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- store/commitment/README.md
store/storage/README.md
Outdated
* Provided fast and efficient queries for versioned raw key/value pairs | ||
* Provide versioned CRUD operations | ||
* Provide versioned batching functionality | ||
* Provide versioned iteration (forward and reverse) functionality | ||
* Provide pruning functionality |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list of responsibilities and functions of SS is comprehensive and well-defined. However, there's a minor typo in the first bullet point: "Provided" should be "Provide". Ensuring grammatical accuracy can enhance the document's professionalism.
- * Provided fast and efficient queries for versioned raw key/value pairs
+ * Provide fast and efficient queries for versioned raw key/value pairs
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
* Provided fast and efficient queries for versioned raw key/value pairs | |
* Provide versioned CRUD operations | |
* Provide versioned batching functionality | |
* Provide versioned iteration (forward and reverse) functionality | |
* Provide pruning functionality | |
* Provide fast and efficient queries for versioned raw key/value pairs | |
* Provide versioned CRUD operations | |
* Provide versioned batching functionality | |
* Provide versioned iteration (forward and reverse) functionality | |
* Provide pruning functionality |
Co-authored-by: cool-developer <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
store/storage/README.md
Outdated
The SQLite implementation is a native Go SS implementation. It fully supports the | ||
`VersionedDatabase` API and the implementation is relatively straightforward and | ||
easy to understand as it’s SQL-based. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description of the SQLite implementation is clear. This section might benefit from a brief mention of potential use cases or scenarios where SQLite would be the preferred choice over RocksDB or PebbleDB, providing readers with practical guidance on selecting an SS backend.
@coderabbitai can you please stop commenting on this PR. You're adding way too much useless noise. |
@alexanderbez, understood. I'll pause my comments on this PR to reduce noise. If you need further assistance in the future, feel free to mention me again. Action performedReviews paused. |
@alexanderbez: Understood, I will stop commenting on this PR. If you need further assistance in the future, feel free to reach out. Thank you for your feedback. Action performedReviews paused. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `storage` package contains the state storage (SS) implementation. Specifically, | ||
it contains RocksDB, PebbleDB, and SQLite (Btree) backend implementations of the | ||
`VersionedDatabase` interface. | ||
|
||
The goal of SS is to provide a modular storage backend, i.e. multiple implementations, | ||
to facilitate storing versioned raw key/value pairs in a fast embedded database, | ||
although an embedded database is not required, i.e. you could use a replicated | ||
RDBMS system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The introduction provides a clear overview of the storage
package's purpose and capabilities. However, to further enhance the document's clarity and usefulness, consider incorporating a brief explanation of the significance of versioned raw key/value pairs in blockchain or distributed ledger technologies. This addition could help readers understand the importance of the features discussed.
### RocksDB | ||
|
||
The RocksDB implementation is a CGO-based SS implementation. It fully supports | ||
the `VersionedDatabase` API and is arguably the most efficient implementation. It | ||
also supports versioning out-of-the-box using User-defined Timestamps in | ||
ColumnFamilies (CF). However, it requires the CGO dependency which can complicate | ||
an app’s build process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description of the RocksDB implementation is informative and highlights its efficiency and versioning capabilities. To improve accessibility for readers who might be unfamiliar with CGO, consider adding a brief explanation of what CGO is and providing a link to more detailed resources.
The PebbleDB implementation is a native Go SS implementation that is primarily an | ||
alternative to RocksDB. Since it does not support CF, results in the fact that we | ||
need to implement versioning (MVCC) ourselves. This comes with added implementation | ||
complexity and potential performance overhead. However, it is a pure Go implementation | ||
and does not require CGO. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The explanation of PebbleDB's implementation provides valuable insights. Including a brief explanation of MVCC (Multi-Version Concurrency Control) could offer readers a deeper understanding of the versioning challenges and enhance the document's educational value.
Benchmarks for basic operations on all supported native SS implementations can | ||
be found in `store/storage/storage_bench_test.go`. | ||
|
||
At the time of writing, the following benchmarks were performed: | ||
|
||
```shell | ||
name time/op | ||
Get/backend_rocksdb_versiondb_opts-10 7.41µs ± 0% | ||
Get/backend_pebbledb_default_opts-10 6.17µs ± 0% | ||
Get/backend_btree_sqlite-10 29.1µs ± 0% | ||
ApplyChangeset/backend_pebbledb_default_opts-10 5.73ms ± 0% | ||
ApplyChangeset/backend_btree_sqlite-10 56.9ms ± 0% | ||
ApplyChangeset/backend_rocksdb_versiondb_opts-10 4.07ms ± 0% | ||
Iterate/backend_pebbledb_default_opts-10 1.04s ± 0% | ||
Iterate/backend_btree_sqlite-10 1.59s ± 0% | ||
Iterate/backend_rocksdb_versiondb_opts-10 778ms ± 0% | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The benchmarks section is valuable for understanding the performance characteristics of each SS backend. To aid interpretation, consider including a brief analysis or commentary on the benchmarks. This could help readers grasp the practical implications of these performance metrics in real-world applications.
Pruning is an implementation and responsibility of the underlying SS backend. | ||
Specifically, the `StorageStore` accepts `store.PruneOptions` which defines the | ||
pruning configuration. During `ApplyChangeset`, the `StorageStore` will check if | ||
pruning should occur based on the current height being committed. If so, it will | ||
delegate a `Prune` call on the underlying SS backend, which can be defined specific | ||
to the implementation, e.g. asynchronous or synchronous. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pruning functionality description is clear. Adding an example or a more detailed explanation of how pruning configurations can vary between different SS backends could provide readers with a clearer understanding of the available customization options.
State storage (SS) does not have a direct notion of state sync. Rather, `snapshots.Manager` | ||
is responsible for creating and restoring snapshots of the entire state. The | ||
`snapshots.Manager` has a `StorageSnapshotter` field which is fulfilled by the | ||
`StorageStore` type, specifically it implements the `Restore` method. The `Restore` | ||
method reads off of a provided channel and writes key/value pairs directly to a | ||
batch object which is committed to the underlying SS engine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The State Sync section is well-explained. Including a diagram or flowchart to visually represent the interaction between snapshots.Manager
, StorageSnapshotter
, and the SS engine could significantly enhance comprehension, especially for visual learners.
|
||
## Non-Consensus Data | ||
|
||
<!-- TODO --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Non-Consensus Data" section is marked as a TODO. Completing this section is crucial for ensuring the documentation's completeness. Non-consensus data handling is an important aspect of state storage, and readers will benefit from understanding how it is managed within the Cosmos SDK.
Would you like assistance in drafting content for this section?
An SS backend is meant to be used within a broader store implementation, as it | ||
only stores data for direct and historical query purposes. We define a `Database` | ||
interface in the `storage` package which is mean to be represent a `VersionedDatabase` | ||
with only the necessary methods. The `StorageStore` interface is meant to wrap or | ||
accept this `Database` type, e.g. RocksDB. | ||
|
||
The `StorageStore` interface is an abstraction or wrapper around the backing SS | ||
engine can be seen as the the main entry point to using SS. | ||
|
||
Higher up the stack, there should exist a `root.Store` implementation. The `root.Store` | ||
is meant to encapsulate both an SS backend and an SC backend. The SS backend is | ||
defined by this `StorageStore` implementation. | ||
|
||
In short, initialize your SS engine of choice and then provide that to `NewStorageStore` | ||
which will further be provided to `root.Store` as the SS backend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usage section effectively describes how an SS backend integrates within a broader store implementation. Including code snippets or examples demonstrating the initialization of an SS engine and its integration with NewStorageStore
and root.Store
could significantly enhance the document's utility by offering readers a clear, actionable guide on implementing the described concepts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
Documentation
commitment
package documentation with details on state commitment (SC) implementation, emphasizing swappable backends, historical proofs, and snapshot functionality.storage
package documentation to include detailed descriptions of theVersionedDatabase
interface implementations like RocksDB, PebbleDB, and SQLite (Btree), highlighting versioned CRUD operations, iteration, and pruning.Refactor