From 5754b4f24d171ee5545c5bdabc19768fdf2eb43c Mon Sep 17 00:00:00 2001 From: mattl-netflix <63665634+mattl-netflix@users.noreply.github.com> Date: Tue, 11 Jun 2024 10:32:46 -0700 Subject: [PATCH] Prefer claiming dead tokens previously owned by the same IP. Begin starting in replace mode even if assigned token IP's are the same. (#1099) --- .../priam/identity/token/TokenRetriever.java | 7 ++-- .../token/AssignedTokenRetrieverTest.java | 7 ++-- .../identity/token/TokenRetrieverTest.java | 36 ++++++++++++++++--- 3 files changed, 37 insertions(+), 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 index 5a2b4fe7c..e368a1a4b 100644 --- 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 index 94c02cb79..2853a0a2a 100644 --- 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 index 700dc05c8..3072c1d64 100644 --- 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 myLiveInstances = ImmutableList.copyOf(tokenToEndpointMap.values()); + ImmutableList 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 tokens = + IntStream.range(0, 7).boxed().map(String::valueOf).collect(Collectors.toList()); + Map myTokenToEndpointMap = + tokens.stream() + .collect(Collectors.toMap(e -> e, e -> String.format("127.1.1.%s", e))); + ImmutableList 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 {