This repository has been archived by the owner on Jan 22, 2025. It is now read-only.
Limit leader schedule search space #8468
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
mvines
reviewed
Feb 26, 2020
|
Patch looks good in concept, CI seems to hate it though :) Totally file a new issue for "what still sucks" too. |
Codecov Report
@@ Coverage Diff @@
## master #8468 +/- ##
========================================
- Coverage 80.3% 80.3% -0.1%
========================================
Files 256 256
Lines 56468 56478 +10
========================================
+ Hits 45351 45354 +3
- Misses 11117 11124 +7 |
mvines
approved these changes
Feb 26, 2020
mergify bot
pushed a commit
that referenced
this pull request
Feb 26, 2020
* Limit leader schedule search space * Fix and add test * Rename (cherry picked from commit 7a2bf7e)
mergify bot
pushed a commit
that referenced
this pull request
Feb 26, 2020
* Limit leader schedule search space * Fix and add test * Rename (cherry picked from commit 7a2bf7e)
solana-grimes
pushed a commit
that referenced
this pull request
Feb 26, 2020
solana-grimes
pushed a commit
that referenced
this pull request
Feb 27, 2020
# for free
to subscribe to this conversation on GitHub.
Already have an account?
#.
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.
Problem
The leader schedule cache
next_leader_slotlooks for a range of consecutive slots for which a given validator is the leader. If that range is big, potentially multiple epochs if there is only one leader, this search can take a lot of time, since each hit also incurs a lookup in Rocks.Summary of Changes
Once the first leader slot
Nis found, limit the last slot toN + GRACE_TICKS_FACTOR * MAX_GRACE_SLOTS. This means:N + GRACE_TICKS_FACTOR * MAX_GRACE_SLOTSGRACE_TICKS_FACTOR * MAX_GRACE_SLOTSlookups in RocksDb because a validator only does a lookup if the slot belongs to them.What still sucks: You still have to iterate through
Nentries in the schedule, granted these iterations are a lot faster because they do not incur a RocksDb lookup. @pgarg66 Maybe we should cache these lookups so the validator doesn't have to start from the beginning every time.Fixes #8445