Merge pull request #4620 from wubin01/k-bucket-bug-fix

fix(net): solve the problem that K-bucket nodes cannot be deleted
tronprotocol · Sep 1, 2022 · 50b6afd · 50b6afd
2 parents 626e31e + 01a428f
commit 50b6afd
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 64 deletions.
diff --git a/framework/src/main/java/org/tron/common/overlay/discover/table/NodeEntry.java b/framework/src/main/java/org/tron/common/overlay/discover/table/NodeEntry.java
@@ -20,28 +20,14 @@
 
 import org.tron.common.overlay.discover.node.Node;
 
-/**
- * Created by kest on 5/25/15.
- */
 public class NodeEntry {
-
-  private byte[] ownerId;
   private Node node;
   private String entryId;
   private int distance;
   private long modified;
 
-  public NodeEntry(Node n) {
-    this.node = n;
-    this.ownerId = n.getId();
-    entryId = n.getHost();
-    distance = distance(ownerId, n.getId());
-    touch();
-  }
-
   public NodeEntry(byte[] ownerId, Node n) {
     this.node = n;
-    this.ownerId = ownerId;
     entryId = n.getHost();
     distance = distance(ownerId, n.getId());
     touch();
@@ -115,6 +101,6 @@ public boolean equals(Object o) {
 
   @Override
   public int hashCode() {
-    return this.node.hashCode();
+    return this.entryId.hashCode();
   }
 }
diff --git a/framework/src/main/java/org/tron/common/overlay/discover/table/NodeTable.java b/framework/src/main/java/org/tron/common/overlay/discover/table/NodeTable.java
@@ -49,48 +49,37 @@ public final void initialize() {
   }
 
   public synchronized Node addNode(Node n) {
-    NodeEntry e = new NodeEntry(node.getId(), n);
-    if (nodes.contains(e)) {
-      nodes.forEach(nodeEntry -> {
-        if (nodeEntry.equals(e)) {
-          nodeEntry.touch();
-        }
-      });
+    NodeEntry entry = getNodeEntry(n);
+    if (entry != null) {
+      entry.touch();
       return null;
     }
+
+    NodeEntry e = new NodeEntry(node.getId(), n);
     NodeEntry lastSeen = buckets[getBucketId(e)].addNode(e);
     if (lastSeen != null) {
       return lastSeen.getNode();
     }
-    if (!nodes.contains(e)) {
-      nodes.add(e);
-    }
+    nodes.add(e);
     return null;
   }
 
   public synchronized void dropNode(Node n) {
-    NodeEntry e = new NodeEntry(node.getId(), n);
-    buckets[getBucketId(e)].dropNode(e);
-    nodes.remove(e);
+    NodeEntry entry = getNodeEntry(n);
+    if (entry != null) {
+      nodes.remove(entry);
+      buckets[getBucketId(entry)].dropNode(entry);
+    }
   }
 
   public synchronized boolean contains(Node n) {
-    NodeEntry e = new NodeEntry(node.getId(), n);
-    for (NodeBucket b : buckets) {
-      if (b.getNodes().contains(e)) {
-        return true;
-      }
-    }
-    return false;
+    return getNodeEntry(n) != null;
   }
 
   public synchronized void touchNode(Node n) {
-    NodeEntry e = new NodeEntry(node.getId(), n);
-    for (NodeBucket b : buckets) {
-      if (b.getNodes().contains(e)) {
-        b.getNodes().get(b.getNodes().indexOf(e)).touch();
-        break;
-      }
+    NodeEntry entry = getNodeEntry(n);
+    if (entry != null) {
+      entry.touch();
     }
   }
 
@@ -104,10 +93,6 @@ public int getBucketsCount() {
     return i;
   }
 
-  public synchronized NodeBucket[] getBuckets() {
-    return buckets;
-  }
-
   public int getBucketId(NodeEntry e) {
     int id = e.getDistance() - 1;
     return id < 0 ? 0 : id;
@@ -118,17 +103,9 @@ public synchronized int getNodesCount() {
   }
 
   public synchronized List<NodeEntry> getAllNodes() {
-    List<NodeEntry> nodes = new ArrayList<>();
-
-    for (NodeBucket b : buckets) {
-      for (NodeEntry e : b.getNodes()) {
-        if (!e.getNode().equals(node)) {
-          nodes.add(e);
-        }
-      }
-    }
-
-    return nodes;
+    List<NodeEntry> list = new ArrayList<>(nodes);
+    list.remove(new NodeEntry(node.getId(), node));
+    return list;
   }
 
   public synchronized List<Node> getClosestNodes(byte[] targetId) {
@@ -145,4 +122,15 @@ public synchronized List<Node> getClosestNodes(byte[] targetId) {
     }
     return closestNodes;
   }
+
+  private NodeEntry getNodeEntry(Node n) {
+    NodeEntry entry = null;
+    for (NodeEntry e: nodes) {
+      if (e.getNode().getHost().equals(n.getHost())) {
+        entry = e;
+        break;
+      }
+    }
+    return entry;
+  }
 }
diff --git a/framework/src/test/java/org/tron/common/overlay/discover/table/NodeEntryTest.java b/framework/src/test/java/org/tron/common/overlay/discover/table/NodeEntryTest.java
@@ -9,20 +9,16 @@ public class NodeEntryTest {
   @Test
   public void test() throws InterruptedException {
     Node node1 = Node.instanceOf("127.0.0.1:10001");
-    NodeEntry nodeEntry = new NodeEntry(node1);
-    int distance = nodeEntry.getDistance();
-    Assert.assertEquals(-256, distance);
+    NodeEntry nodeEntry = new NodeEntry(Node.getNodeId(), node1);
 
     long lastModified = nodeEntry.getModified();
-    //System.out.println(lastModified);
     Thread.sleep(1);
     nodeEntry.touch();
     long nowModified = nodeEntry.getModified();
-    //System.out.println(nowModified);
     Assert.assertNotEquals(lastModified, nowModified);
 
     Node node2 = Node.instanceOf("127.0.0.1:10002");
-    NodeEntry nodeEntry2 = new NodeEntry(node2);
+    NodeEntry nodeEntry2 = new NodeEntry(Node.getNodeId(), node2);
     boolean isDif = nodeEntry.equals(nodeEntry2);
     Assert.assertTrue(isDif);
   }

diff --git a/framework/src/test/java/org/tron/common/overlay/discover/table/NodeTableTest.java b/framework/src/test/java/org/tron/common/overlay/discover/table/NodeTableTest.java
@@ -133,6 +133,9 @@ public void dropNodeTest() {
     Assert.assertTrue(nodeTable.contains(node));
     nodeTable.dropNode(node);
     Assert.assertTrue(!nodeTable.contains(node));
+    nodeTable.addNode(node);
+    nodeTable.dropNode(new Node(ids.get(1), ips[0], 10000, 10000));
+    Assert.assertTrue(!nodeTable.contains(node));
   }
 
   @Test

diff --git a/framework/src/test/java/org/tron/common/overlay/discover/table/TimeComparatorTest.java b/framework/src/test/java/org/tron/common/overlay/discover/table/TimeComparatorTest.java
@@ -9,10 +9,10 @@ public class TimeComparatorTest {
   @Test
   public void test() throws InterruptedException {
     Node node1 = Node.instanceOf("127.0.0.1:10001");
-    NodeEntry ne1 = new NodeEntry(node1);
+    NodeEntry ne1 = new NodeEntry(Node.getNodeId(), node1);
     Thread.sleep(1);
     Node node2 = Node.instanceOf("127.0.0.1:10002");
-    NodeEntry ne2 = new NodeEntry(node2);
+    NodeEntry ne2 = new NodeEntry(Node.getNodeId(), node2);
     TimeComparator tc = new TimeComparator();
     int result = tc.compare(ne1, ne2);
     Assert.assertEquals(1, result);