Merge pull request #825 from cthumuluru/3.x

Rolling back the changes to use gossip info while grabbing pre-assigned and dead tokens. Gossip information doesn't seem to reflect the correct state of the cluster always. This is blocking C* nodes from joining the ring. Temporarily rolling back the fix to use Gossip info.
Netflix · Jun 7, 2019 · a3c826f · a3c826f
2 parents 6e0c0b4 + 4794c01
commit a3c826f
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 31 deletions.
diff --git a/priam/src/main/java/com/netflix/priam/identity/token/DeadTokenRetriever.java b/priam/src/main/java/com/netflix/priam/identity/token/DeadTokenRetriever.java
@@ -20,7 +20,6 @@
 import com.netflix.priam.identity.IPriamInstanceFactory;
 import com.netflix.priam.identity.PriamInstance;
 import com.netflix.priam.identity.config.InstanceInfo;
-import com.netflix.priam.identity.token.TokenRetrieverUtils.GossipParseException;
 import com.netflix.priam.utils.Sleeper;
 import java.util.List;
 import java.util.Random;
@@ -112,29 +111,9 @@ public PriamInstance get() throws Exception {
             // remove it as we marked it down...
             factory.delete(priamInstance);
 
-            // find the replaced IP
-            try {
-                replacedIp =
-                        TokenRetrieverUtils.inferTokenOwnerFromGossip(
-                                allInstancesWithinCluster,
-                                priamInstance.getToken(),
-                                priamInstance.getDC());
-
-                // Lets not replace the instance if gossip info is not merging!!
-                if (replacedIp == null) return null;
-                logger.info(
-                        "Will try to replace token: {} with replacedIp (from gossip info): {} instead of ip from Token database: {}",
-                        priamInstance.getToken(),
-                        replacedIp,
-                        priamInstance.getHostIP());
-            } catch (GossipParseException e) {
-                // In case of gossip exception, fallback to IP in token database.
-                this.replacedIp = priamInstance.getHostIP();
-                logger.info(
-                        "Will try to replace token: {} with replacedIp from Token database: {}",
-                        priamInstance.getToken(),
-                        priamInstance.getHostIP());
-            }
+            // use entry in the token database always.
+            // this can cause "can't replace live token errors.
+            this.replacedIp = priamInstance.getHostIP();
 
             PriamInstance result;
             try {

diff --git a/priam/src/main/java/com/netflix/priam/identity/token/TokenRetrieverUtils.java b/priam/src/main/java/com/netflix/priam/identity/token/TokenRetrieverUtils.java
@@ -36,6 +36,12 @@ public class TokenRetrieverUtils {
     public static String inferTokenOwnerFromGossip(
             List<? extends PriamInstance> allIds, String token, String dc)
             throws GossipParseException {
+        // TODO: Gossip info in some cases doesn't reflect the real C* state.
+        // Not using gossip info for now.
+        if (!useGossipInfo()) {
+            return null;
+        }
+
         // Avoid using dead instance who we are trying to replace (duh!!)
         // Avoid other regions instances to avoid communication over public ip address.
         List<? extends PriamInstance> eligibleInstances =
@@ -100,6 +106,12 @@ public static String inferTokenOwnerFromGossip(
         return null;
     }
 
+    // TODO: Gossip info in some cases doesn't reflect the real C* state.
+    // Not using gossip info for now.
+    private static boolean useGossipInfo() {
+        return false;
+    }
+
     // helper method to get the token owner IP from a Cassandra node.
     private static String getIp(String host, String token) throws GossipParseException {
         String response = null;

diff --git a/priam/src/test/java/com/netflix/priam/identity/token/DeadTokenRetrieverTest.java b/priam/src/test/java/com/netflix/priam/identity/token/DeadTokenRetrieverTest.java
@@ -94,7 +94,7 @@ public void testNoReplacementNoSpotAvailable() throws Exception {
         };
     }
 
-    @Test
+    // @Test
     // There is a potential slot for dead token but we are unable to replace.
     public void testNoReplacementNoGossipMatch(@Mocked SystemUtils systemUtils) throws Exception {
         List<PriamInstance> allInstances = getInstances(2);
@@ -150,7 +150,7 @@ public void testNoReplacementNoGossipMatch(@Mocked SystemUtils systemUtils) thro
         };
     }
 
-    @Test
+    // @Test
     public void testReplacementGossipMatch(@Mocked SystemUtils systemUtils) throws Exception {
         List<PriamInstance> allInstances = getInstances(6);
         List<String> racMembership = getRacMembership(2);

diff --git a/priam/src/test/java/com/netflix/priam/identity/token/TokenRetrieverUtilsTest.java b/priam/src/test/java/com/netflix/priam/identity/token/TokenRetrieverUtilsTest.java
@@ -12,7 +12,6 @@
 import junit.framework.Assert;
 import mockit.Expectations;
 import mockit.Mocked;
-import org.junit.Test;
 
 public class TokenRetrieverUtilsTest {
     private static final String APP = "testapp";
@@ -47,7 +46,7 @@ public class TokenRetrieverUtilsTest {
                     .collect(Collectors.toList())
                     .toArray(new String[0]);
 
-    @Test
+    // @Test
     public void testRetrieveTokenOwnerWhenGossipAgrees(@Mocked SystemUtils systemUtils)
             throws Exception {
         // updates instances with new instance owning token 4 as per token database.
@@ -80,7 +79,7 @@ public void testRetrieveTokenOwnerWhenGossipAgrees(@Mocked SystemUtils systemUti
     }
 
     @SuppressWarnings("unchecked")
-    @Test
+    // @Test
     public void testRetrieveTokenOwnerWhenGossipDisagrees(@Mocked SystemUtils systemUtils)
             throws Exception {
         // updates instances with new instance owning token 4 as per token database.
@@ -118,7 +117,7 @@ public void testRetrieveTokenOwnerWhenGossipDisagrees(@Mocked SystemUtils system
         Assert.assertEquals(null, replaceIp);
     }
 
-    @Test
+    // @Test
     public void testRetrieveTokenOwnerWhenAllHostsInGossipReturnsNull(
             @Mocked SystemUtils systemUtils) throws Exception {
         // updates instances with new instance owning token 4 as per token database.
@@ -150,7 +149,7 @@ public void testRetrieveTokenOwnerWhenAllHostsInGossipReturnsNull(
         Assert.assertNull(replaceIp);
     }
 
-    @Test(expected = TokenRetrieverUtils.GossipParseException.class)
+    // @Test(expected = TokenRetrieverUtils.GossipParseException.class)
     public void testRetrieveTokenOwnerWhenAllInstancesThrowGossipParseException(
             @Mocked SystemUtils systemUtils) throws TokenRetrieverUtils.GossipParseException {
         // updates instances with new instance owning token 4 as per token database.