Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(networking): get relay number of connections from protocol conns/streams #1609

Merged
merged 4 commits into from
Apr 12, 2023

Conversation

alrevuelta
Copy link
Contributor

@alrevuelta alrevuelta commented Mar 14, 2023

This PR leverages the changes introduced by #1622.
Closes #1566

Summary:

  • The amount of relay connections (inbound and outbound) was not tracked properly, since connections "in general" were counted as relay connections. For example, a node connected to 20 store nodes would report 20 relay connections, but this is not always the case.
  • This PR fixes that, and starts differenciating connections per protocol and per direction (both physical and muxed). This should allow the node to have a better view on the current connections/streams and act accordingly (allowing the peer manager to take decission based on this data)
  • It also introduces waku_streams_peers metric and fixes waku_connected_peers, that should offer more granularity to node operators, providing more info on which protocols are being used by the existing connections.
  • Uses directly nim-libp2p fields exposed in fix: connect instead dial relay peer #1622.

@alrevuelta
Copy link
Contributor Author

cc @Menduist

@alrevuelta alrevuelta changed the title chore(networking): get streams and protos chore(networking): get streams and protos to track protocol conns/streams Mar 17, 2023
@alrevuelta alrevuelta changed the title chore(networking): get streams and protos to track protocol conns/streams chore(networking): get streams and protos to track per protocol conns/streams Mar 17, 2023
@alrevuelta alrevuelta force-pushed the access-protos branch 2 times, most recently from 2fee3fc to d33a7a7 Compare March 31, 2023 14:57
@alrevuelta alrevuelta changed the title chore(networking): get streams and protos to track per protocol conns/streams chore(networking): get relay number of connections from protocol conns/streams Mar 31, 2023
@alrevuelta alrevuelta force-pushed the access-protos branch 4 times, most recently from 29459f2 to 24f28a8 Compare April 11, 2023 09:53
@alrevuelta alrevuelta marked this pull request as ready for review April 11, 2023 10:02
@alrevuelta alrevuelta force-pushed the access-protos branch 2 times, most recently from 0352e21 to 24e40ee Compare April 11, 2023 10:07
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

Cool feature and will be great to have this accurately reported. Change request relates to knowledge of codecs within peer manager, which I think we can avoid if I'm not missing something.

waku/v2/node/peer_manager/peer_manager.nim Outdated Show resolved Hide resolved
waku/v2/node/peer_manager/peer_manager.nim Outdated Show resolved Hide resolved
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

Last request before merging is to move the new utils to /waku/common. We're trying to slowly deprecate the somewhat hotchpotch mix of utils: if a utility is closely related to a specific module e.g. peers, it should be moved closer to the peer modules in future, if it is truly general or common, as I think both of yours are, it should go to /waku/common. Apologies for missing in the first review. Otherwise lgtm.

waku/v2/node/peer_manager/peer_manager.nim Outdated Show resolved Hide resolved
waku/v2/utils/flatten.nim Outdated Show resolved Hide resolved
@alrevuelta alrevuelta merged commit 73cbafa into master Apr 12, 2023
@alrevuelta alrevuelta deleted the access-protos branch April 12, 2023 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore: improve relayConnectivityLoop to make sure in/out conns are relay conns
2 participants