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

removes branch to recover legacy shreds #4488

Merged
merged 1 commit into from
Jan 20, 2025

Conversation

behzadnouri
Copy link

@behzadnouri behzadnouri commented Jan 16, 2025

Problem

Legacy shreds are discarded on all clusters:
https://github.com/anza-xyz/agave/blob/91d0d0cae/ledger/src/shred.rs#L1275-L1277

Removing the branch to recover legacy shreds would allow to further simplify and optimize shreds recovery code.

Summary of Changes

The commit removes branch to recover legacy shreds.

@behzadnouri behzadnouri force-pushed the rm-recover-legacy-shreds branch from 288bfc8 to f62079f Compare January 16, 2025 17:32
@behzadnouri behzadnouri changed the title stops recovering legacy shreds removes branch to recover legacy shreds Jan 16, 2025
@behzadnouri behzadnouri force-pushed the rm-recover-legacy-shreds branch 3 times, most recently from 78df153 to f5b17dc Compare January 17, 2025 19:58
@behzadnouri behzadnouri force-pushed the rm-recover-legacy-shreds branch from f5b17dc to 1b28913 Compare January 18, 2025 13:50
carllin
carllin previously approved these changes Jan 19, 2025
Comment on lines +1116 to +1119
debug_assert_matches!(
shred.common_header().shred_variant,
ShredVariant::MerkleCode { .. } | ShredVariant::MerkleData { .. }
);
Copy link

Choose a reason for hiding this comment

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

is this necessary?

Copy link
Author

Choose a reason for hiding this comment

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

not really, just a minor sanity check.
I guess it should be fine since it is only "debug" assert.

Legacy shreds are discarded on all clusters:
https://github.com/anza-xyz/agave/blob/91d0d0cae/ledger/src/shred.rs#L1275-L1277

Removing the branch to recover legacy shreds would allow to further
simplify and optimize shreds recovery code.
@behzadnouri behzadnouri force-pushed the rm-recover-legacy-shreds branch from 48d5cb2 to 4b33a44 Compare January 20, 2025 00:56
@behzadnouri
Copy link
Author

@carllin I had to rebase to resolve merge conflicts.
Can you please re-approve once you get the chance? thanks.

@behzadnouri behzadnouri requested a review from carllin January 20, 2025 02:07
@behzadnouri behzadnouri merged commit 28acb1c into anza-xyz:master Jan 20, 2025
47 checks passed
@behzadnouri behzadnouri deleted the rm-recover-legacy-shreds branch January 20, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants