Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Split peer slots between full and light nodes #10688

Merged
merged 4 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions client/cli/src/params/network_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,12 @@ pub struct NetworkParams {
#[structopt(long = "out-peers", value_name = "COUNT", default_value = "25")]
pub out_peers: u32,

/// Specify the maximum number of incoming connections we're accepting.
/// Maximum number of inbound full nodes peers.
#[structopt(long = "in-peers", value_name = "COUNT", default_value = "25")]
pub in_peers: u32,
/// Maximum number of inbound light nodes peers.
#[structopt(long = "in-peers-light", value_name = "COUNT", default_value = "100")]
pub in_peers_light: u32,

/// Disable mDNS discovery.
///
Expand Down Expand Up @@ -203,7 +206,7 @@ impl NetworkParams {
boot_nodes,
net_config_path,
default_peers_set: SetConfig {
in_peers: self.in_peers,
in_peers: self.in_peers + self.in_peers_light,
out_peers: self.out_peers,
reserved_nodes: self.reserved_nodes.clone(),
non_reserved_mode: if self.reserved_only {
Expand All @@ -212,6 +215,7 @@ impl NetworkParams {
NonReservedPeerMode::Accept
},
},
default_peers_set_num_full: self.in_peers + self.out_peers,
listen_addresses,
public_addresses,
extra_sets: Vec::new(),
Expand Down
9 changes: 8 additions & 1 deletion client/network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,11 @@ pub struct NetworkConfiguration {
pub request_response_protocols: Vec<RequestResponseConfig>,
/// Configuration for the default set of nodes used for block syncing and transactions.
pub default_peers_set: SetConfig,
/// Number of substreams to reserve for full nodes for block syncing and transactions.
/// Any other slot will be dedicated to light nodes.
///
/// This value is implicitly capped to `default_set.out_peers + default_set.in_peers`.
pub default_peers_set_num_full: u32,
/// Configuration for extra sets of nodes.
pub extra_sets: Vec<NonDefaultSetConfig>,
/// Client identifier. Sent over the wire for debugging purposes.
Expand Down Expand Up @@ -473,14 +478,16 @@ impl NetworkConfiguration {
node_key: NodeKeyConfig,
net_config_path: Option<PathBuf>,
) -> Self {
let default_peers_set = SetConfig::default();
Self {
net_config_path,
listen_addresses: Vec::new(),
public_addresses: Vec::new(),
boot_nodes: Vec::new(),
node_key,
request_response_protocols: Vec::new(),
default_peers_set: Default::default(),
default_peers_set_num_full: default_peers_set.in_peers + default_peers_set.out_peers,
default_peers_set,
extra_sets: Vec::new(),
client_version: client_version.into(),
node_name: node_name.into(),
Expand Down
27 changes: 26 additions & 1 deletion client/network/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,19 @@ pub struct Protocol<B: BlockT> {
pending_messages: VecDeque<CustomMessageOutcome<B>>,
config: ProtocolConfig,
genesis_hash: B::Hash,
/// State machine that handles the list of in-progress requests. Only full node peers are
/// registered.
sync: ChainSync<B>,
// All connected peers
// All connected peers. Contains both full and light node peers.
peers: HashMap<PeerId, Peer<B>>,
chain: Arc<dyn Client<B>>,
/// List of nodes for which we perform additional logging because they are important for the
/// user.
important_peers: HashSet<PeerId>,
/// Value that was passed as part of the configuration. Used to cap the number of full nodes.
default_peers_set_num_full: usize,
/// Number of slots to allocate to light nodes.
default_peers_set_num_light: usize,
/// Used to report reputation changes.
peerset_handle: sc_peerset::PeersetHandle,
/// Handles opening the unique substream and sending and receiving raw messages.
Expand Down Expand Up @@ -428,6 +434,12 @@ impl<B: BlockT> Protocol<B> {
genesis_hash: info.genesis_hash,
sync,
important_peers,
default_peers_set_num_full: network_config.default_peers_set_num_full as usize,
default_peers_set_num_light: {
let total = network_config.default_peers_set.out_peers +
network_config.default_peers_set.in_peers;
total.saturating_sub(network_config.default_peers_set_num_full) as usize
},
peerset_handle: peerset_handle.clone(),
behaviour,
notification_protocols: network_config
Expand Down Expand Up @@ -808,6 +820,19 @@ impl<B: BlockT> Protocol<B> {
}
}

if status.roles.is_full() && self.sync.num_peers() >= self.default_peers_set_num_full {
debug!(target: "sync", "Too many full nodes, rejecting {}", who);
self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
return Err(())
} else if status.roles.is_light() &&
Copy link
Contributor

Choose a reason for hiding this comment

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

This else is redundant, please get rid of it. I love that you used roles.is_light() here, although it is actually always !roles.is_full(), but having it made it easier to read the code. Why are we not worried here about overflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

self.peers contains both full and light peers, but self.sync only contains full peers, so the subtraction should always succeed

(self.peers.len() - self.sync.num_peers()) < self.default_peers_set_num_light
{
// Make sure that not all slots are occupied by light clients.
debug!(target: "sync", "Too many light nodes, rejecting {}", who);
self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
return Err(())
}

let peer = Peer {
info: PeerInfo {
roles: status.roles,
Expand Down
5 changes: 5 additions & 0 deletions client/network/src/protocol/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,11 @@ impl<B: BlockT> ChainSync<B> {
self.downloaded_blocks
}

/// Returns the current number of peers stored within this state machine.
pub fn num_peers(&self) -> usize {
self.peers.len()
}

/// Handle a new connected peer.
///
/// Call this method whenever we connect to a new peer.
Expand Down
3 changes: 1 addition & 2 deletions client/service/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -808,8 +808,7 @@ where
let (handler, protocol_config) = StateRequestHandler::new(
&protocol_id,
client.clone(),
config.network.default_peers_set.in_peers as usize +
config.network.default_peers_set.out_peers as usize,
config.network.default_peers_set_num_full as usize,
);
spawn_handle.spawn("state-request-handler", Some("networking"), handler.run());
protocol_config
Expand Down