Skip to content

Commit

Permalink
Prefer claiming dead tokens previously owned by the same IP. Begin st…
Browse files Browse the repository at this point in the history 
…arting in replace mode even if assigned token IP's are the same.
  • Loading branch information
@mattl-netflix
mattl-netflix committed Jun 10, 2024
1 parent e93d85e commit 1487921
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 14 deletions.
18 changes: 14 additions & 4 deletions priam/src/main/java/com/netflix/priam/identity/token/TokenRetriever.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,16 @@ public PriamInstance retriableCall() throws Exception {
.filter(i -> !racInstanceIds.contains(i.getInstanceId()))
.collect(Collectors.toList());
Optional<PriamInstance> candidate =
instances.stream().filter(i -> !isNew(i)).findFirst();
instances
.stream()
.filter(i -> !isNew(i))
.reduce(
(current, next) ->
myInstanceInfo
.getPrivateIP()
.equals(next.getHostIP())
? next
: current);
candidate.ifPresent(i -> replacedIp = getReplacedIpForExistingToken(allIds, i));
if (replacedIp == null) {
candidate = instances.stream().filter(i -> isNew(i)).findFirst();
Expand Down Expand Up @@ -216,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: "
Expand Down
7 changes: 2 additions & 5 deletions priam/src/test/java/com/netflix/priam/identity/token/AssignedTokenRetrieverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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
Expand Down
55 changes: 50 additions & 5 deletions priam/src/test/java/com/netflix/priam/identity/token/TokenRetrieverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,25 @@ public void testReplacementGossipMatch(@Mocked SystemUtils systemUtils) throws E
Truth.assertThat(tokenRetriever.getReplacedIp().get()).isEqualTo("127.0.0.3");
}

@Test
public void testPrioritizeDeadTokensWithSameIP(@Mocked SystemUtils systemUtils)
throws Exception {
create(0, "iid_1", "host_0", "127.0.0.1", instanceInfo.getRac(), 0 + "");
create(1, "iid_2", "host_1", "127.1.1.0", instanceInfo.getRac(), 1 + "");
new Expectations() {
{
membership.getRacMembership();
result = ImmutableSet.of();
SystemUtils.getDataFromUrl(anyString);
returns(null, null);
}
};
TokenRetriever tokenRetriever = getTokenRetriever();
Truth.assertThat(tokenRetriever.grabExistingToken().getHostIP()).isEqualTo("127.1.1.0");
Truth.assertThat(tokenRetriever.getReplacedIp().isPresent()).isTrue();
Truth.assertThat(tokenRetriever.getReplacedIp().get()).isEqualTo("127.1.1.0");
}

@Test
public void testPrioritizeDeadTokens(@Mocked SystemUtils systemUtils) throws Exception {
create(0, "iid_0", "host_0", "127.0.0.0", instanceInfo.getRac(), 0 + "");
Expand Down Expand Up @@ -296,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);
Expand All @@ -315,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);
Expand All @@ -326,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() {
Expand All @@ -340,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 {
Expand Down

0 comments on commit 1487921

Please sign in to comment.