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

Skip gossip requests with different shred version #10240

Merged
merged 1 commit into from
May 28, 2020

Conversation

sakridge
Copy link
Contributor

@sakridge sakridge commented May 26, 2020

Problem

Cross-network gossip traffic creating extra work for validators.

Write lock contention on process_pull_requests causes other users on the gossip lock to go slow.

Summary of Changes

Only process requests from peers where shred_version == 0 or shred_version matches our own.

Split process_pull_requests into process_pull_requests and generate_pull_responses. Only proccess_pull_requests needs to take the write lock.

Fixes #

@sakridge sakridge changed the title Skip gossip requests with different shred version than ours Skip gossip requests with different shred version May 26, 2020
@sakridge sakridge force-pushed the shred-version-filter branch from 5d20c3c to b80e768 Compare May 26, 2020 15:45
@codecov
Copy link

codecov bot commented May 26, 2020

Codecov Report

Merging #10240 into master will decrease coverage by 0.0%.
The diff coverage is 89.4%.

@@            Coverage Diff            @@
##           master   #10240     +/-   ##
=========================================
- Coverage    81.5%    81.5%   -0.1%     
=========================================
  Files         288      288             
  Lines       66608    66704     +96     
=========================================
+ Hits        54303    54371     +68     
- Misses      12305    12333     +28     

@sakridge sakridge force-pushed the shred-version-filter branch from b80e768 to 5f1e9de Compare May 26, 2020 20:37
@sakridge sakridge requested a review from carllin May 26, 2020 20:40
@sakridge sakridge force-pushed the shred-version-filter branch from 5f1e9de to 30f23ee Compare May 26, 2020 21:18
if shred_version != 0 && shred_version != my_shred_version {
// Allow someone to update their own ContactInfo if they
// can change shred versions if needed.
crds_values.retain(|crds_value| match &crds_value.data {
Copy link
Contributor

Choose a reason for hiding this comment

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

clippy might yell at you to make this an if let since it's only matching one case.

@sakridge sakridge force-pushed the shred-version-filter branch from 30f23ee to 31092d5 Compare May 26, 2020 21:31
carllin
carllin previously approved these changes May 26, 2020
@carllin
Copy link
Contributor

carllin commented May 26, 2020

I wonder if the filter read lock taking a long time will start affecting validators pushing things like Votes to gossip

@sakridge sakridge force-pushed the shred-version-filter branch from 31092d5 to 01445b4 Compare May 26, 2020 21:36
@mergify mergify bot dismissed carllin’s stale review May 26, 2020 21:36

Pull request has been modified.

@sakridge
Copy link
Contributor Author

I wonder if the filter read lock taking a long time will start affecting validators pushing things like Votes to gossip

Yea, it's definitely not ideal. That one and also other heavy read users like new_pull_requests can also stall write users on the lock.

@sakridge sakridge force-pushed the shred-version-filter branch from 01445b4 to c51051d Compare May 27, 2020 15:16
@sakridge sakridge merged commit 3f508b3 into solana-labs:master May 28, 2020
@sakridge sakridge deleted the shred-version-filter branch May 28, 2020 18:38
@sakridge sakridge added the v1.2 label May 28, 2020
mergify bot pushed a commit that referenced this pull request May 28, 2020
@sakridge sakridge added the v1.1 label Jun 8, 2020
mergify bot pushed a commit that referenced this pull request Jun 8, 2020
sakridge added a commit to sakridge/solana that referenced this pull request Jun 8, 2020
sakridge added a commit that referenced this pull request Jun 9, 2020
* Skip gossip requests with different shred version and split lock (#10240)


(cherry picked from commit 3f508b3)

* More cluster stats and add epoch stakes cache in retransmit stage (#10345)

* More cluster info metrics for push request/response counts

* Cache staked peers for the epoch

(cherry picked from commit ef37b82)

* Cache tvu peers for broadcast (#10373)


(cherry picked from commit 2cf719a)

* Add pull request count metrics (#10421)


(cherry picked from commit 3d2230f)
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