Skip to content

replay: extend last fec set check for 32+ retransmitter signed shreds #2101

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.

Sign up for GitHub

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

Merged
AshwinSekar merged 11 commits into from
Jul 18, 2024
Merged

replay: extend last fec set check for 32+ retransmitter signed shreds #2101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8070c3a
replay: extend last fec set check for 32+ retransmitter signed shreds
AshwinSekar Jul 11, 2024
498a131
pr feedback: use separate feature flag
AshwinSekar Jul 16, 2024
3c2a3a5
pr feedback: is_retransmitter_signed -> is_retransmitter_signed_varia…
AshwinSekar Jul 16, 2024
9a3445a
pr feedback: update doc comment fail -> error
AshwinSekar Jul 16, 2024
f0935ae
pr feedback: hash -> bank_hash for report metrics
AshwinSekar Jul 16, 2024
2c11c74
refactor metrics inside blockstore fn, return block_id for future use
AshwinSekar Jul 16, 2024
4ecfbba
pr feedback: gate metrics reporting
AshwinSekar Jul 16, 2024
9916e60
pr feedback: do not distinguish impossible combos, simplify check code
AshwinSekar Jul 17, 2024
b27d78e
pr feedback: remove report_metrics helper fn
AshwinSekar Jul 17, 2024
c88824c
pr feedback: remove metric
AshwinSekar Jul 18, 2024
df32e7b
pr feedback: block_id -> last_fec_set_merkle_root
AshwinSekar Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 14 additions & 27 deletions core/src/replay_stage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3076,37 +3076,22 @@ impl ReplayStage {
}
}

if bank.collector_id() != my_pubkey {
// If the block does not have at least DATA_SHREDS_PER_FEC_BLOCK shreds in the last FEC set,
// mark it dead. No reason to perform this check on our leader block.
if !blockstore
.is_last_fec_set_full(bank.slot())
.inspect_err(|e| {
warn!(
"Unable to determine if last fec set is full for slot {} {},
marking as dead: {e:?}",
bank.slot(),
bank.hash()
)
})
.unwrap_or(false)
{
// Update metric regardless of feature flag
datapoint_warn!(
"incomplete_final_fec_set",
("slot", bank_slot, i64),
("hash", bank.hash().to_string(), String)
);
Comment thread
AshwinSekar marked this conversation as resolved.
if bank
.feature_set
.is_active(&solana_sdk::feature_set::vote_only_full_fec_sets::id())
{
let _block_id = if bank.collector_id() != my_pubkey {
Comment thread
AshwinSekar marked this conversation as resolved.
// If the block does not have at least DATA_SHREDS_PER_FEC_BLOCK correctly retransmitted
Comment thread
bw-solana marked this conversation as resolved.
// shreds in the last FEC set, mark it dead. No reason to perform this check on our leader block.
match blockstore.check_last_fec_set_and_get_block_id(
bank.slot(),
bank.hash(),
&bank.feature_set,
) {
Ok(block_id) => block_id,
Err(result_err) => {
Copy link
Copy Markdown

@behzadnouri behzadnouri Jul 18, 2024

Choose a reason for hiding this comment

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

Instead of match, it might be more readable to use inspect_err:
https://doc.rust-lang.org/std/result/enum.Result.html#method.inspect_err
followed by unwrap_or_default.

Copy link
Copy Markdown
Author

@AshwinSekar AshwinSekar Jul 18, 2024

Choose a reason for hiding this comment

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

It doesn't work because we can't continue from the inspect_err closure.

let root = bank_forks.read().unwrap().root();
Self::mark_dead_slot(
blockstore,
bank,
root,
&BlockstoreProcessorError::IncompleteFinalFecSet,
&result_err,
rpc_subscriptions,
duplicate_slots_tracker,
duplicate_confirmed_slots,
Expand All @@ -3120,7 +3105,9 @@ impl ReplayStage {
continue;
}
}
}
} else {
None
};

let r_replay_stats = replay_stats.read().unwrap();
let replay_progress = bank_progress.replay_progress.clone();
Expand Down
Loading