Skip to content

Commit 403ec00

Browse files
lexnvbkchr
andauthored
notifications: Exit protocols on handle drop to save up CPU of minimal-relay-chains (#376)
This PR ensures that notification tasks are fully terminated when the associated handle is dropped, addressing an issue specific to `minimal-relay-chains`. We have operated from the assumption that the handle is never supposed to get dropped, since protocols cannot be dynamically injected after the backend is started. This PR effectively reduces the CPU usage of minimal-relay-chains from 7% to 0.1%. We have noticed that the CPU consumption of minimal-relay-chains has increased compared to libp2p backends. However, our metrics for CPU consumption of full relay chains have drastically improved. The issue is the following: - substrate needs a block-announces notification config to initiate the network components. - the minimal-relay-chains do not need this notification around, and drop the associated handle. - this behaviour causes a CPU loop that continuously spams the following log line: ``` litep2p::notification: [Parachain] user protocol has exited, exiting protocol=/b0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe/block-announces/1 ``` CPU before and after: ![Screenshot 2025-04-17 at 17 44 23](https://github.com/user-attachments/assets/34a91cd4-7354-498a-ae99-b77d7b103f5b) Fixes: paritytech/polkadot-sdk#7998 --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Bastian Köcher <[email protected]>
1 parent 5f07325 commit 403ec00

File tree

2 files changed

+33
-5
lines changed

2 files changed

+33
-5
lines changed

src/protocol/notification/mod.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,7 +1606,9 @@ impl NotificationProtocol {
16061606
}
16071607

16081608
/// Handle next notification event.
1609-
async fn next_event(&mut self) {
1609+
///
1610+
/// Returns `true` when the user command stream was dropped.
1611+
async fn next_event(&mut self) -> bool {
16101612
// biased select is used because the substream events must be prioritized above other events
16111613
// that is because a closed substream is detected by either `substreams` or `negotiation`
16121614
// and if that event is not handled with priority but, e.g., inbound substream is
@@ -1783,9 +1785,16 @@ impl NotificationProtocol {
17831785
);
17841786
}
17851787
}
1788+
1789+
// User commands.
17861790
command = self.command_rx.recv() => match command {
17871791
None => {
1788-
tracing::debug!(target: LOG_TARGET, "user protocol has exited, exiting");
1792+
tracing::debug!(
1793+
target: LOG_TARGET,
1794+
protocol = %self.protocol,
1795+
"user protocol has exited, exiting"
1796+
);
1797+
return true;
17891798
}
17901799
Some(command) => match command {
17911800
NotificationCommand::OpenSubstream { peers } => {
@@ -1811,14 +1820,14 @@ impl NotificationProtocol {
18111820
}
18121821
},
18131822
}
1823+
1824+
false
18141825
}
18151826

18161827
/// Start [`NotificationProtocol`] event loop.
18171828
pub(crate) async fn run(mut self) {
18181829
tracing::debug!(target: LOG_TARGET, "starting notification event loop");
18191830

1820-
loop {
1821-
self.next_event().await;
1822-
}
1831+
while !self.next_event().await {}
18231832
}
18241833
}

src/protocol/notification/tests/notification.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,3 +1117,22 @@ async fn second_inbound_substream_opened_while_outbound_substream_was_opening()
11171117
state => panic!("invalid state for peer: {state:?}"),
11181118
}
11191119
}
1120+
1121+
#[tokio::test]
1122+
async fn drop_handle_exits_protocol() {
1123+
let _ = tracing_subscriber::fmt()
1124+
.with_env_filter(tracing_subscriber::EnvFilter::from_default_env())
1125+
.try_init();
1126+
1127+
let (mut protocol, handle, _sender, _tx) = make_notification_protocol();
1128+
1129+
// Simulate a handle drop.
1130+
drop(handle);
1131+
1132+
// Call `next_event` and ensure it returns true.
1133+
let result = protocol.next_event().await;
1134+
assert!(
1135+
result,
1136+
"Expected `next_event` to return true when `command_rx` is dropped"
1137+
);
1138+
}

0 commit comments

Comments
 (0)