cleanup block indices for missing blocks #15040
Open
+558
−212
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
kasey
force-pushed
the
repair-idx-on-read
branch
from
c9fa7ad to
c4aa1da
Compare
kasey
changed the title
kasey
force-pushed
the
repair-idx-on-read
branch
from
cb0e184 to
137d9b6
Compare
nisdas
reviewed
Mar 18, 2025
| // - Seeking before the beginning of the bucket (the `start` param is lower than the lowest slot). | ||
| // In both of these cases k,v will be nil and we can handle the same way using `edge` to | ||
| // - continue to the next iteration. If the following Prev() key/value is nil, Prev has gone past the beginning. | ||
| // - Otherwise, iterate as usual. |
There was a problem hiding this comment.
Do we have a unit test for these particular edge cases ?
There was a problem hiding this comment.
Yes, TestStore_Blocks_FiltersCorrectly that touch the upper bounds of the blocks with SetStartSlot + SetEndSlot. LMK if that is sufficient, or if you think we need a separate test.
Comment on lines
+139
to
+152
| func (q *QueryFilter) SimpleSlotRange() (primitives.Slot, primitives.Slot, bool) { | ||
| if len(q.queries) != 2 || q.queries[StartSlot] == nil || q.queries[EndSlot] == nil { | ||
| return 0, 0, false | ||
| } | ||
| start, ok := q.queries[StartSlot].(primitives.Slot) | ||
| if !ok { | ||
| return 0, 0, false | ||
| } | ||
| end, ok := q.queries[EndSlot].(primitives.Slot) | ||
| if !ok { | ||
| return 0, 0, false | ||
| } | ||
| return start, end, true | ||
| } |
There was a problem hiding this comment.
Good candidate for unit test
kasey
force-pushed
the
repair-idx-on-read
branch
3 times, most recently
from
b9a428d to
360b869
Compare
kasey
force-pushed
the
repair-idx-on-read
branch
from
360b869 to
2c57ad0
Compare
nisdas
previously approved these changes
Mar 28, 2025
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.
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
For any existing nodes impacted by the block cleanup bug fixed recently, this is a hack to help those nodes self-heal and avoid a complete db wipe and resync.
**Notes for reviewers **
blockParentRootIndicesBucketmaps from a block root to its children. When the db is in this state, we don't have the parent root of the missing block to clean up the index going the other direction. As I was wrapping this PR up I did think a couple ways to do it.blocksForSlotRange, we could keep track of roots in the previous slot so that when we add something to thebadBlocksslice for cleanup we have parent roots to check for the missing root. If the block root is in the first slot of the range we won't have seen its possible parents and need to fall back to method 1, so this is really just an optimization for method 1 and might not be worth the complexity.Acknowledgements