From 995bae6fe5b7f227bf3b78656220ad5b7f03c6af Mon Sep 17 00:00:00 2001 From: Jeremy Kong Date: Thu, 12 Oct 2023 11:00:07 +0100 Subject: [PATCH] [PDS-403847] [III] Part 3: Accept Nodes Presenting Plausible-Evolution IDs Even If Not In Quorum (#6768) We now admit new nodes that match the existing topology even in the absence of a quorum of new nodes. --- .../cassandra/CassandraLogHelper.java | 9 ++ .../cassandra/CassandraTopologyValidator.java | 100 ++++++++++-------- .../keyvalue/cassandra/HostIdEvolution.java | 12 +-- .../cassandra/NonSoftFailureHostIdResult.java | 48 +++++++++ .../CassandraTopologyValidatorTest.java | 46 ++++---- .../cassandra/HostIdEvolutionTest.java | 17 +-- .../NonSoftFailureHostIdResultTest.java | 48 +++++++++ changelog/@unreleased/pr-6768.v2.yml | 7 ++ 8 files changed, 208 insertions(+), 79 deletions(-) create mode 100644 atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/NonSoftFailureHostIdResult.java create mode 100644 atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/NonSoftFailureHostIdResultTest.java create mode 100644 changelog/@unreleased/pr-6768.v2.yml diff --git a/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraLogHelper.java b/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraLogHelper.java index 34e9a7144e9..28631a6669f 100644 --- a/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraLogHelper.java +++ b/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraLogHelper.java @@ -28,9 +28,11 @@ import java.util.Collection; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import javax.annotation.Nullable; +import one.util.streamex.EntryStream; import org.apache.cassandra.thrift.TokenRange; import org.immutables.value.Value; @@ -60,6 +62,13 @@ static List tokenRangesToServer(Multimap, CassandraServe .collect(Collectors.toList()); } + static Map idSupportingHostIdResultMap( + Map cassandraServerMap) { + return EntryStream.of(cassandraServerMap) + .mapKeys(CassandraServer::cassandraHostName) + .toMap(); + } + public static List tokenMap(RangeMap> tokenMap) { return tokenMap.asMapOfRanges().entrySet().stream() .map(rangeListToHostEntry -> String.format( diff --git a/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraTopologyValidator.java b/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraTopologyValidator.java index 1b9be732f52..ec9eea6761b 100644 --- a/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraTopologyValidator.java +++ b/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraTopologyValidator.java @@ -46,6 +46,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; import java.util.stream.Collectors; +import java.util.stream.Stream; import one.util.streamex.EntryStream; import org.apache.thrift.TApplicationException; import org.immutables.value.Value; @@ -167,14 +168,14 @@ Set getNewHostsWithInconsistentTopologies( SafeArg.of("newlyAddedHosts", CassandraLogHelper.collectionOfHosts(newlyAddedHostsWithoutOrigin)), SafeArg.of("allHosts", CassandraLogHelper.collectionOfHosts(allHosts.keySet()))); - Map hostIdsByServerWithoutSoftFailures = + Map hostIdsByServerWithoutSoftFailures = fetchHostIdsIgnoringSoftFailures(allHosts); - Map currentServersWithoutSoftFailures = EntryStream.of( + Map currentServersWithoutSoftFailures = EntryStream.of( hostIdsByServerWithoutSoftFailures) .removeKeys(newlyAddedHosts::containsKey) .toMap(); - Map newServersWithoutSoftFailures = EntryStream.of( + Map newServersWithoutSoftFailures = EntryStream.of( hostIdsByServerWithoutSoftFailures) .filterKeys(newlyAddedHosts::containsKey) .toMap(); @@ -185,7 +186,8 @@ Set getNewHostsWithInconsistentTopologies( ClusterTopologyResult topologyResultFromNewServers = maybeGetConsistentClusterTopology(newServersWithoutSoftFailures); Set configuredServersSnapshot = configuredServers.get(); - Map newServersFromConfig = EntryStream.of(newServersWithoutSoftFailures) + Map newServersFromConfig = EntryStream.of( + newServersWithoutSoftFailures) .filterKeys(server -> configuredServersSnapshot.contains(server.cassandraHostName())) .toMap(); return getNewHostsWithInconsistentTopologiesFromTopologyResult( @@ -212,8 +214,8 @@ Set getNewHostsWithInconsistentTopologies( private Set getNewHostsWithInconsistentTopologiesFromTopologyResult( ClusterTopologyResult topologyResult, - Map newServersWithoutSoftFailures, - Map serversToConsiderWhenNoQuorumPresent, + Map newServersWithoutSoftFailures, + Map serversToConsiderWhenNoQuorumPresent, Set newlyAddedHosts, Set allHosts) { switch (topologyResult.type()) { @@ -237,47 +239,49 @@ private Set getNewHostsWithInconsistentTopologiesFromTopologyRe // of what the old servers were thinking, since in containerised deployments all nodes can change // between refreshes for legitimate reasons (but they should still refer to the same underlying // cluster). - if (pastConsistentTopologies.get() == null) { + ConsistentClusterTopologies pastTopologies = pastConsistentTopologies.get(); + if (pastTopologies == null) { // We don't have a record of what worked in the past, and since this state means we're validating // the initial config servers, we don't have another source of truth here. return newServersWithoutSoftFailures.keySet(); } - Optional maybeTopologies = maybeGetConsistentClusterTopology( + Map matchingServers = EntryStream.of( serversToConsiderWhenNoQuorumPresent) - .agreedTopologies(); - if (maybeTopologies.isEmpty()) { - log.info( - "No quorum was detected in original set of servers, and the filtered set of servers were" - + " also not in agreement. Not adding new servers in this case.", - SafeArg.of("newServers", CassandraLogHelper.collectionOfHosts(newlyAddedHosts)), - SafeArg.of("allServers", CassandraLogHelper.collectionOfHosts(allHosts))); - return newServersWithoutSoftFailures.keySet(); - } - ConsistentClusterTopologies newNodesAgreedTopologies = maybeTopologies.get(); - ConsistentClusterTopologies pastTopologySnapshot = pastConsistentTopologies.get(); - if (!pastTopologySnapshot.sharesAtLeastOneHostId(newNodesAgreedTopologies.hostIds())) { + .filterValues(result -> result.type() == HostIdResult.Type.SUCCESS + && pastTopologies.sharesAtLeastOneHostId(result.hostIds())) + .toMap(); + + if (matchingServers.isEmpty()) { log.info( - "No quorum was detected among the original set of servers. While a filtered set of servers" - + " could reach a consensus, this differed from the last agreed value among the old" - + " servers, and is not demonstrably a plausible evolution of the last agreed" - + " topology. Not adding new servers in this case.", - SafeArg.of("pastConsistentTopologies", pastTopologySnapshot), - SafeArg.of("newNodesAgreedTopologies", newNodesAgreedTopologies), + "No quorum was detected in original set of servers, and the filtered set of servers did" + + " not include any servers which presented a plausible evolution of the last agreed" + + " topology. Not adding new servers in this case.", + SafeArg.of("pastConsistentTopologies", pastTopologies), SafeArg.of("newServers", CassandraLogHelper.collectionOfHosts(newlyAddedHosts)), - SafeArg.of("allServers", CassandraLogHelper.collectionOfHosts(allHosts))); + SafeArg.of("allServers", CassandraLogHelper.collectionOfHosts(allHosts)), + SafeArg.of( + "hostIdResults", + CassandraLogHelper.idSupportingHostIdResultMap( + serversToConsiderWhenNoQuorumPresent))); return newServersWithoutSoftFailures.keySet(); } log.info( - "No quorum was detected among the original set of servers. A filtered set of servers reached a" - + " consensus that was a plausible evolution of the last agreed value among the old" - + " servers. Adding new servers that were in consensus.", - SafeArg.of("pastConsistentTopologies", pastTopologySnapshot), - SafeArg.of("newNodesAgreedTopologies", newNodesAgreedTopologies), + "No quorum was detected among the original set of servers. Some servers in a filtered set of" + + " servers presented host IDs that were a plausible evolution of the last agreed value" + + " among the old servers. Adding new servers that were in consensus.", + SafeArg.of("pastConsistentTopologies", pastTopologies), + SafeArg.of( + "hostIdResults", + CassandraLogHelper.idSupportingHostIdResultMap(serversToConsiderWhenNoQuorumPresent)), + SafeArg.of( + "serversMatchingPastTopology", + CassandraLogHelper.idSupportingHostIdResultMap(matchingServers)), SafeArg.of("newServers", CassandraLogHelper.collectionOfHosts(newlyAddedHosts)), SafeArg.of("allServers", CassandraLogHelper.collectionOfHosts(allHosts))); - pastConsistentTopologies.set(pastTopologySnapshot.merge(newNodesAgreedTopologies)); - return Sets.difference( - newServersWithoutSoftFailures.keySet(), newNodesAgreedTopologies.serversInConsensus()); + + ConsistentClusterTopologies mergedTopologies = pastTopologies.merge(matchingServers); + pastConsistentTopologies.set(mergedTopologies); + return Sets.difference(newServersWithoutSoftFailures.keySet(), matchingServers.keySet()); default: throw new SafeIllegalStateException( "Unexpected cluster topology result type", SafeArg.of("type", topologyResult.type())); @@ -296,7 +300,7 @@ private Set getNewHostsWithInconsistentTopologiesFromTopologyRe * @return If consensus could be reached, the topology and the list of valid hosts, otherwise empty. */ private ClusterTopologyResult maybeGetConsistentClusterTopology( - Map hostIdsByServerWithoutSoftFailures) { + Map hostIdsByServerWithoutSoftFailures) { // If all our queries fail due to soft failures, then our consensus is an empty set of host ids if (hostIdsByServerWithoutSoftFailures.isEmpty()) { NodesAndSharedTopology emptySetOfHostIds = NodesAndSharedTopology.builder() @@ -310,7 +314,7 @@ private ClusterTopologyResult maybeGetConsistentClusterTopology( Map> hostIdsWithoutFailures = EntryStream.of(hostIdsByServerWithoutSoftFailures) .filterValues(result -> result.type() == HostIdResult.Type.SUCCESS) - .mapValues(HostIdResult::hostIds) + .mapValues(NonSoftFailureHostIdResult::hostIds) .toMap(); // Only consider hosts that have the endpoint for quorum calculations. @@ -339,7 +343,7 @@ private ClusterTopologyResult maybeGetConsistentClusterTopology( return ClusterTopologyResult.dissent(); } - private Map fetchHostIdsIgnoringSoftFailures( + private Map fetchHostIdsIgnoringSoftFailures( Map servers) { Map results = EntryStream.of(servers).mapValues(this::fetchHostIds).toMap(); @@ -355,6 +359,7 @@ private Map fetchHostIdsIgnoringSoftFailures( return EntryStream.of(results) .removeValues(result -> result.type() == HostIdResult.Type.SOFT_FAILURE) + .mapValues(NonSoftFailureHostIdResult::wrap) .toMap(); } @@ -397,13 +402,22 @@ default boolean sharesAtLeastOneHostId(Set otherHostIds) { return !Sets.intersection(hostIds(), otherHostIds).isEmpty(); } - default ConsistentClusterTopologies merge(ConsistentClusterTopologies other) { + default ConsistentClusterTopologies merge(Map additionalNodes) { + Set topologies = EntryStream.of(additionalNodes) + .mapKeyValue((server, result) -> NodesAndSharedTopology.builder() + .hostIds(result.hostIds()) + .serversInConsensus(Set.of(server)) + .build()) + .collect(Collectors.toSet()); Preconditions.checkArgument( - sharesAtLeastOneHostId(other.hostIds()), + HostIdEvolution.existsPlausibleEvolutionOfHostIdSets( + Stream.of(nodesAndSharedTopologies(), topologies) + .flatMap(Set::stream) + .map(NodesAndSharedTopology::hostIds) + .collect(Collectors.toSet())), "Should not merge topologies that do not share at least one host id."); - return ImmutableConsistentClusterTopologies.builder() - .from(this) - .addAllNodesAndSharedTopologies(other.nodesAndSharedTopologies()) + return ConsistentClusterTopologies.builder() + .nodesAndSharedTopologies(Sets.union(nodesAndSharedTopologies(), topologies)) .build(); } diff --git a/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/HostIdEvolution.java b/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/HostIdEvolution.java index c6cae4ec69c..d4a981a08ad 100644 --- a/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/HostIdEvolution.java +++ b/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/HostIdEvolution.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; +import com.palantir.logsafe.Preconditions; import java.util.HashSet; import java.util.Iterator; import java.util.Set; @@ -32,8 +33,9 @@ private HostIdEvolution() { * Returns true iff there exists a plausible sequence of cluster changes, measured through differences in snapshots * of the host IDs of the cluster, that could have led to the given set of snapshots. Host IDs are generated as * UUIDs: we thus consider that two snapshots of host IDs that contain at least one common element to be plausible - * evolutions of the same cluster, since we assume UUIDs will not collide. A consequence of this is that the - * empty set is not considered to be a plausible evolution of any other host ID set, including the empty set itself. + * evolutions of the same cluster, since we assume UUIDs will not collide. + *

+ * The sets provided are expected to be non-empty; this method will throw if encountering an empty set. *

* Notice that this method may give us false negatives as the cluster may go through more than one transition in * between the snapshots of host IDs we are able to read. However, in the absence of UUID collisions, this method @@ -43,11 +45,7 @@ public static boolean existsPlausibleEvolutionOfHostIdSets(Set> sets if (sets.isEmpty()) { return true; } - if (sets.contains(ImmutableSet.of())) { - // If present, this will not have a nonempty intersection with any other set, so *not* all sets would be - // connected by non-empty intersections. - return false; - } + Preconditions.checkArgument(!sets.contains(ImmutableSet.of()), "Empty sets of host ids are not allowed"); Set> remainingUnconnectedSets = new HashSet<>(sets); diff --git a/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/NonSoftFailureHostIdResult.java b/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/NonSoftFailureHostIdResult.java new file mode 100644 index 00000000000..51a2f29b175 --- /dev/null +++ b/atlasdb-cassandra/src/main/java/com/palantir/atlasdb/keyvalue/cassandra/NonSoftFailureHostIdResult.java @@ -0,0 +1,48 @@ +/* + * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.atlasdb.keyvalue.cassandra; + +import com.palantir.logsafe.Preconditions; +import java.util.Set; +import org.immutables.value.Value; + +@Value.Immutable +public interface NonSoftFailureHostIdResult { + @Value.Parameter + HostIdResult result(); + + @Value.Derived + default HostIdResult.Type type() { + return result().type(); + } + + @Value.Derived + default Set hostIds() { + return result().hostIds(); + } + + static NonSoftFailureHostIdResult wrap(HostIdResult result) { + return ImmutableNonSoftFailureHostIdResult.of(result); + } + + @Value.Check + default void check() { + Preconditions.checkArgument( + type() != HostIdResult.Type.SOFT_FAILURE, + "Soft failures are not allowed in a NonSoftFailureHostIdResult."); + } +} diff --git a/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraTopologyValidatorTest.java b/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraTopologyValidatorTest.java index 8fbb2ba448c..759dcc3b87d 100644 --- a/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraTopologyValidatorTest.java +++ b/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/CassandraTopologyValidatorTest.java @@ -367,7 +367,7 @@ public void validateNewlyAddedHostsNewHostsAddedIfTheyAgreeWithOldHostsOnPreviou } @Test - public void returnsAllNewHostsIfNoConsensusOnNewHostsWhenNoQuorumAndCurrentServersExist() { + public void acceptsNewHostsIfNoConsensusOnNewHostsWhenNoQuorumAndCurrentServersExist() { Iterator uuidIterator = UUIDS.iterator(); Map allHosts = initialiseHosts(ALL_HOSTS); Map oldHosts = @@ -389,8 +389,8 @@ public void returnsAllNewHostsIfNoConsensusOnNewHostsWhenNoQuorumAndCurrentServe assertThat(hostsOffline).hasSizeGreaterThanOrEqualTo((ALL_HOSTS.size() + 1) / 2); assertThat(validator.getNewHostsWithInconsistentTopologies( mapToTokenRangeOrigin(newCassandraServers), allHosts)) - .as("rejects all servers when no quorum from all hosts and no quorum on new hosts") - .containsExactlyInAnyOrderElementsOf(newCassandraServers); + .as("accepts servers with plausible evolution when no quorum from all hosts and no quorum on new hosts") + .isEmpty(); } @Test @@ -514,7 +514,7 @@ public void validateNewlyAddedHostsAddedIfAgreedWithOldTopologyWhenNoQuorumAndNo } @Test - public void returnsAllNewHostsIfNoConsensusOnConfigHostsWhenNoQuorumAndNoCurrentServers() { + public void acceptsNewHostsIfNoConsensusOnConfigHostsWhenNoQuorumAndNoCurrentServers() { Iterator uuidIterator = UUIDS.iterator(); Map allHosts = initialiseHosts(ALL_HOSTS); Map oldHosts = @@ -537,8 +537,9 @@ public void returnsAllNewHostsIfNoConsensusOnConfigHostsWhenNoQuorumAndNoCurrent splitHostOriginBetweenLastKnownAndConfig( oldCassandraServers, Sets.difference(allCassandraServers, oldCassandraServers)), allHosts)) - .as("rejects all servers when no quorum from all hosts and no quorum on available hosts") - .containsExactlyInAnyOrderElementsOf(allCassandraServers); + .as("accepts new servers with a plausible evolution when no quorum from all hosts and no quorum on" + + " available hosts") + .containsExactlyInAnyOrderElementsOf(oldCassandraServers); } @Test @@ -744,12 +745,11 @@ public void mergedConsistentClusterTopologyShouldHaveCorrectIdsAndServers() { .addHostIds("one", "two", "three") .build()) .build(); - ConsistentClusterTopologies newTopologies = original.merge(ConsistentClusterTopologies.builder() - .addNodesAndSharedTopologies(NodesAndSharedTopology.builder() - .addServersInConsensus(createCassandraServer("banana"), createCassandraServer("cherry")) - .addHostIds("one", "four", "five") - .build()) - .build()); + ConsistentClusterTopologies newTopologies = original.merge(ImmutableMap.of( + createCassandraServer("banana"), + NonSoftFailureHostIdResult.wrap(HostIdResult.success(ImmutableSet.of("one", "four"))), + createCassandraServer("cherry"), + NonSoftFailureHostIdResult.wrap(HostIdResult.success(ImmutableSet.of("one", "five"))))); assertThat(newTopologies.serversInConsensus()) .isEqualTo(Sets.union(original.serversInConsensus(), newTopologies.serversInConsensus())); @@ -765,19 +765,19 @@ public void throwsIfMergingConsistentClusterTopologiesThatDoNotOverlap() { .build()) .build(); - assertThatThrownBy(() -> existing.merge(ConsistentClusterTopologies.builder() - .addNodesAndSharedTopologies(NodesAndSharedTopology.builder() - .addServersInConsensus(createCassandraServer("banana")) - .addHostIds("four", "five", "six") - .build()) - .build())) + assertThatThrownBy(() -> existing.merge(ImmutableMap.of( + createCassandraServer("banana"), + NonSoftFailureHostIdResult.wrap(HostIdResult.success(ImmutableSet.of("one", "four"))), + createCassandraServer("cherry"), + NonSoftFailureHostIdResult.wrap(HostIdResult.success(ImmutableSet.of("five", "six")))))) .isInstanceOf(SafeIllegalArgumentException.class) .hasMessage("Should not merge topologies that do not share at least one host id."); - assertThatThrownBy(() -> existing.merge(ConsistentClusterTopologies.builder() - .addNodesAndSharedTopologies(NodesAndSharedTopology.builder() - .addServersInConsensus(createCassandraServer("carrot")) - .build()) - .build())) + + assertThatThrownBy(() -> existing.merge(ImmutableMap.of( + createCassandraServer("banana"), + NonSoftFailureHostIdResult.wrap(HostIdResult.success(ImmutableSet.of("four", "five"))), + createCassandraServer("cherry"), + NonSoftFailureHostIdResult.wrap(HostIdResult.success(ImmutableSet.of("five", "six")))))) .isInstanceOf(SafeIllegalArgumentException.class) .hasMessage("Should not merge topologies that do not share at least one host id."); } diff --git a/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/HostIdEvolutionTest.java b/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/HostIdEvolutionTest.java index 292ca4d9f31..c6789747e77 100644 --- a/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/HostIdEvolutionTest.java +++ b/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/HostIdEvolutionTest.java @@ -17,8 +17,10 @@ package com.palantir.atlasdb.keyvalue.cassandra; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.google.common.collect.ImmutableSet; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import org.junit.Test; public class HostIdEvolutionTest { @@ -39,9 +41,15 @@ public void singleNonemptySetConnectedToItselfByNonemptyIntersections() { } @Test - public void singleEmptySetNotConnectedToItselfByNonemptyIntersections() { - assertThat(HostIdEvolution.existsPlausibleEvolutionOfHostIdSets(ImmutableSet.of(ImmutableSet.of()))) - .isFalse(); + public void throwsIfEmptySetsProvided() { + assertThatThrownBy( + () -> HostIdEvolution.existsPlausibleEvolutionOfHostIdSets(ImmutableSet.of(ImmutableSet.of()))) + .isInstanceOf(SafeIllegalArgumentException.class) + .hasMessage("Empty sets of host ids are not allowed"); + assertThatThrownBy(() -> HostIdEvolution.existsPlausibleEvolutionOfHostIdSets( + ImmutableSet.of(ImmutableSet.of("samphire", "taro", "ume"), ImmutableSet.of()))) + .isInstanceOf(SafeIllegalArgumentException.class) + .hasMessage("Empty sets of host ids are not allowed"); } @Test @@ -60,9 +68,6 @@ public void twoDisjointSetsNotConnectedByNonemptyIntersections() { assertThat(HostIdEvolution.existsPlausibleEvolutionOfHostIdSets(ImmutableSet.of( ImmutableSet.of("nectarine", "orange", "pear"), ImmutableSet.of("quince", "rhubarb")))) .isFalse(); - assertThat(HostIdEvolution.existsPlausibleEvolutionOfHostIdSets( - ImmutableSet.of(ImmutableSet.of("samphire", "taro", "ume"), ImmutableSet.of()))) - .isFalse(); } @Test diff --git a/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/NonSoftFailureHostIdResultTest.java b/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/NonSoftFailureHostIdResultTest.java new file mode 100644 index 00000000000..8cbf8656f1d --- /dev/null +++ b/atlasdb-cassandra/src/test/java/com/palantir/atlasdb/keyvalue/cassandra/NonSoftFailureHostIdResultTest.java @@ -0,0 +1,48 @@ +/* + * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.atlasdb.keyvalue.cassandra; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; + +import com.google.common.collect.ImmutableList; +import org.junit.Test; + +public class NonSoftFailureHostIdResultTest { + @Test + public void throwsWhenCreatingWithSoftFailure() { + assertThatThrownBy(() -> NonSoftFailureHostIdResult.wrap(HostIdResult.softFailure())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Soft failures are not allowed in a NonSoftFailureHostIdResult."); + } + + @Test + public void canCreateWithSuccessAndRetrieveOriginal() { + HostIdResult success = HostIdResult.success(ImmutableList.of("alice", "bob")); + NonSoftFailureHostIdResult wrappedSuccess = NonSoftFailureHostIdResult.wrap(success); + + assertThat(wrappedSuccess.result()).isEqualTo(success); + } + + @Test + public void canCreateWithHardFailureAndRetrieveOriginal() { + HostIdResult hardFailure = HostIdResult.hardFailure(); + NonSoftFailureHostIdResult wrappedSuccess = NonSoftFailureHostIdResult.wrap(hardFailure); + + assertThat(wrappedSuccess.result()).isEqualTo(hardFailure); + } +} diff --git a/changelog/@unreleased/pr-6768.v2.yml b/changelog/@unreleased/pr-6768.v2.yml new file mode 100644 index 00000000000..fcbba28fa55 --- /dev/null +++ b/changelog/@unreleased/pr-6768.v2.yml @@ -0,0 +1,7 @@ +type: fix +fix: + description: Nodes presenting a plausible evolution of the previously agreed cluster topology can now be accepted + during a `NO_QUORUM` state, even if there is no overall quorum among new nodes. Previously, a quorum among the + new nodes was required which was unnecessarily restrictive. + links: + - https://github.com/palantir/atlasdb/pull/6768