Skip to content
This repository has been archived by the owner on Apr 6, 2020. It is now read-only.

Fix putBlock() edge case #79

Merged
merged 1 commit into from
Dec 7, 2018
Merged

Fix putBlock() edge case #79

merged 1 commit into from
Dec 7, 2018

Conversation

vpulim
Copy link
Contributor

@vpulim vpulim commented Dec 3, 2018

This fixes an intermittent bug in putBlock() that is caused by _saveHeads() and batchDbOps() not being executed as part of the same db transaction. If _saveHeads() completes but batchDbOps() fails, it can lead to the head information being inconsistent with the actual head block.

@vpulim vpulim requested a review from holgerd77 December 3, 2018 03:09
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.934% when pulling 24e2744 on fix-save-heads into f96aa9a on master.

@holgerd77
Copy link
Member

I am low on time for the week, if someone else could jump in for the review would be glad!

@whymarrh
Copy link
Contributor

whymarrh commented Dec 3, 2018

@vpulim I restarted one of the builds that had saw a Travis apt-get error. I'll take a closer look soon.

@vpulim
Copy link
Contributor Author

vpulim commented Dec 6, 2018

@whymarrh Sorry to be pushy, but the client is blocking on this in order to address a major stability bug. It looks like the travis build is passing now. Let me know if you have any questions on the code changes. Essentially, the change ensures that if any of the db ops fail in putBlock(), the whole operation fails. This will keep the db in the correct state in case anything goes wrong during a putBlock() operation.

Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

Thanks @vpulim

@whymarrh whymarrh merged commit 4283cae into master Dec 7, 2018
@whymarrh whymarrh deleted the fix-save-heads branch December 7, 2018 11:39
@vpulim
Copy link
Contributor Author

vpulim commented Dec 7, 2018

@whymarrh awesome! thank you

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

Successfully merging this pull request may close these issues.

4 participants