replay: do not start leader for a block we already have shreds for #2416
Conversation
|
I'll address |
AshwinSekar
force-pushed
the
start-leader-check-blockstore
branch
from
4da42a2 to
3928df1
Compare
|
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
…2416) * replay: do not start leader for a block we already have shreds for * pr feedback: comment, move existing check to blockstore fn * move blockstore read after tick height check * pr feedback: resuse blockstore fn in next_leader_slot (cherry picked from commit 15dbe7f) # Conflicts: # poh/src/poh_recorder.rs
| // as it was not part of the rooted fork. If this slot is not the first slot for this leader, | ||
| // and the first slot was previously ticked over, the check in `leader_schedule_cache::next_leader_slot` | ||
| // will not suffice, as it only checks if there are shreds for the first slot. |
There was a problem hiding this comment.
Interestingly enough fn next_leader_slot returns (start_slot, last_slot) and last_slot is used to calculate leader_last_tick_height in PohRecorder but we only use leader_last_tick_height inside would_be_leader. Maybe reached_leader_tick needs to use it as well?
There was a problem hiding this comment.
It could be an option, it would allow us to remove the check here for when we tick into the next leader:
agave/core/src/replay_stage.rs
Lines 2073 to 2075 in c99095d
However I think if we use it in reached_leader_tick, we should recompute our next leader window. Otherwise if we don't reset in time, we could end up missing our next leader window, which the current code prevents.
…for (backport of #2416) (#2484) * replay: do not start leader for a block we already have shreds for (#2416) * replay: do not start leader for a block we already have shreds for * pr feedback: comment, move existing check to blockstore fn * move blockstore read after tick height check * pr feedback: resuse blockstore fn in next_leader_slot (cherry picked from commit 15dbe7f) # Conflicts: # poh/src/poh_recorder.rs * fix conflicts --------- Co-authored-by: Ashwin Sekar <ashwin@anza.xyz> Co-authored-by: Ashwin Sekar <ashwin@solana.com>
…nza-xyz#2416) * replay: do not start leader for a block we already have shreds for * pr feedback: comment, move existing check to blockstore fn * move blockstore read after tick height check * pr feedback: resuse blockstore fn in next_leader_slot
Problem
See solana-labs#32679
In certain scenarios where the first leader block is not produced, however the second (or later) leader block is produced we can end up reproducing this block after resetting to a previous block.
Summary of Changes
When
poh_recorderchecks for leader slot, additionally checkblockstoreto see if shreds have already been inserted.