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

Add scalable gossip library #1546

Merged
merged 7 commits into from
Nov 15, 2018
Merged

Conversation

aeyakovenko
Copy link
Member

@aeyakovenko aeyakovenko commented Oct 18, 2018

Implements a scalable gossip protocol.

@aeyakovenko aeyakovenko added work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Oct 18, 2018
@garious
Copy link
Contributor

garious commented Oct 18, 2018

What I see here is 3 independent uses of crdts all thrown into one structure awkwardly named Crdt 10 days after we merged a PR to stop using that term as struct name, #1450.

@aeyakovenko
Copy link
Member Author

@garious its just not type agnostic. This actually implements a crdt with a merge strategy for all the different Value types.

@aeyakovenko
Copy link
Member Author

@garious what do you think about this version? i got rid of the crdt names, and explicitly documented what this replicates

@garious
Copy link
Contributor

garious commented Oct 19, 2018

I don't like the approach of mixed the data type into the data structure and justifying it only by saying that it is cumbersome to do otherwise. To me that suggests you don't fully grok the data structure and that this implementation, like the one in cluster_info.rs, still shoots from the hip. I'd think that as soon as that clean separation existed, this would be implemented in two modules: one with a data structure and that references its relationship is a particular CRDT; one with the data that implements some Replica trait and describes how certain operations commute and so it can be used by a state-based CRDT.

@aeyakovenko
Copy link
Member Author

@garious can you show me an example of a clean heterogeneous type data structure in any language. once you figure that out, you can easily update this structure to that :)

src/lib.rs Outdated Show resolved Hide resolved
@garious garious changed the title Gossip cleanup Add scalable gossip library Nov 3, 2018
@aeyakovenko aeyakovenko removed noCI Suppress CI on this Pull Request work in progress This isn't quite right yet labels Nov 9, 2018
Separate the data storage and merge strategy from the newtwork IO boundary.
Implement an eager push overlay for transporting recent messages.

Simulation shows fast convergance with 20k nodes.
@aeyakovenko aeyakovenko requested review from mvines and pgarg66 and removed request for pgarg66 November 14, 2018 15:15
@aeyakovenko
Copy link
Member Author

@mvines this seems to work! i would like to merge before i get another big set of conflicts.

@mvines mvines requested review from carllin and rob-solana November 14, 2018 15:43
@mvines
Copy link
Member

mvines commented Nov 14, 2018

@rob-solana / @carllin - please review for content too. You guys are more familiar with this part of the code than I

@@ -0,0 +1,378 @@
//! Crds Gossip Pull overlay
//! This module implements the anti-entropy protocol for the newwork.
Copy link
Member

Choose a reason for hiding this comment

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

"newwork"

//!
//! The basic strategy is as follows:
//! 1. Construct a bloom filter of the local data set
//! 2. Randomly ask a node on the network for data that is is not contained in the bloom filter.
Copy link
Member

Choose a reason for hiding this comment

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

"is is"

//! 1. Construct a bloom filter of the local data set
//! 2. Randomly ask a node on the network for data that is is not contained in the bloom filter.
//!
//! Bloom filters have a false positive rate. Each requests uses a differnet bloom filter
Copy link
Member

Choose a reason for hiding this comment

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

"differnet" -- heh, we gotta get you an editor with spelcheck!

/// time when a request to `from` was initiated
/// This is used for weighted random selection durring `new_pull_request`
/// It's important to use the local nodes request creation time as the weight
/// instaad of the response received time otherwise failed nodes will increase their weight.
Copy link
Member

Choose a reason for hiding this comment

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

"instaad"

//!
//! Main differences are:
//! 1. There is no `max hop`. Messages are signed with a local wallclock. If they are outside of
//! the local nodes wallclock window they are droped silently.
Copy link
Member

Choose a reason for hiding this comment

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

"droped"

Copy link
Contributor

@rob-solana rob-solana left a comment

Choose a reason for hiding this comment

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

neat!

@aeyakovenko aeyakovenko merged commit a41254e into solana-labs:master Nov 15, 2018
willhickey pushed a commit that referenced this pull request May 31, 2024
…1506) (#1546)

consensus: make shallow threshold checks log only (#1506)

* consensus: make shallow threshold checks log only

* pr feedback: comment, make check more readable

(cherry picked from commit 6859d65)

Co-authored-by: Ashwin Sekar <[email protected]>
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 1, 2024
…olana-labs#1506) (solana-labs#1546)

consensus: make shallow threshold checks log only (solana-labs#1506)

* consensus: make shallow threshold checks log only

* pr feedback: comment, make check more readable

(cherry picked from commit 6859d65)

Co-authored-by: Ashwin Sekar <[email protected]>
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.

4 participants