parallelizes gossip packets receiver with processing of requests #17647

behzadnouri · 2021-06-01T22:01:50Z

Problem

Gossip packet processing is composed of two stages:

The first is consuming packets from the socket, deserializing,
sanitizing and verifying them:
https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2510-L2521
The second is actually processing the requests/messages:
https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2585-L2605

The former does not acquire any locks and so can be parallelized with
the later, allowing better pipelineing properties and smaller latency in
responding to gossip requests or propagating messages.

Summary of Changes

Added a new stage to gossip consuming packets received from the socket,
deserializing, sanitizing and verifying them and then sending them down the
channel for the actual handling of requests/messages.

codecov · 2021-06-02T20:20:58Z

Codecov Report

Merging #17647 (a4e6e6b) into master (7f0349b) will decrease coverage by 0.0%.
The diff coverage is 86.8%.

@@            Coverage Diff            @@
##           master   #17647     +/-   ##
=========================================
- Coverage    82.8%    82.7%   -0.1%     
=========================================
  Files         431      431             
  Lines      120590   120629     +39     
=========================================
- Hits        99866    99862      -4     
- Misses      20724    20767     +43

Gossip packet processing is composed of two stages: * The first is consuming packets from the socket, deserializing, sanitizing and verifying them: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2510-L2521 * The second is actually processing the requests/messages: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2585-L2605 The former does not acquire any locks and so can be parallelized with the later, allowing better pipelineing properties and smaller latency in responding to gossip requests or propagating messages.

carllin · 2021-06-05T00:24:09Z

gossip/src/cluster_info.rs

@@ -2659,10 +2683,35 @@ impl ClusterInfo {
        Ok(())
    }

-    pub fn listen(
+    pub(crate) fn consume(


nit: functions in the code base that start threads generally start with start_*_thread, how about start_socket_consume_thread()

carllin · 2021-06-05T00:24:31Z

gossip/src/cluster_info.rs

+    // Consumes packets received from the socket, deserializing, sanitizing and
+    // verifying them and then sending them down the channel for the actual
+    // handling of requests/messages.
+    fn run_consume(


nit: rename to run_socket_consume to be more specific, as "consume" is pretty generic

carllin · 2021-06-05T00:25:34Z

gossip/src/cluster_info.rs

+                }
+            }
+        };
+        let thread_name = String::from("gossip-consume");


nit: "gossip-consume" -> "gossip-socket-consume"

thread names in linux are limited to 15 characters, and everything after the 15th character is trimmed when inspecting running processes from shell, which will make it confusing if I use a longer name here.

carllin · 2021-06-05T00:26:11Z

gossip/src/gossip_service.rs

@@ -56,11 +56,15 @@ impl GossipService {
            1,
        );
        let (response_sender, response_receiver) = channel();
-        let t_responder = streamer::responder("gossip", gossip_socket, response_receiver);
+        let (consume_sender, listen_receiver) = channel();
+        let t_consume =


nit t_consume -> t_socket_consume

carllin · 2021-06-05T00:32:29Z

gossip/src/cluster_info.rs

+                match self.run_consume(&receiver, &sender, &thread_pool) {
+                    Err(GossipError::RecvTimeoutError(RecvTimeoutError::Disconnected)) => break,
+                    Err(GossipError::RecvTimeoutError(RecvTimeoutError::Timeout)) => (),
+                    Err(err) => error!("gossip consume: {}", err),


Nice, the only other potential error other than the Timeout is Ok(sender.send(packets)?), https://github.com/solana-labs/solana/pull/17647/files#diff-b07406ad1c913d21d39a4163cb34f48c55cef6d472c814419a732f3937ffceddR2631. Hopefully this one doesn't have a chance of being too spammy?

yeah, I guess should be fine.

Pull request has been modified.

) Gossip packet processing is composed of two stages: * The first is consuming packets from the socket, deserializing, sanitizing and verifying them: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2510-L2521 * The second is actually processing the requests/messages: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2585-L2605 The former does not acquire any locks and so can be parallelized with the later, allowing better pipelineing properties and smaller latency in responding to gossip requests or propagating messages. (cherry picked from commit cab30e2)

) (#17807) Gossip packet processing is composed of two stages: * The first is consuming packets from the socket, deserializing, sanitizing and verifying them: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2510-L2521 * The second is actually processing the requests/messages: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2585-L2605 The former does not acquire any locks and so can be parallelized with the later, allowing better pipelineing properties and smaller latency in responding to gossip requests or propagating messages. (cherry picked from commit cab30e2) Co-authored-by: behzad nouri <[email protected]>

) Gossip packet processing is composed of two stages: * The first is consuming packets from the socket, deserializing, sanitizing and verifying them: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2510-L2521 * The second is actually processing the requests/messages: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2585-L2605 The former does not acquire any locks and so can be parallelized with the later, allowing better pipelineing properties and smaller latency in responding to gossip requests or propagating messages. (cherry picked from commit cab30e2) # Conflicts: # core/src/cluster_info.rs

…kport #17647) (#19474) * parallelizes gossip packets receiver with processing of requests (#17647) Gossip packet processing is composed of two stages: * The first is consuming packets from the socket, deserializing, sanitizing and verifying them: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2510-L2521 * The second is actually processing the requests/messages: https://github.com/solana-labs/solana/blob/7f0349b29/gossip/src/cluster_info.rs#L2585-L2605 The former does not acquire any locks and so can be parallelized with the later, allowing better pipelineing properties and smaller latency in responding to gossip requests or propagating messages. (cherry picked from commit cab30e2) # Conflicts: # core/src/cluster_info.rs * removes backport merge conflicts Co-authored-by: behzad nouri <[email protected]>

behzadnouri force-pushed the gossip-listen branch 5 times, most recently from 5065a58 to 9d49350 Compare June 2, 2021 18:15

behzadnouri force-pushed the gossip-listen branch from 9d49350 to d53eaa1 Compare June 3, 2021 15:02

behzadnouri changed the title ~~parallelizes gossip listen with packets processing~~ parallelizes gossip packets receiver with processing of requests Jun 3, 2021

behzadnouri requested review from sakridge and carllin June 3, 2021 16:40

carllin reviewed Jun 5, 2021

View reviewed changes

carllin previously approved these changes Jun 5, 2021

View reviewed changes

renames

a4e6e6b

behzadnouri merged commit cab30e2 into solana-labs:master Jun 7, 2021

behzadnouri deleted the gossip-listen branch June 7, 2021 18:36

behzadnouri added the v1.7 label Jun 7, 2021

mergify bot mentioned this pull request Jun 7, 2021

parallelizes gossip packets receiver with processing of requests (backport #17647) #17807

Merged

behzadnouri mentioned this pull request Jul 8, 2021

solana-gossip-responder panic!ed #14074

Closed

brooksprumo mentioned this pull request Aug 23, 2021

backport 19361 v17 #19380

Closed

behzadnouri added the v1.6 label Aug 27, 2021

mergify bot mentioned this pull request Aug 27, 2021

parallelizes gossip packets receiver with processing of requests (backport #17647) #19474

Merged

behzadnouri added v1.6 and removed v1.6 labels Aug 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parallelizes gossip packets receiver with processing of requests #17647

parallelizes gossip packets receiver with processing of requests #17647

behzadnouri commented Jun 1, 2021 •

edited

Loading

codecov bot commented Jun 2, 2021 •

edited

Loading

carllin Jun 5, 2021

behzadnouri Jun 7, 2021

carllin Jun 5, 2021 •

edited

Loading

behzadnouri Jun 7, 2021

carllin Jun 5, 2021

behzadnouri Jun 7, 2021

carllin Jun 5, 2021

behzadnouri Jun 7, 2021

carllin Jun 5, 2021

behzadnouri Jun 7, 2021

parallelizes gossip packets receiver with processing of requests #17647

parallelizes gossip packets receiver with processing of requests #17647

Conversation

behzadnouri commented Jun 1, 2021 • edited Loading

Problem

Summary of Changes

codecov bot commented Jun 2, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carllin Jun 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

behzadnouri commented Jun 1, 2021 •

edited

Loading

codecov bot commented Jun 2, 2021 •

edited

Loading

carllin Jun 5, 2021 •

edited

Loading