Skip to content
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

Replay slots from blocktree in new forks #2975

Merged
merged 1 commit into from
Mar 2, 2019

Conversation

aeyakovenko
Copy link
Member

@aeyakovenko aeyakovenko commented Feb 27, 2019

Problem

Replay stage is not tracking forks correctly as they are delivered to Blocktree.

Summary of Changes

Replay stage main loop:

  1. for all frozen forks, ask blocktree for any new slots chaining to those forks, and create new banks
  2. for all live forks, ask blocktree for any new blobs for those forks
  3. if any live forks process to a max tick for the slot, vote
  4. goto 1

Fixes #

@aeyakovenko aeyakovenko changed the title Replay slots from blocktree in new forks [very wip] Replay slots from blocktree in new forks Feb 27, 2019
src/bank_forks.rs Outdated Show resolved Hide resolved
src/tvu.rs Outdated
@@ -34,7 +33,7 @@ use std::sync::{Arc, RwLock};
use std::thread;

pub struct TvuRotationInfo {
pub bank: Arc<Bank>, // Bank to use
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you wouldn't rather just take a bank and a slot?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rob-solana block tree doesn't have any data for the next bank yet.

// RPC can be made aware of last slot's bank
to_leader_sender
.send(TvuRotationInfo {
tick_height: bank.tick_height(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how come we don't just send the new Bank?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rob-solana because the notification will move into replay stage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rob-solana where is the new bank? i just have a parent in that context.

runtime/src/bank.rs Outdated Show resolved Hide resolved
src/bank_forks.rs Outdated Show resolved Hide resolved
src/bank_forks.rs Outdated Show resolved Hide resolved
src/replay_stage.rs Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 28, 2019

Codecov Report

Merging #2975 into master will increase coverage by <.1%.
The diff coverage is 94.1%.

@@           Coverage Diff            @@
##           master   #2975     +/-   ##
========================================
+ Coverage    77.4%   77.4%   +<.1%     
========================================
  Files         132     132             
  Lines       20257   20207     -50     
========================================
- Hits        15680   15660     -20     
+ Misses       4577    4547     -30

@aeyakovenko aeyakovenko force-pushed the simple_replay_stage branch 2 times, most recently from 6bc7c16 to 65b8a66 Compare March 1, 2019 18:39
@rob-solana
Copy link
Contributor

looks tight

@aeyakovenko aeyakovenko force-pushed the simple_replay_stage branch from 65b8a66 to fd0e764 Compare March 1, 2019 21:03
@aeyakovenko aeyakovenko changed the title [very wip] Replay slots from blocktree in new forks Replay slots from blocktree in new forks Mar 2, 2019
@aeyakovenko aeyakovenko marked this pull request as ready for review March 2, 2019 00:00
@aeyakovenko aeyakovenko requested a review from mvines March 2, 2019 00:01
// TODO: Remove this soon once we boot the leader from ClusterInfo
cluster_info.write().unwrap().set_leader(leader_id);
if next_leader == my_id {
let mut wforks = bank_forks.write().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make this a one-liner? these wXXX or rXXX bug my eyes...

bank_forks.write().unwrap().insert(tpu_bank);

@aeyakovenko aeyakovenko force-pushed the simple_replay_stage branch from 1a6bb3e to 52bd20d Compare March 2, 2019 04:19
@garious
Copy link
Contributor

garious commented Mar 2, 2019

Can you rebase?

@aeyakovenko aeyakovenko force-pushed the simple_replay_stage branch from 52bd20d to 6b2697f Compare March 2, 2019 04:25
@aeyakovenko aeyakovenko added the automerge Merge this Pull Request automatically once CI passes label Mar 2, 2019
@solana-grimes solana-grimes merged commit b1a6481 into solana-labs:master Mar 2, 2019
wen-coding added a commit to wen-coding/solana that referenced this pull request Oct 8, 2024
* wen_restart: Add wen_restart_coordinator argument.

* rename LEADER_INDEX with COORDINATOR_INDEX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants