Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

removes first_coding_index from erasure recovery code#16646

Merged
behzadnouri merged 1 commit intosolana-labs:masterfrom
behzadnouri:rm-first-coding-index
Apr 23, 2021
Merged

removes first_coding_index from erasure recovery code#16646
behzadnouri merged 1 commit intosolana-labs:masterfrom
behzadnouri:rm-first-coding-index

Conversation

@behzadnouri
Copy link
Copy Markdown
Contributor

@behzadnouri behzadnouri commented Apr 19, 2021

Problem

first_coding_index is the same as the set_index and is so redundant:
https://github.com/solana-labs/solana/blob/37b8587d4/ledger/src/blockstore_meta.rs#L49-L60

Summary of Changes

  • removed first_coding_index

@behzadnouri behzadnouri changed the title removes first coding index from erasure recovery code removes first_coding_index from erasure recovery code Apr 19, 2021
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 19, 2021

Codecov Report

Merging #16646 (3180cfd) into master (26fa71a) will increase coverage by 0.0%.
The diff coverage is 100.0%.

❗ Current head 3180cfd differs from pull request most recent head 724d95c. Consider uploading reports for the commit 724d95c to get more accurate results

@@           Coverage Diff            @@
##           master   #16646    +/-   ##
========================================
  Coverage    82.9%    82.9%            
========================================
  Files         418      416     -2     
  Lines      115893   115463   -430     
========================================
- Hits        96183    95828   -355     
+ Misses      19710    19635    -75     

@behzadnouri behzadnouri force-pushed the rm-first-coding-index branch from edba3e5 to e05afea Compare April 21, 2021 12:57
@behzadnouri behzadnouri marked this pull request as ready for review April 21, 2021 12:59
@behzadnouri behzadnouri force-pushed the rm-first-coding-index branch from 2aec979 to 3180cfd Compare April 21, 2021 14:11
@behzadnouri behzadnouri requested review from carllin and sakridge April 21, 2021 15:35
@behzadnouri behzadnouri force-pushed the rm-first-coding-index branch from 3180cfd to 077278e Compare April 21, 2021 19:13
/// Which erasure set in the slot this is
pub set_index: u64,
/// First coding index in the FEC set
pub first_coding_index: u64,
Copy link
Copy Markdown
Contributor

@carllin carllin Apr 22, 2021

Choose a reason for hiding this comment

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

hmmm since these are stored/fetched from Rocks, is this going to cause compatibility issues if we try to fetch old ErasureMeta's?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

you are right. fixed

@behzadnouri behzadnouri force-pushed the rm-first-coding-index branch from 077278e to 874eee2 Compare April 22, 2021 11:57
@behzadnouri behzadnouri requested a review from carllin April 22, 2021 12:00
@behzadnouri behzadnouri force-pushed the rm-first-coding-index branch from 874eee2 to 724d95c Compare April 22, 2021 12:03
@behzadnouri behzadnouri merged commit 0319414 into solana-labs:master Apr 23, 2021
@behzadnouri behzadnouri deleted the rm-first-coding-index branch April 23, 2021 12:00
mergify Bot pushed a commit that referenced this pull request Apr 23, 2021
mergify Bot added a commit that referenced this pull request Apr 23, 2021
first_coding_index is the same as the set_index and is so redundant:
https://github.com/solana-labs/solana/blob/37b8587d4/ledger/src/blockstore_meta.rs#L49-L60

(cherry picked from commit 0319414)

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Dec 6, 2021
solana-labs#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a dedicated field to
represent.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Dec 6, 2021
solana-labs#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a separate field to
represent.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Dec 8, 2021
solana-labs#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a separate field to
represent.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Dec 9, 2021
solana-labs#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a separate field to
represent.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Dec 9, 2021
solana-labs#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a separate field to
represent.
behzadnouri added a commit that referenced this pull request Dec 10, 2021
#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a separate field to
represent.
mergify Bot pushed a commit that referenced this pull request Dec 10, 2021
#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a separate field to
represent.

(cherry picked from commit 49ba09b)
mergify Bot added a commit that referenced this pull request Dec 10, 2021
#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a separate field to
represent.

(cherry picked from commit 49ba09b)

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
mergify Bot pushed a commit that referenced this pull request Feb 3, 2022
#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a separate field to
represent.

(cherry picked from commit 49ba09b)

# Conflicts:
#	ledger/src/blockstore.rs
mergify Bot added a commit that referenced this pull request Feb 3, 2022
…2912)

* adds back ErasureMeta::first_coding_index field (#21623)

#16646
removed first_coding_index since the field is currently redundant and
always equal to fec_set_index.
However, with upcoming changes to erasure coding schema, this will no
longer be the same as fec_set_index and so requires a separate field to
represent.

(cherry picked from commit 49ba09b)

# Conflicts:
#	ledger/src/blockstore.rs

* removes mergify merge conflicts

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants