vacp2p / nim-libp2p

libp2p implementation in Nim
https://vacp2p.github.io/nim-libp2p/docs/
MIT License
242 stars 52 forks source link

feat: behaviour penalty when non-priority queue reaches maxNumElementsInNonPriorityQueue #1083

Open diegomrsantos opened 2 months ago

diegomrsantos commented 2 months ago

This PR applies a behavior penalty to peers whose non-prio queue reaches the max limit configured, instead of the previous strategy of disconnecting the peer. A conservative penalty of 0.0001 is added to behaviourPenalty for each message tried to be sent when the queue is over the limit, and the message is discarded. This usually results in a behaviourPenalty around [0.1, 0.2] when the score is updated and its value around [-0.4, -0.1].

It causes the peer to be pruned due to its negative score. This PR also clears the non-prio queue at this moment.

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 84.60%. Comparing base (03f67d3) to head (a6237bd). Report is 2 commits behind head on unstable.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/vacp2p/nim-libp2p/pull/1083/graphs/tree.svg?width=650&height=150&src=pr&token=M88zHaQffJ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vacp2p)](https://app.codecov.io/gh/vacp2p/nim-libp2p/pull/1083?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vacp2p) ```diff @@ Coverage Diff @@ ## unstable #1083 +/- ## ============================================ - Coverage 84.82% 84.60% -0.23% ============================================ Files 91 91 Lines 15428 15486 +58 ============================================ + Hits 13087 13102 +15 - Misses 2341 2384 +43 ``` | [Files](https://app.codecov.io/gh/vacp2p/nim-libp2p/pull/1083?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vacp2p) | Coverage Δ | | |---|---|---| | [libp2p/protocols/pubsub/gossipsub/behavior.nim](https://app.codecov.io/gh/vacp2p/nim-libp2p/pull/1083?src=pr&el=tree&filepath=libp2p%2Fprotocols%2Fpubsub%2Fgossipsub%2Fbehavior.nim&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vacp2p#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvZ29zc2lwc3ViL2JlaGF2aW9yLm5pbQ==) | `88.86% <0.00%> (+0.02%)` | :arrow_up: | | [libp2p/protocols/pubsub/pubsubpeer.nim](https://app.codecov.io/gh/vacp2p/nim-libp2p/pull/1083?src=pr&el=tree&filepath=libp2p%2Fprotocols%2Fpubsub%2Fpubsubpeer.nim&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vacp2p#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvcHVic3VicGVlci5uaW0=) | `87.59% <70.00%> (+1.45%)` | :arrow_up: | ... and [9 files with indirect coverage changes](https://app.codecov.io/gh/vacp2p/nim-libp2p/pull/1083/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vacp2p)
diegomrsantos commented 2 months ago

This log shows the moment when the peer 16U23VggQ non-prio queue reaches maxNumElementsInNonPriorityQueue (currently 1024) at 2024-04-10 15:35:48.273+02:00. At this moment, a behavior penalty of 0.0001 is applied. It keeps increasing by the same amount until it reaches 0.08230000000000133. At 2024-04-10 15:35:57.562+02:00, the peer score becomes negative (-0.1079571840000035). The peer is pruned and the queue is cleared. At 2024-04-10 16:05:57.471 the peer score becomes 0 again. After that, something similar happens again, but this time, after the peer score becomes negative, it's disconnected `DBG 2024-04-10 16:15:14.749+02:00 Received Goodbye message topics="peer_proto" reason="Disconnected (129)" peer=16U23VggQ`. The agent was prysm and I don't know if it's different from teku, but in the latter, this error means too many peers. nimbus-log.txt