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

Bitwise compress incomplete epoch slots#8341

Merged
pgarg66 merged 1 commit into
solana-labs:masterfrom
pgarg66:compress-incomplete-epoch-slots
Feb 20, 2020
Merged

Bitwise compress incomplete epoch slots#8341
pgarg66 merged 1 commit into
solana-labs:masterfrom
pgarg66:compress-incomplete-epoch-slots

Conversation

@pgarg66
Copy link
Copy Markdown
Contributor

@pgarg66 pgarg66 commented Feb 20, 2020

Problem

Incomplete slot information in EpochSlots could cause the message to grow larger than MTU size. The current message format is not ABI forward compatible with changes required to split it in MTU sized chunks.

Summary of Changes

  1. Add an index field to EpochSlots gossip message
  2. Store slots as bits in the compressed list
  3. Store multiple bitmaps so that disjoint incomplete slot information can be transmitted over multiple packets.

Copy link
Copy Markdown
Member

@aeyakovenko aeyakovenko left a comment

Choose a reason for hiding this comment

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

Perfect!

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 20, 2020

Codecov Report

Merging #8341 into master will increase coverage by <.1%.
The diff coverage is 91.6%.

@@           Coverage Diff            @@
##           master   #8341     +/-   ##
========================================
+ Coverage    80.5%   80.5%   +<.1%     
========================================
  Files         253     253             
  Lines       55412   55416      +4     
========================================
+ Hits        44637   44642      +5     
+ Misses      10775   10774      -1

@pgarg66 pgarg66 merged commit ea8d9d1 into solana-labs:master Feb 20, 2020
@pgarg66 pgarg66 deleted the compress-incomplete-epoch-slots branch February 20, 2020 04:24
@mvines mvines added the v0.23 label Feb 20, 2020
@mvines
Copy link
Copy Markdown
Contributor

mvines commented Feb 20, 2020

This needs to be backported to 0.23 since it's an ABI change to Gossip, and thus must be picked up when we restart TdS

mergify Bot pushed a commit that referenced this pull request Feb 20, 2020
(cherry picked from commit ea8d9d1)

# Conflicts:
#	core/src/cluster_info.rs
#	core/src/crds_value.rs
@pgarg66
Copy link
Copy Markdown
Contributor Author

pgarg66 commented Feb 20, 2020

This needs to be backported to 0.23 since it's an ABI change to Gossip, and thus must be picked up when we restart TdS

@mvines in that case, this should be backported as well. #8276

The current PR is dependent on 8276

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.

3 participants