Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

[refer #393] Support periodic random connection eviction #394

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

knizhnik
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Dec 18, 2020

Coverage Status

Coverage remained the same at 46.984% when pulling 90d55c1 on i393 into 30f3bea on master.

Copy link
Contributor

@kushti kushti left a comment

Choose a reason for hiding this comment

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

Looks good in general, but I need to test it (also, consider writing tests for the code). Please do PR description (and correct title maybe , as PR is not just about eviction). Also, please do comments, left some notes on that.

@@ -149,15 +149,24 @@ object PeerManager {
sc: ScorexContext): Map[InetSocketAddress, PeerInfo] = knownPeers
}

private def getIpGroup(addr : InetSocketAddress) : Int = {
val ip = addr.getAddress.getAddress
val group = ((ip(0) & 0xFF) << 8) | (ip(1) & 0xFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please comment non-trivial logic like this

val group = ((ip(0) & 0xFF) << 8) | (ip(1) & 0xFF)
group
}

case class RandomPeerExcluding(excludedPeers: Seq[PeerInfo]) extends GetPeers[Option[PeerInfo]] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do a ScalaDoc describing implemented strategy details

knizhnik added a commit that referenced this pull request Jan 22, 2021
[refer #394] Try to choose candidates from different groups

Signed-off-by: Konstantin Knizhnik <[email protected]>
Base automatically changed from master to main February 3, 2021 23:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants