Skip to content
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

refactor: disable remaining fast node logic in mutable tree #589

Merged
merged 3 commits into from
Oct 16, 2022

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Oct 14, 2022

Skip fast node removals when fast node is disabled to avoid any possibility of those removals being read, leading to inconsistent state between nodes that have fast nodes and the nodes that do not.

@p0mvn p0mvn requested a review from a team as a code owner October 14, 2022 19:28
@faddat
Copy link
Contributor

faddat commented Oct 15, 2022

thank you from Notional and evmos

But really, fastnode has worked 100% of the time, and it's baroque to if wannagofast=false {allkinds of stuff} everywhere in the codebase.

seriously why would we kill art itself because some archives took a long time to upgrade?

they're archives, after all.

fastnode worked 100% of the time, until there was this zany idea we might not want to use it because multi-terabyte archive nodes with millions of accounts took a long time to upgrade....

v-homsi pushed a commit to v-homsi/iavl that referenced this pull request Oct 15, 2022
@tac0turtle tac0turtle enabled auto-merge (squash) October 16, 2022 08:42
@tac0turtle tac0turtle merged commit 78a5f11 into master Oct 16, 2022
@tac0turtle tac0turtle deleted the iavl/disable-fast-node-hash branch October 16, 2022 08:46
@yihuang
Copy link
Collaborator

yihuang commented Oct 18, 2022

leading to inconsistent state between nodes that have fast nodes and the nodes that do not.

does this cause observable inconsistency, like app-hash mismatch, or just a preventive change?

@p0mvn
Copy link
Member Author

p0mvn commented Oct 18, 2022

leading to inconsistent state between nodes that have fast nodes and the nodes that do not.

does this cause observable inconsistency, like app-hash mismatch, or just a preventive change?

Just a preventive change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants