Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ADR-040: Storage and SMT State Commitments #8430
ADR-040: Storage and SMT State Commitments #8430
Changes from all commits
11728cf
662ec91
5fdbe5d
fa8e9e3
864927e
78215b2
6dd0323
250b5ff
374916f
e90bf8a
8602b3e
ca39df5
aedce21
f704279
06d1952
7537c84
1cc123e
d321dac
80d0122
962a28b
bb89798
19d2126
356f987
42e7f08
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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'm defining database snapshot here, so I prefer to use snapshot mechanism here, so I prefer to keep the original language.
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.
Okay I may have misunderstood. This is what some DBs call snapshots, and distinct from state sync snapshots as used in the ABCI, right? (although it can be used to implement ABCI snapshots)
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, here we are talking about a database engine mechanism.
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 seems a bit confusing to me. Pruning of Snapshots and pruning of application states, currently, are two separate configurable parameters. Are we merging these two? If so can it worded this way.
What is the impact to disk size with this design?
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 you define application state pruning? For me, it is removing not needed records by a module (eg removing zero balances).
In this document I don't refer to "application state pruning". We don't do app state pruning here.
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 am talking about how we currently prune application states or versions. You are talking about pruning versions or snapshots which are used for versions. This is application state pruning.
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'm not sure I understand your concern. ADR-40 is not about pruning application state. Old SS (state storage) versions (a version of the whole state) are covered by snapshots. If we want to remove an old version we remove a snapshot.
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 isn't specific to abci.Query. Might make more sense to reword in the sense of querying.
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.
Why it's not specific for
abci.Query
? I don't see any other use-case than using the ABCI to query old state.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.
Users want to query old state as well.. Many dont want to go through abci.Query.
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 they do it now? Are you talking about a new feature?
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.
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.
same here - I prefer to be consistent and use snapshot.