-
Notifications
You must be signed in to change notification settings - Fork 28
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
chore: avoid using RocksDB keys estimation #496
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38aa5b3
to
58899d6
Compare
599c77c
to
c436d15
Compare
msbrogli
requested changes
Oct 11, 2022
58899d6
to
cb6ba8f
Compare
c436d15
to
5283917
Compare
luislhl
commented
Oct 18, 2022
Codecov Report
@@ Coverage Diff @@
## dev #496 +/- ##
==========================================
- Coverage 83.69% 83.57% -0.13%
==========================================
Files 184 187 +3
Lines 16452 16829 +377
Branches 2520 2590 +70
==========================================
+ Hits 13770 14065 +295
- Misses 2230 2300 +70
- Partials 452 464 +12
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
4001833
to
9bb01d2
Compare
9bb01d2
to
3e76498
Compare
jansegre
approved these changes
Oct 24, 2022
msbrogli
approved these changes
Oct 25, 2022
3e76498
to
1df4287
Compare
…e it to get_vertices_count
… changing its state
1df4287
to
9de646f
Compare
jansegre
approved these changes
Nov 1, 2022
msbrogli
approved these changes
Nov 1, 2022
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This was created from the discussion #490 (comment)
Acceptance Criteria
We should prefer to use the InfoIndex when possible to get a count of the number of vertices, instead of relying on specific logic for each kind of storage to count the number of vertices.
Specifically, we notices that the specific logic we had to count the number of vertices in RocksDB was not accurate. See the link above for more info.
This change highlighted a bug we had in the InfoIndex count of txs/blocks when a split-brain happens. It was counting the txs/blocks of the winner chain twice when the nodes connected again and the winner was chosen. We had to fix this bug in this PR as well.