-
Notifications
You must be signed in to change notification settings - Fork 834
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
Peering - disconnect peers that have multiple discovery ports #7089
Conversation
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]>
very cool find @pinges - are there more tests you wanted to add? |
11 holesky nodes - 10 of them get to 100% peers within 2h the 11th one eventually gets to 100% peers (although it does take ~7h) Current behaviour on holesky is > 90% of nodes get stuck at 0 peers indefinitely (require setting static peers to sync) See #6805 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs a changelog entry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just needs a changelog entry
private final LHMWithMaxSize<String, Integer> ipAddressCheckMap = | ||
new LHMWithMaxSize<>(DEFAULT_BUCKET_SIZE * N_BUCKETS); | ||
private final CircularFifoQueue<String> invalidIPs = | ||
new CircularFifoQueue<>(DEFAULT_BUCKET_SIZE * N_BUCKETS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious - why these specific collection types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CircularFifo is a simple queue with a maximum size.
LHMWithMaxSize is a private class, it is a LinkedHashMap that I have extended to have a maximum size.
Lukas complained about the name of the class and I will rename it :-)
Signed-off-by: Sally MacFarlane <[email protected]>
@@ -51,6 +54,10 @@ public class PeerTable { | |||
private final Map<Bytes, Integer> distanceCache; | |||
private BloomFilter<Bytes> idBloom; | |||
private int evictionCnt = 0; | |||
private final LHMWithMaxSize<String, Integer> ipAddressCheckMap = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should rename LHMWithMaxSize
as LinkedHashMapWithMaxSize
to be clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do :-)
Signed-off-by: [email protected] <[email protected]>
…me IP and TCP port with different discovery ports (hyperledger#7089) Find and remove peers from the peer table that share the same IP and TCP port with different discovery ports --------- Signed-off-by: [email protected] <[email protected]> Signed-off-by: Sally MacFarlane <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Justin Florentine <[email protected]>
…me IP and TCP port with different discovery ports (hyperledger#7089) Find and remove peers from the peer table that share the same IP and TCP port with different discovery ports --------- Signed-off-by: [email protected] <[email protected]> Signed-off-by: Sally MacFarlane <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]>
…me IP and TCP port with different discovery ports (hyperledger#7089) Find and remove peers from the peer table that share the same IP and TCP port with different discovery ports --------- Signed-off-by: [email protected] <[email protected]> Signed-off-by: Sally MacFarlane <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Justin Florentine <[email protected]>
PR description
In discovery we seemed to see a lot of "misconfigured" nodes that share the same IP address and TCP port, but use different discovery ports. This PR checks for these nodes and removes them from the peer table.
Fixes #6805