JAVA-3168 Copy node info for contact points on initial node refresh only from first match by endpoint

patch by Alex Sasnouskikh; reviewed by Andy Tolbert and Alexandre Dura for JAVA-3168

Author: jahstreet

core/src/main/java/com/datastax/oss/driver/internal/core/metadata/InitialNodeListRefresh.java

@@ -18,14 +18,16 @@
 package com.datastax.oss.driver.internal.core.metadata;
 
 import com.datastax.oss.driver.api.core.metadata.EndPoint;
-import com.datastax.oss.driver.api.core.metadata.Node;
 import com.datastax.oss.driver.internal.core.context.InternalDriverContext;
 import com.datastax.oss.driver.internal.core.metadata.token.TokenFactory;
 import com.datastax.oss.driver.internal.core.metadata.token.TokenFactoryRegistry;
 import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
 import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList;
 import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
@@ -63,22 +65,31 @@ public Result compute(
     TokenFactory tokenFactory = null;
 
     Map<UUID, DefaultNode> newNodes = new HashMap<>();
+    // Contact point nodes don't have host ID as well as other info yet, so we fill them with node
+    // info found on first match by endpoint
+    Set<EndPoint> matchedContactPoints = new HashSet<>();
+    List<DefaultNode> addedNodes = new ArrayList<>();
 
     for (NodeInfo nodeInfo : nodeInfos) {
       UUID hostId = nodeInfo.getHostId();
       if (newNodes.containsKey(hostId)) {
         LOG.warn(
             "[{}] Found duplicate entries with host_id {} in system.peers, "
-                + "keeping only the first one",
+                + "keeping only the first one {}",
             logPrefix,
-            hostId);
+            hostId,
+            newNodes.get(hostId));
       } else {
         EndPoint endPoint = nodeInfo.getEndPoint();
-        DefaultNode node = findIn(contactPoints, endPoint);
-        if (node == null) {
+        DefaultNode contactPointNode = findContactPointNode(endPoint);
+        DefaultNode node;
+        if (contactPointNode == null || matchedContactPoints.contains(endPoint)) {
           node = new DefaultNode(endPoint, context);
+          addedNodes.add(node);
           LOG.debug("[{}] Adding new node {}", logPrefix, node);
         } else {
+          matchedContactPoints.add(contactPointNode.getEndPoint());
+          node = contactPointNode;
           LOG.debug("[{}] Copying contact point {}", logPrefix, node);
         }
         if (tokenMapEnabled && tokenFactory == null && nodeInfo.getPartitioner() != null) {
@@ -90,14 +101,11 @@ public Result compute(
     }
 
     ImmutableList.Builder<Object> eventsBuilder = ImmutableList.builder();
-
-    for (DefaultNode newNode : newNodes.values()) {
-      if (findIn(contactPoints, newNode.getEndPoint()) == null) {
-        eventsBuilder.add(NodeStateEvent.added(newNode));
-      }
+    for (DefaultNode addedNode : addedNodes) {
+      eventsBuilder.add(NodeStateEvent.added(addedNode));
     }
     for (DefaultNode contactPoint : contactPoints) {
-      if (findIn(newNodes.values(), contactPoint.getEndPoint()) == null) {
+      if (!matchedContactPoints.contains(contactPoint.getEndPoint())) {
         eventsBuilder.add(NodeStateEvent.removed(contactPoint));
       }
     }
@@ -108,10 +116,10 @@ public Result compute(
         eventsBuilder.build());
   }
 
-  private DefaultNode findIn(Iterable<? extends Node> nodes, EndPoint endPoint) {
-    for (Node node : nodes) {
+  private DefaultNode findContactPointNode(EndPoint endPoint) {
+    for (DefaultNode node : contactPoints) {
       if (node.getEndPoint().equals(endPoint)) {
-        return (DefaultNode) node;
+        return node;
       }
     }
     return null;

core/src/test/java/com/datastax/oss/driver/internal/core/metadata/InitialNodeListRefreshTest.java

@@ -48,6 +48,8 @@ public class InitialNodeListRefreshTest {
   private UUID hostId1;
   private UUID hostId2;
   private UUID hostId3;
+  private UUID hostId4;
+  private UUID hostId5;
 
   @Before
   public void setup() {
@@ -61,10 +63,12 @@ public void setup() {
     hostId1 = UUID.randomUUID();
     hostId2 = UUID.randomUUID();
     hostId3 = UUID.randomUUID();
+    hostId4 = UUID.randomUUID();
+    hostId5 = UUID.randomUUID();
   }
 
   @Test
-  public void should_copy_contact_points() {
+  public void should_copy_contact_points_on_first_endpoint_match_only() {
     // Given
     Iterable<NodeInfo> newInfos =
         ImmutableList.of(
@@ -76,6 +80,17 @@ public void should_copy_contact_points() {
             DefaultNodeInfo.builder()
                 .withEndPoint(contactPoint2.getEndPoint())
                 .withHostId(hostId2)
+                .build(),
+            DefaultNodeInfo.builder().withEndPoint(endPoint3).withHostId(hostId3).build(),
+            DefaultNodeInfo.builder()
+                // address translator can translate node addresses to the same endpoints
+                .withEndPoint(contactPoint2.getEndPoint())
+                .withHostId(hostId4)
+                .build(),
+            DefaultNodeInfo.builder()
+                // address translator can translate node addresses to the same endpoints
+                .withEndPoint(endPoint3)
+                .withHostId(hostId5)
                 .build());
     InitialNodeListRefresh refresh =
         new InitialNodeListRefresh(newInfos, ImmutableSet.of(contactPoint1, contactPoint2));
@@ -86,11 +101,26 @@ public void should_copy_contact_points() {
     // Then
     // contact points have been copied to the metadata, and completed with missing information
     Map<UUID, Node> newNodes = result.newMetadata.getNodes();
-    assertThat(newNodes).containsOnlyKeys(hostId1, hostId2);
+    assertThat(newNodes).containsOnlyKeys(hostId1, hostId2, hostId3, hostId4, hostId5);
     assertThat(newNodes.get(hostId1)).isEqualTo(contactPoint1);
     assertThat(contactPoint1.getHostId()).isEqualTo(hostId1);
     assertThat(newNodes.get(hostId2)).isEqualTo(contactPoint2);
     assertThat(contactPoint2.getHostId()).isEqualTo(hostId2);
+    // And
+    // node has been added for the new endpoint
+    assertThat(newNodes.get(hostId3).getEndPoint()).isEqualTo(endPoint3);
+    assertThat(newNodes.get(hostId3).getHostId()).isEqualTo(hostId3);
+    // And
+    // nodes have been added for duplicated endpoints
+    assertThat(newNodes.get(hostId4).getEndPoint()).isEqualTo(contactPoint2.getEndPoint());
+    assertThat(newNodes.get(hostId4).getHostId()).isEqualTo(hostId4);
+    assertThat(newNodes.get(hostId5).getEndPoint()).isEqualTo(endPoint3);
+    assertThat(newNodes.get(hostId5).getHostId()).isEqualTo(hostId5);
+    assertThat(result.events)
+        .containsExactlyInAnyOrder(
+            NodeStateEvent.added((DefaultNode) newNodes.get(hostId3)),
+            NodeStateEvent.added((DefaultNode) newNodes.get(hostId4)),
+            NodeStateEvent.added((DefaultNode) newNodes.get(hostId5)));
   }
 
   @Test