Trie: Critical DB-Consisteny Bug #3264
Comments
|
btw I made a backup of that corrupted state in the database. you can basically run it with these options and it should help you reproduce it |
|
Thanks for reporting this and coming up with a backup and reproduction scenario. I'll see if I can reproduce this locally and get back. Workload is high currently with ongoing developments, but this is something we need to get to the bottom of as it could perhaps reveal more general issues that go beyond the scope of your implementation. |
|
So to recap and confirm my understanding, here's what I'm getting from the issue:
Obviously, this should not be happening. However it is hard for me to get a good grasp on the actual issue since it happens in the context of external usage that I have limited familiarity and understanding of. From the issue you've linked, I see these lines in the logs: And before that, I see this error: I'm not sure what exactly the expected order of operations, but might this be a race condition where the root is set independantly (e.g. through the trie.root() method) without the If I'm misunderstanding the issue, would you be able to provide a relatively minimal reproductible example (that can be run natively in the context of our repository). Then I'd be able to troubleshoot further, and generate test cases. |
For months now the reconciliation algo has been plagued by bugs surrounding the ethereumjs/trie library. We've opened many tickets on Github: - #146 - #131 - ethereumjs/ethereumjs-monorepo#3264 - ethereumjs/ethereumjs-monorepo#3645 The pattern of the problem was always that somehow that `trie.root()` couldn't be found using `trie.checkRoot`, which seemed almost like a contradiction, especially when doing `await trie.checkRoot(trie.root())`. We had initially introduced the checkpointing of the trie because of some rather theoretical problem regarding what would happen if during the reconciliation the trie updates and, at the same times, sends level comparisons to a peer. So to use checkpointing for us was primarily used to implement atomicity when storing data. We wanted to just store the remote trie's leaves in batches as to make sure not to interrupt the algorithm to compare the trie's levels. At the same time, the insertion of new leaves into such a trie is costly as a big part of its hashes have to be recomputed to arrive at a new root. However, I think what has happened with our implementation of the sync.put method is that the checkpointing led to the trie writes not being processed sequentially which also lead to all sorts of problems in the reconciliation. The reconciliation is purposefully built in a way where it first synchronizes old leaves and only then new leaves. While a working reconciliation doesn't have any issues with storing comments, a fundamentally asynchronous reconciliation will attempt to store comments where the original upvote hasn't been made yet, leading to the message not being processed initially. Another big problem ended up being that the ethereumjs/trie library isn't mature with regards to handling the application shutting down, and so a lot of the above mentioned issues actually describe the ethereumjs/trie library reaching a non-recoverable state. Funnily enough, however, all it took to fix all of the above problems was to remove all notions of checkpointing and commits. While it does make the reconciliation algorithm MUCH slower (because it is now synchronous), it also made it much more reliable and almost free of errors during interaction.
For months now the reconciliation algo has been plagued by bugs surrounding the ethereumjs/trie library. We've opened many tickets on Github: - #146 - #131 - ethereumjs/ethereumjs-monorepo#3264 - ethereumjs/ethereumjs-monorepo#3645 The pattern of the problem was always that somehow that `trie.root()` couldn't be found using `trie.checkRoot`, which seemed almost like a contradiction, especially when doing `await trie.checkRoot(trie.root())`. We had initially introduced the checkpointing of the trie because of some rather theoretical problem regarding what would happen if during the reconciliation the trie updates and, at the same times, sends level comparisons to a peer. So to use checkpointing for us was primarily used to implement atomicity when storing data. We wanted to just store the remote trie's leaves in batches as to make sure not to interrupt the algorithm to compare the trie's levels. At the same time, the insertion of new leaves into such a trie is costly as a big part of its hashes have to be recomputed to arrive at a new root. However, I think what has happened with our implementation of the sync.put method is that the checkpointing led to the trie writes not being processed sequentially which also lead to all sorts of problems in the reconciliation. The reconciliation is purposefully built in a way where it first synchronizes old leaves and only then new leaves. While a working reconciliation doesn't have any issues with storing comments, a fundamentally asynchronous reconciliation will attempt to store comments where the original upvote hasn't been made yet, leading to the message not being processed initially. Another big problem ended up being that the ethereumjs/trie library isn't mature with regards to handling the application shutting down, and so a lot of the above mentioned issues actually describe the ethereumjs/trie library reaching a non-recoverable state. Funnily enough, however, all it took to fix all of the above problems was to remove all notions of checkpointing and commits. While it does make the reconciliation algorithm MUCH slower (because it is now synchronous), it also made it much more reliable and almost free of errors during interaction.
|
No follow-up, will close. Feel free to re-open if you have got further information! |
Some related follow up here |
here's what I just wrote down:
attestate/kiwistand#131
I have reported this to Gabriel before but not sure if y'all have looked into this. My suspicion is that somehow the ethereumjs library throws or anyways doesn't finish a write somehow. Or that maybe I kill the ethereumjs execution and then this leads to inconsistency. But in anycase, IMO, I should never manage to get any irrecoverable state from ethereumjs/trie. Happy to answer any questions
The text was updated successfully, but these errors were encountered: