-
Notifications
You must be signed in to change notification settings - Fork 75
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
ETCM-463: Add PeerStatisticsActor to track message counts #849
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aakoshh
force-pushed
the
ETCM-463-track-peer-stats
branch
2 times, most recently
from
December 9, 2020 21:06
06dd621
to
a891426
Compare
aakoshh
force-pushed
the
ETCM-463-track-peer-stats
branch
from
December 9, 2020 21:08
a891426
to
8637bea
Compare
…rray and negative indexes.
aakoshh
changed the title
ETCM-463: Track message stats in PeerManagerActor
ETCM-463: Add PeerStatisticsActor to track message counts
Dec 11, 2020
src/main/scala/io/iohk/ethereum/network/PeerStatisticsActor.scala
Outdated
Show resolved
Hide resolved
src/main/scala/io/iohk/ethereum/network/PeerStatisticsActor.scala
Outdated
Show resolved
Hide resolved
src/main/scala/io/iohk/ethereum/network/PeerStatisticsActor.scala
Outdated
Show resolved
Hide resolved
@lemastero @biandratti @enriquerodbe as you will taking over project soon, I have added you as a reviewers so you will be aware what is happening here |
aakoshh
force-pushed
the
ETCM-463-track-peer-stats
branch
from
December 11, 2020 17:36
d4a6110
to
8876c38
Compare
aakoshh
force-pushed
the
ETCM-463-track-peer-stats
branch
from
December 13, 2020 09:13
0becdfb
to
505dda9
Compare
KonradStaniec
approved these changes
Dec 14, 2020
biandratti
reviewed
Dec 14, 2020
src/test/scala/io/iohk/ethereum/network/TimeSlotStatsSpec.scala
Outdated
Show resolved
Hide resolved
biandratti
reviewed
Dec 14, 2020
src/main/scala/io/iohk/ethereum/network/PeerStatisticsActor.scala
Outdated
Show resolved
Hide resolved
src/main/scala/io/iohk/ethereum/network/PeerStatisticsActor.scala
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Allow #833 to prioritise which peers to prune by counting the number of messages coming from them in a fixed time window.
Proposed Solution
Adds a
TimeSlotStats
counter to thePeerManagerActor
, which now subscribes toMessageFromPeer
events by the event bus and adds 1 to the peer every time we receive a response for a request we sent. Doesn't increment the counter for requests sent by the other side, although that could make sense too, depends on what we consider a valuable connection: one that serves us data, or one that makes its presence known by requesting it.Some of this can be abused though, like
NewBlockHashes
, if it sends the same notification repeatedly, or the notifications about non-existing hashes; these conditions should be monitored on a different level and penalised. I'm guessing that would mean another component would have to publish events to the message bus about such faults, which would be a new event type.The
TimeSlotStats
can be used with any statistic that can be merged, so we can always extend it to include faults, data size; less obvious would be the response time, at least not with this subscription, but perhaps thePeerActor
can send messages to its parent directly instead.The idea is that we can call
peerMessageStats.getAll()
when we want to prune and use it to provide an ordering for which peers to prune first: the ones which we haven't pulled any data from in the last 30 minutes for example.(I added the counter as a
var
to the actor, seemed like there would be quite some noise if I introduce some kind of state wrapper for all the placescontext become ...
occurs, but let me know if that's preferred).