-
Notifications
You must be signed in to change notification settings - Fork 138
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
Flat Storage #399
Flat Storage #399
Conversation
Your Render PR Server URL is https://nomicon-pr-399.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-ccqvvs4gqg45e3h5vql0. |
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.
Some initial feedback. In general, looks very good to me.
neps/nep-0399.md
Outdated
Since one value ref takes 40 bytes, limit of total size of changed value refs in a block | ||
is then 3.2 MiB. | ||
|
||
To sum it up, we will have < 54 MiB for one block, and ~1.1 GiB for 20 blocks. |
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.
- How do we ensure that FSDs are kept in memory? Will we use a memory only data struct for them like hash maps?
- Do we know how many memory accesses we need to check if a worst case FSD of 54 MiB has a given key or not?
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.
- Yes. They will be stored in
FlatStateDelta
. They are also persisted on disk, but we don't read from disk during block processing. We only read from disk for this column when node first starts and FlatStorage loads deltas for all blocks after the flat head into memory. - Most of the times 2 because in most case there is no fork, and the chain head should be at last final block + 2. Worst case, it can be up to he number of blocks between the last final block to this block. If we implement the idea of setting gas limit to zero for blocks that are further than X blocks from the final block, then the number of in memory hashmap lookup will be X.
|
neps/nep-0399.md
Outdated
|
||
However, the consensus algorithm doesn’t provide any guarantees in the distance of blocks | ||
that we need to process since it could be arbitrarily long for a block to be finalized. | ||
To solve this problem, we make another proposal (TODO: attach link for the proposal) to |
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.
This TODO needs to be resolved (or removed) before accepting.
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.
I plan to write separate NEP for this. WIP.
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.
I suppose the additional NEP will cover this topic in detail. I am curious about the case where h
or one its children has many outgoing branches vs. h
has a single deep branch. Will the proposal below work just fine in both cases and ensure that we do not store too many FSD in memory?
neps/nep-0399.md
Outdated
FlatStorage only needs to store at most X FSDs in memory. | ||
|
||
### FSD size estimation | ||
To set the value of X, we need to see how many block deltas can fit in memory. |
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.
Is there any chance on abnormal conditions of the network (maybe during an attack), that this value of X
is not large enough, and the client will return a different value after reading than the one expected?
As I understand, we are fetching this value from a non-finalized block, so there is a chance for this to happen, even if highly improbable. How would the system react in that case?
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.
Good question! This is actually the part that I left out about the proposal to set gas limit to zero for blocks with height larger than the latest final block’s height + X. If we implement that proposal, blocks that are further than X blocks away from the last final block can't include any new chunks, in other words, they are just empty blocks. This is a way to allow the blockchain to recover in case of some abnormal conditions. With that change, FlatStorage only need to store deltas for blocks from the last final block to last final block + X, thus at most X blocks. Any other block further than last final block + X will just have an empty delta.
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.
Didn't mean to approve, it was a misclick.
Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>
As the moderator, I would like to thank @mzhangmzz for submitting this NEP, and for the @near/wg-protocol working group members for your review. Based on the voting indications above, it seems like this proposal is close to a decision. We are therefore scheduling the fourth Protocol Work Group call, where this NEP can enter the final voting stage. Anyone can discuss the technical details by adding your comments to this discussion. And join the call to learn about the final decision and how to get more involved. Meeting Info |
Thank you to everyone who attended the fourth Protocol Working Group meeting last Thursday! The working group members reviewed the NEP and reached the following consensus: Status: Approved Meeting Recording: @mzhangmzz and @Longarithm Thank you for authoring this NEP! Next Steps:
|
@mzhangmzz Are we ready to merge this NEP? I just want to double-check that there are no outstanding last-minute fixes. |
Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>
Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>
Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>
Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>
We are ready to merge. I applied Bowen's feedback about mentioning protocol change explicitly, and I agree that it is worth a separate section. Also shortened "storage costs" sections because now we have better-written proposals. |
In Cosmos I made a research for a similar problem. I proposed a two DB model:
Note, that the state design in Cosmos SDK is a bit different: currently each module can have it's own store (which leads to potential problems related to failures caused by not atomic writes). The ADR: https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-040-storage-and-smt-state-commitments.md |
Proposal of Flat Storage to improve state storage reads, which aims to make state storage more stable and make fees more predictable, and sketch the plan for future improvements.