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

Better logging for notifications #5500

Closed
wants to merge 4 commits into from
Closed

Better logging for notifications #5500

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5465590
Better logging for notifications and buffer size increase
tomaka Apr 2, 2020
8cc37a6
Address review
tomaka Apr 2, 2020
5fbc87a
Merge remote-tracking branch 'upstream/master' into notifs-queue-change
tomaka Apr 2, 2020
1d93d52
Merge branch 'master' into notifs-queue-change
tomaka Apr 3, 2020
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
13 changes: 9 additions & 4 deletions client/network/src/protocol/generic_proto/handler/notif_out.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,13 @@ impl IntoProtocolsHandler for NotifsOutHandlerProto {
DeniedUpgrade
}

fn into_handler(self, _: &PeerId, _: &ConnectedPoint) -> Self::Handler {
fn into_handler(self, peer_id: &PeerId, _: &ConnectedPoint) -> Self::Handler {
NotifsOutHandler {
protocol_name: self.protocol_name,
when_connection_open: Instant::now(),
state: State::Disabled,
events_queue: SmallVec::new(),
peer_id: peer_id.clone(),
}
}
}
Expand Down Expand Up @@ -108,6 +109,9 @@ pub struct NotifsOutHandler {
/// This queue must only ever be modified to insert elements at the back, or remove the first
/// element.
events_queue: SmallVec<[ProtocolsHandlerEvent<NotificationsOut, (), NotifsOutHandlerOut, void::Void>; 16]>,

/// Who we are connected to.
peer_id: PeerId,
}

/// Our relationship with the node we're connected to.
Expand Down Expand Up @@ -300,11 +304,12 @@ impl ProtocolsHandler for NotifsOutHandler {

NotifsOutHandlerIn::Send(msg) =>
if let State::Open { substream, .. } = &mut self.state {
if let Some(Ok(_)) = substream.send(msg).now_or_never() {
} else {
if !matches!(substream.send(msg).now_or_never(), Some(Ok(_))) {
log::warn!(
target: "sub-libp2p",
"📞 Failed to push message to queue, dropped it"
"📞 Queue of notifications with {} is full, dropped message (protocol: {:?})",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"📞 Queue of notifications with {} is full, dropped message (protocol: {:?})",
"📞 Notification queue with peer {} is full, dropped message (protocol: {:?})",

This sounds better to me. Not a native speaker though.

self.peer_id,
self.protocol_name,
);
}
} else {
Expand Down
5 changes: 2 additions & 3 deletions client/network/src/protocol/generic_proto/upgrade/notifications.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ use unsigned_varint::codec::UviBytes;

/// Maximum allowed size of the two handshake messages, in bytes.
const MAX_HANDSHAKE_SIZE: usize = 1024;
/// Maximum number of buffered messages before we consider the remote unresponsive and kill the
/// substream.
const MAX_PENDING_MESSAGES: usize = 256;
/// Maximum number of buffered messages before we refuse to accept more.
const MAX_PENDING_MESSAGES: usize = 1024;

/// Upgrade that accepts a substream, sends back a status message, then becomes a unidirectional
/// stream of messages.
Expand Down