Prefer claiming dead tokens previously owned by the same IP. Begin st…

…arting in replace mode even if assigned token IP's are the same. (#1099)
Netflix · Jun 11, 2024 · 5754b4f · 5754b4f
1 parent 479ae9e
commit 5754b4f
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 13 deletions.
diff --git a/priam/src/main/java/com/netflix/priam/identity/token/TokenRetriever.java b/priam/src/main/java/com/netflix/priam/identity/token/TokenRetriever.java
@@ -225,12 +225,13 @@ private String getReplacedIpForAssignedToken(
                 == TokenRetrieverUtils.InferredTokenOwnership.TokenInformationStatus.GOOD) {
             Preconditions.checkNotNull(inferredTokenOwnership.getTokenInformation());
             String inferredIp = inferredTokenOwnership.getTokenInformation().getIpAddress();
-            if (!inferredIp.equals(myInstanceInfo.getHostIP())
-                    && !inferredIp.equals(myInstanceInfo.getPrivateIP())) {
-                if (inferredTokenOwnership.getTokenInformation().isLive()) {
+            if (inferredTokenOwnership.getTokenInformation().isLive()) {
+                if (!inferredIp.equals(myInstanceInfo.getHostIP())
+                        && !inferredIp.equals(myInstanceInfo.getPrivateIP())) {
                     throw new TokenRetrieverUtils.GossipParseException(
                             "We have been assigned a token that C* thinks is alive. Throwing to buy time in the hopes that Gossip just needs to settle.");
                 }
+            } else {
                 ipToReplace = inferredIp;
                 logger.info(
                         "Priam found that the token is not alive according to Cassandra and we should start Cassandra in replace mode with replace ip: "

diff --git a/priam/src/test/java/com/netflix/priam/identity/token/AssignedTokenRetrieverTest.java b/priam/src/test/java/com/netflix/priam/identity/token/AssignedTokenRetrieverTest.java
@@ -26,7 +26,7 @@ public class AssignedTokenRetrieverTest {
     public static final String DEAD_APP = "testapp-dead";
 
     @Test
-    public void grabAssignedTokenStartDbInBootstrapModeWhenGossipAgreesCurrentInstanceIsTokenOwner(
+    public void grabAssignedTokenStartDbInReplaceModeWhenGossipAgreesCurrentIPIsOwnerAndNotLive(
             @Mocked IPriamInstanceFactory factory,
             @Mocked IConfiguration config,
             @Mocked IMembership membership,
@@ -59,9 +59,6 @@ public void grabAssignedTokenStartDbInBootstrapModeWhenGossipAgreesCurrentInstan
                 instanceInfo.getInstanceId();
                 result = liveHosts.get(0).getInstanceId();
 
-                instanceInfo.getHostIP();
-                result = liveHosts.get(0).getHostIP();
-
                 TokenRetrieverUtils.inferTokenOwnerFromGossip(
                         ImmutableSet.copyOf(liveHosts),
                         liveHosts.get(0).getToken(),
@@ -75,7 +72,7 @@ public void grabAssignedTokenStartDbInBootstrapModeWhenGossipAgreesCurrentInstan
                         factory, membership, config, instanceInfo, sleeper, tokenManager);
         InstanceIdentity instanceIdentity =
                 new InstanceIdentity(factory, membership, config, instanceInfo, tokenRetriever);
-        Truth.assertThat(instanceIdentity.isReplace()).isFalse();
+        Truth.assertThat(instanceIdentity.isReplace()).isTrue();
     }
 
     @Test

diff --git a/priam/src/test/java/com/netflix/priam/identity/token/TokenRetrieverTest.java b/priam/src/test/java/com/netflix/priam/identity/token/TokenRetrieverTest.java
@@ -315,8 +315,8 @@ public void testNewTokenGenerationMultipleInstancesWithLargetEnoughIds() throws
     }
 
     @Test
-    public void testPreassignedTokenNotReplacedIfPublicIPMatch(@Mocked SystemUtils systemUtils)
-            throws Exception {
+    public void testPreassignedTokenNotReplacedIfLiveAndPublicIPMatch(
+            @Mocked SystemUtils systemUtils) throws Exception {
         // IP in DB doesn't matter so we make it different to confirm that
         create(0, instanceInfo.getInstanceId(), "host_0", "1.2.3.4", "az1", 0 + "");
         getInstances(5);
@@ -334,8 +334,8 @@ public void testPreassignedTokenNotReplacedIfPublicIPMatch(@Mocked SystemUtils s
     }
 
     @Test
-    public void testPreassignedTokenNotReplacedIfPrivateIPMatch(@Mocked SystemUtils systemUtils)
-            throws Exception {
+    public void testPreassignedTokenNotReplacedIfLiveAndPrivateIPMatch(
+            @Mocked SystemUtils systemUtils) throws Exception {
         // IP in DB doesn't matter so we make it different to confirm that
         create(0, instanceInfo.getInstanceId(), "host_0", "1.2.3.4", "az1", 0 + "");
         getInstances(5);
@@ -345,7 +345,7 @@ public void testPreassignedTokenNotReplacedIfPrivateIPMatch(@Mocked SystemUtils
                         .collect(
                                 Collectors.toMap(
                                         String::valueOf, e -> String.format("127.1.1.%s", e)));
-        ImmutableList<String> myLiveInstances = ImmutableList.copyOf(tokenToEndpointMap.values());
+        ImmutableList<String> myLiveInstances = ImmutableList.copyOf(myTokenToEndpointMap.values());
         String gossipResponse = getStatus(myLiveInstances, myTokenToEndpointMap);
 
         new Expectations() {
@@ -359,6 +359,32 @@ public void testPreassignedTokenNotReplacedIfPrivateIPMatch(@Mocked SystemUtils
         Truth.assertThat(tokenRetriever.getReplacedIp().isPresent()).isFalse();
     }
 
+    @Test
+    public void testPreassignedTokenReplacedIfNotLive(@Mocked SystemUtils systemUtils)
+            throws Exception {
+        // IP in DB doesn't matter so we make it different to confirm that
+        create(0, instanceInfo.getInstanceId(), "host_0", "1.2.3.4", "az1", 0 + "");
+        getInstances(5);
+        List<String> tokens =
+                IntStream.range(0, 7).boxed().map(String::valueOf).collect(Collectors.toList());
+        Map<String, String> myTokenToEndpointMap =
+                tokens.stream()
+                        .collect(Collectors.toMap(e -> e, e -> String.format("127.1.1.%s", e)));
+        ImmutableList<String> myLiveInstances =
+                ImmutableList.copyOf(tokens.stream().skip(1).collect(Collectors.toList()));
+        String gossipResponse = getStatus(myLiveInstances, myTokenToEndpointMap);
+
+        new Expectations() {
+            {
+                SystemUtils.getDataFromUrl(anyString);
+                returns(gossipResponse, gossipResponse, null, "random_value", gossipResponse);
+            }
+        };
+        TokenRetriever tokenRetriever = getTokenRetriever();
+        tokenRetriever.get();
+        Truth.assertThat(tokenRetriever.getReplacedIp().isPresent()).isTrue();
+    }
+
     @Test
     public void testGetPreassignedTokenThrowsIfOwnerIPIsLive(@Mocked SystemUtils systemUtils)
             throws Exception {