JAVA-3005: Refresh entire node list when new node added (#1604)

akhaku · web-flow · commit ffcf0d560a58 · 2022-07-13T14:09:38.000-05:00
Empirically, Cassandra does not appear to send TopologyChangeType.REMOVED_NODE when a node in the ring crashes hard and is eventually replaced, even though the bad node is removed from the system tables. This results in the driver holding on to that node until a restart. The 3.x driver refreshes the entire node list when a node is added, so when a new node comes up to replace the crashed node, the crashed node is removed from the driver's state. With this change we port that behavior to the 4.x driver and refresh the entire node list when a new node is added. This also fixes JAVA-3002.
diff --git a/changelog/README.md b/changelog/README.md
@@ -2,6 +2,10 @@
 
 <!-- Note: contrary to 3.x, insert new entries *first* in their section -->
 
+### 4.14.2 (in progress)
+
+- [bug] JAVA-3002 JAVA-3005: Refresh entire node list when a new node is added
+
 ### 4.14.1
 
 - [improvement] JAVA-3013: Upgrade dependencies to address CVEs and other security issues, 4.14.1 edition
diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/NodeStateManager.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/NodeStateManager.java
@@ -185,10 +185,10 @@ private void onDebouncedTopologyEvent(TopologyEvent event) {
             }
           } else {
             LOG.debug(
-                "[{}] Received UP event for unknown node {}, adding it",
+                "[{}] Received UP event for unknown node {}, refreshing node list",
                 logPrefix,
                 event.broadcastRpcAddress);
-            metadataManager.addNode(event.broadcastRpcAddress);
+            metadataManager.refreshNodes();
           }
           break;
         case SUGGEST_DOWN:
diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/NodeStateManagerTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/NodeStateManagerTest.java
@@ -147,7 +147,7 @@ public void should_apply_up_event_if_node_is_unknown_or_down() {
   }
 
   @Test
-  public void should_add_node_if_up_event_and_not_in_metadata() {
+  public void should_refresh_node_list_if_up_event_and_not_in_metadata() {
     // Given
     new NodeStateManager(context);
 
@@ -157,7 +157,7 @@ public void should_add_node_if_up_event_and_not_in_metadata() {
 
     // Then
     verify(eventBus, never()).fire(any(NodeStateEvent.class));
-    verify(metadataManager).addNode(NEW_ADDRESS);
+    verify(metadataManager).refreshNodes();
   }
 
   @Test

Original file line number	Diff line number	Diff line change
`@@ -185,10 +185,10 @@ private void onDebouncedTopologyEvent(TopologyEvent event) {`
`185`	`185`	`}`
`186`	`186`	`} else {`
`187`	`187`	`LOG.debug(`
`188`		`- "[{}] Received UP event for unknown node {}, adding it",`
	`188`	`+ "[{}] Received UP event for unknown node {}, refreshing node list",`
`189`	`189`	`logPrefix,`
`190`	`190`	`event.broadcastRpcAddress);`
`191`		`- metadataManager.addNode(event.broadcastRpcAddress);`
	`191`	`+ metadataManager.refreshNodes();`
`192`	`192`	`}`
`193`	`193`	`break;`
`194`	`194`	`case SUGGEST_DOWN:`
Original file line number	Diff line number	Diff line change
`@@ -147,7 +147,7 @@ public void should_apply_up_event_if_node_is_unknown_or_down() {`
`147`	`147`	`}`
`148`	`148`
`149`	`149`	`@Test`
`150`		`- public void should_add_node_if_up_event_and_not_in_metadata() {`
	`150`	`+ public void should_refresh_node_list_if_up_event_and_not_in_metadata() {`
`151`	`151`	`// Given`
`152`	`152`	`new NodeStateManager(context);`
`153`	`153`
`@@ -157,7 +157,7 @@ public void should_add_node_if_up_event_and_not_in_metadata() {`
`157`	`157`
`158`	`158`	`// Then`
`159`	`159`	`verify(eventBus, never()).fire(any(NodeStateEvent.class));`
`160`		`- verify(metadataManager).addNode(NEW_ADDRESS);`
	`160`	`+ verify(metadataManager).refreshNodes();`
`161`	`161`	`}`
`162`	`162`
`163`	`163`	`@Test`