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

Store states in the DB before their blocks #712

Merged
merged 1 commit into from
Dec 12, 2019
Merged

Conversation

michaelsproul
Copy link
Member

Issue Addressed

#692

Proposed Changes

We've been seeing a few issues with database inconsistency on the testnet, so this is a quick patch to hopefully address those issues. I'd like to address the issue more thoroughly once our choice of DB has settled down (or we come up with an API to abstract over atomic transactions in different DBs).

@michaelsproul michaelsproul added the ready-for-review The code is ready for review label Dec 11, 2019
@michaelsproul
Copy link
Member Author

To test the effectiveness of this change I'm going to try syncing a node where I drop 50% of the block storing ops... it should work, but it'll be slower.

@michaelsproul
Copy link
Member Author

My test didn't end up producing many conclusive results. I ended up making it so that when storing the block the node would panic with probability 0.001, and this seemed to allow a few recoveries (start, sync, die, repeat) until it got stuck. I think this is still an improvement over the current state of things (I didn't see any DBInconsistent errors).

Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this doesn't fully solve the problem, but it makes a significant step towards it. Nice!

@paulhauner paulhauner added ready-to-merge and removed ready-for-review The code is ready for review labels Dec 12, 2019
@michaelsproul michaelsproul merged commit 4eba265 into master Dec 12, 2019
@michaelsproul michaelsproul deleted the state-before-block branch December 12, 2019 01:48
@AgeManning AgeManning mentioned this pull request Dec 19, 2019
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.

2 participants