docs: inter-block cache specification #14370
Conversation
docs/spec/store/interblock-cache.md
Outdated
|
|
||
| #### Iteration | ||
|
|
||
| Iteration is not supported. |
There was a problem hiding this comment.
In the implementation, since CommitKVStoreCache embeds a CommitKVStore, it does expose the CommitKVStore iteration API.
Since this is just a system model, I guess it's sound to underspecify here, but probably surprising to the reader?
There was a problem hiding this comment.
I've added a sentence. Please check.
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
|
|
||
| ## Synopsis | ||
|
|
||
| The inter-block cache is an in-memory cache storing (in-most-cases) immutable state that modules need to read in between blocks. When enabled, all sub-stores of a multi store, e.g., `rootmulti`, are wrapped. |
There was a problem hiding this comment.
can we drop (in-most-cases)?
| The inter-block cache is an in-memory cache storing (in-most-cases) immutable state that modules need to read in between blocks. When enabled, all sub-stores of a multi store, e.g., `rootmulti`, are wrapped. | |
| The inter-block cache is an in-memory cache storing immutable state that modules need to read in between blocks. When enabled, all sub-stores of a multi store, e.g., `rootmulti`, are wrapped. |
There was a problem hiding this comment.
Actually, I wonder if there is any guarantee that the inter-block cache stores the immutable state. Thoughts on what enables this? Is it the ARC? If so, I will highlight this.
There was a problem hiding this comment.
It's not immutable. If a key is updated, the cache will be updated.
There was a problem hiding this comment.
I know it is not immutable. My point is whether there is anything in place that in case of reaching the max capacity of the cache, it is likely that immutable state remains cache. My guess is that the fact that the cache is an ARC may help with that: it tracks both frequency and recency of use. Thus, under the assumption that immutable state is more frequently queried, the ARC may help guaranteeing that this is almost always cached, even when max capacity is reached.
If we agree that the above is correct, I will highlight that it is important that the cache implementation is an ARC (or something similar that enables the above), instead of just discussing it as an implementation detail.
Also, I want to get rid of the word immutable, it is confusing: nothing is immutable per se, we only mean keys that a rarely updated.
|
|
||
| The inter-block cache requires that the cache implementation to provide methods to create a cache, add a key/value pair, remove a key/value pair and retrieve the value associated to a key. In this specification, we assume that a `Cache` feature offers this functionality through the following methods: | ||
|
|
||
| * `NewCache(size: int)` creates a new cache with `size` capacity and returns it. |
There was a problem hiding this comment.
| * `NewCache(size: int)` creates a new cache with `size` capacity and returns it. | |
| * `NewCache(size int)` creates a new cache with `size` capacity and returns it. |
nit: Cannot we use Go syntax everywhere?
| * `Add(key: string, value: []byte)` inserts a key/value pair into the `Cache`. | ||
| * `Remove(key: string)` removes the key/value pair identified by `key` from `Cache`. | ||
|
|
||
| The specification also assumes that `CommitKVStore` offers the following API: |
There was a problem hiding this comment.
Like here, why not simply display a Go interface?
|
|
||
| `CommitKVCacheManager` implements the cache manager. It maintains a mapping from a store key to a `KVStore`. | ||
|
|
||
| ```typescript |
…os-sdk into manuel/interblock-cache-spec
The main reason is here. In any case, if everyone feels more comfortable with just using go, it is fine with me, it was just a recommendation. If we decide to go for go, we should update the template accordingly. |
It does not need to be valid syntax (so it solve your point, like #14020 (comment)), but as it lives in the docs, and the SDK is mainly Go, it feels weird to have another language. |
|
merging this, we can do follow ups |
Description
Contributes to: #12986
Proposes a specification for the inter-block cache feature following the spec template.
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...
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...
!in the type prefix if API or client breaking change