From 3378431bb94c76c01ebf0d327d4f47c1e7bc2cfe Mon Sep 17 00:00:00 2001 From: Dmitry Kropachev Date: Mon, 12 Jan 2026 22:43:32 -0400 Subject: [PATCH] Switch all Node API to use List instead of Set Set - doesn't guarantee the order. When driver works with Nodes, say read replicas from TokenMap, Tablets, calculate replicas, API user should expect that order does matter and preserved. For that we need to switch all these APIs and internal structures to use Lists --- .../oss/driver/api/core/metadata/Tablet.java | 18 +++ .../driver/api/core/metadata/TokenMap.java | 152 +++++++++++++++++- .../BasicLoadBalancingPolicy.java | 26 +-- .../DefaultLoadBalancingPolicy.java | 4 +- .../core/metadata/DefaultTabletMap.java | 41 +++-- .../token/CanonicalNodeListBuilder.java | 56 +++++++ .../core/metadata/token/DefaultTokenMap.java | 12 +- .../token/EverywhereReplicationStrategy.java | 15 +- .../core/metadata/token/KeyspaceTokenMap.java | 15 +- .../token/LocalReplicationStrategy.java | 9 +- .../NetworkTopologyReplicationStrategy.java | 6 +- .../metadata/token/ReplicationStrategy.java | 29 +++- .../token/SimpleReplicationStrategy.java | 7 +- ...asicLoadBalancingPolicyDcFailoverTest.java | 8 +- ...BalancingPolicyPreferredRemoteDcsTest.java | 15 +- ...BasicLoadBalancingPolicyQueryPlanTest.java | 41 ++--- ...faultLoadBalancingPolicyQueryPlanTest.java | 45 +++--- .../core/metadata/DefaultTabletMapTest.java | 25 ++- .../metadata/token/DefaultTokenMapTest.java | 52 +++--- ...etworkTopologyReplicationStrategyTest.java | 37 +++-- .../token/SimpleReplicationStrategyTest.java | 16 +- .../basic/TokenMapAndShardIdLookup.java | 8 +- .../DefaultLoadBalancingPolicyIT.java | 4 +- .../loadbalancing/LWTLoadBalancingIT.java | 6 +- .../oss/driver/core/metadata/SchemaIT.java | 2 +- .../oss/driver/core/metadata/TokenITBase.java | 4 +- pom.xml | 2 +- 27 files changed, 469 insertions(+), 186 deletions(-) create mode 100644 core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/CanonicalNodeListBuilder.java diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/metadata/Tablet.java b/core/src/main/java/com/datastax/oss/driver/api/core/metadata/Tablet.java index f279927cf34..d1bc6f1af02 100644 --- a/core/src/main/java/com/datastax/oss/driver/api/core/metadata/Tablet.java +++ b/core/src/main/java/com/datastax/oss/driver/api/core/metadata/Tablet.java @@ -1,6 +1,7 @@ package com.datastax.oss.driver.api.core.metadata; import com.datastax.oss.driver.shaded.guava.common.annotations.Beta; +import java.util.List; import java.util.Set; /** @@ -25,8 +26,25 @@ public interface Tablet extends Comparable { */ public long getLastToken(); + /** + * Returns replica nodes in the order reported by the tablets-routing-v1 payload. + * + *

This set is immutable + * + * @deprecated Use {@link #getReplicaNodesList()} instead. + */ + @Deprecated public Set getReplicaNodes(); + /** + * Returns replica nodes in the order reported by the tablets-routing-v1 payload. + * + *

This list is immutable. + * + * @return ordered list of replica nodes for this tablet + */ + public List getReplicaNodesList(); + /** * Looks up the shard number for specific replica Node. * diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/metadata/TokenMap.java b/core/src/main/java/com/datastax/oss/driver/api/core/metadata/TokenMap.java index dcc73961098..031a4258eee 100644 --- a/core/src/main/java/com/datastax/oss/driver/api/core/metadata/TokenMap.java +++ b/core/src/main/java/com/datastax/oss/driver/api/core/metadata/TokenMap.java @@ -31,10 +31,12 @@ import com.datastax.oss.driver.api.core.metadata.token.TokenRange; import com.datastax.oss.driver.api.core.session.Session; import com.datastax.oss.driver.api.core.type.codec.TypeCodec; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; import java.nio.ByteBuffer; +import java.util.List; import java.util.Set; /** @@ -114,9 +116,30 @@ default Set getTokenRanges(@NonNull String keyspaceName, @NonNull No return getTokenRanges(CqlIdentifier.fromCql(keyspaceName), replica); } - /** The replicas for a given partition key in the given keyspace. */ + /** + * The replicas for a given partition key in the given keyspace. + * + * @deprecated Use {@link #getReplicasList(CqlIdentifier, Partitioner, ByteBuffer)} instead. + */ + @NonNull + @Deprecated + default Set getReplicas( + @NonNull CqlIdentifier keyspace, + @Nullable Partitioner partitioner, + @NonNull ByteBuffer partitionKey) { + return ImmutableSet.copyOf(getReplicasList(keyspace, partitioner, partitionKey)); + } + + /** + * The replicas for a given partition key in the given keyspace, in ring order. + * + * @param keyspace the keyspace for which to get replicas + * @param partitioner the partitioner to use or {@code null} for this TokenMap's partitioner + * @param partitionKey the partition key in its serialized form + * @return ordered list of replica nodes for the given partition key + */ @NonNull - Set getReplicas( + List getReplicasList( @NonNull CqlIdentifier keyspace, @Nullable Partitioner partitioner, @NonNull ByteBuffer partitionKey); @@ -124,17 +147,37 @@ Set getReplicas( /** * Shortcut for {@link #getReplicas(CqlIdentifier, Partitioner, ByteBuffer) getReplicas(keyspace, * null, partitionKey)}. + * + * @deprecated Use {@link #getReplicasList(CqlIdentifier, ByteBuffer)} instead. */ @NonNull + @Deprecated default Set getReplicas(@NonNull CqlIdentifier keyspace, @NonNull ByteBuffer partitionKey) { return getReplicas(keyspace, null, partitionKey); } + /** + * Shortcut for {@link #getReplicasList(CqlIdentifier, Partitioner, ByteBuffer) + * getReplicasList(keyspace, null, partitionKey)}. + * + * @param keyspace the keyspace for which to get replicas + * @param partitionKey the partition key in its serialized form + * @return ordered list of replica nodes for the given partition key + */ + @NonNull + default List getReplicasList( + @NonNull CqlIdentifier keyspace, @NonNull ByteBuffer partitionKey) { + return getReplicasList(keyspace, null, partitionKey); + } + /** * Shortcut for {@link #getReplicas(CqlIdentifier, Partitioner, ByteBuffer) * getReplicas(CqlIdentifier.fromCql(keyspaceName), partitioner, partitionKey)}. + * + * @deprecated Use {@link #getReplicasList(String, Partitioner, ByteBuffer)} instead. */ @NonNull + @Deprecated default Set getReplicas( @NonNull String keyspaceName, @Nullable Partitioner partitioner, @@ -142,28 +185,95 @@ default Set getReplicas( return getReplicas(CqlIdentifier.fromCql(keyspaceName), partitioner, partitionKey); } + /** + * Shortcut for {@link #getReplicasList(CqlIdentifier, Partitioner, ByteBuffer) + * getReplicasList(CqlIdentifier.fromCql(keyspaceName), partitioner, partitionKey)}. + * + * @param keyspaceName the keyspace name for which to get replicas + * @param partitioner the partitioner to use or {@code null} for this TokenMap's partitioner + * @param partitionKey the partition key in its serialized form + * @return ordered list of replica nodes for the given partition key + */ + @NonNull + default List getReplicasList( + @NonNull String keyspaceName, + @Nullable Partitioner partitioner, + @NonNull ByteBuffer partitionKey) { + return getReplicasList(CqlIdentifier.fromCql(keyspaceName), partitioner, partitionKey); + } + /** * Shortcut for {@link #getReplicas(CqlIdentifier, Partitioner, ByteBuffer) * getReplicas(CqlIdentifier.fromCql(keyspaceName), null, partitionKey)}. + * + * @deprecated Use {@link #getReplicasList(String, ByteBuffer)} instead. */ @NonNull + @Deprecated default Set getReplicas(@NonNull String keyspaceName, @NonNull ByteBuffer partitionKey) { return getReplicas(CqlIdentifier.fromCql(keyspaceName), null, partitionKey); } - /** The replicas for a given token in the given keyspace. */ + /** + * Shortcut for {@link #getReplicasList(CqlIdentifier, Partitioner, ByteBuffer) + * getReplicasList(CqlIdentifier.fromCql(keyspaceName), null, partitionKey)}. + * + * @param keyspaceName the keyspace name for which to get replicas + * @param partitionKey the partition key in its serialized form + * @return ordered list of replica nodes for the given partition key + */ + @NonNull + default List getReplicasList( + @NonNull String keyspaceName, @NonNull ByteBuffer partitionKey) { + return getReplicasList(CqlIdentifier.fromCql(keyspaceName), null, partitionKey); + } + + /** + * The replicas for a given token in the given keyspace. + * + * @deprecated Use {@link #getReplicasList(CqlIdentifier, Token)} instead. + */ @NonNull + @Deprecated Set getReplicas(@NonNull CqlIdentifier keyspace, @NonNull Token token); + /** + * The replicas for a given token in the given keyspace, in ring order. + * + * @param keyspace the keyspace for which to get replicas + * @param token the token for which to get replicas + * @return ordered list of replica nodes for the given token + */ + @NonNull + default List getReplicasList(@NonNull CqlIdentifier keyspace, @NonNull Token token) { + return ImmutableList.copyOf(getReplicas(keyspace, token)); + } + /** * Shortcut for {@link #getReplicas(CqlIdentifier, Token) * getReplicas(CqlIdentifier.fromCql(keyspaceName), token)}. + * + * @deprecated Use {@link #getReplicasList(String, Token)} instead. */ @NonNull + @Deprecated default Set getReplicas(@NonNull String keyspaceName, @NonNull Token token) { return getReplicas(CqlIdentifier.fromCql(keyspaceName), token); } + /** + * Shortcut for {@link #getReplicasList(CqlIdentifier, Token) + * getReplicasList(CqlIdentifier.fromCql(keyspaceName), token)}. + * + * @param keyspaceName the keyspace name for which to get replicas + * @param token the token for which to get replicas + * @return ordered list of replica nodes for the given token + */ + @NonNull + default List getReplicasList(@NonNull String keyspaceName, @NonNull Token token) { + return getReplicasList(CqlIdentifier.fromCql(keyspaceName), token); + } + /** * The replicas for a given range in the given keyspace. * @@ -171,21 +281,57 @@ default Set getReplicas(@NonNull String keyspaceName, @NonNull Token token * range extends over multiple nodes, it only returns the nodes that are replicas for the last * token of the range. In other words, this method is a shortcut for {@code getReplicas(keyspace, * range.getEnd())}. + * + * @deprecated Use {@link #getReplicasList(CqlIdentifier, TokenRange)} instead. */ @NonNull + @Deprecated default Set getReplicas(@NonNull CqlIdentifier keyspace, @NonNull TokenRange range) { return getReplicas(keyspace, range.getEnd()); } + /** + * The replicas for a given range in the given keyspace, in ring order. + * + *

It is assumed that the input range does not overlap across multiple node ranges. If the + * range extends over multiple nodes, it only returns the nodes that are replicas for the last + * token of the range. In other words, this method is a shortcut for {@code + * getReplicasList(keyspace, range.getEnd())}. + * + * @param keyspace the keyspace for which to get replicas + * @param range the token range for which to get replicas + * @return ordered list of replica nodes for the given token range + */ + @NonNull + default List getReplicasList(@NonNull CqlIdentifier keyspace, @NonNull TokenRange range) { + return getReplicasList(keyspace, range.getEnd()); + } + /** * Shortcut for {@link #getReplicas(CqlIdentifier, TokenRange) * getReplicas(CqlIdentifier.fromCql(keyspaceName), range)}. + * + * @deprecated Use {@link #getReplicasList(String, TokenRange)} instead. */ @NonNull + @Deprecated default Set getReplicas(@NonNull String keyspaceName, @NonNull TokenRange range) { return getReplicas(CqlIdentifier.fromCql(keyspaceName), range); } + /** + * Shortcut for {@link #getReplicasList(CqlIdentifier, TokenRange) + * getReplicasList(CqlIdentifier.fromCql(keyspaceName), range)}. + * + * @param keyspaceName the keyspace name for which to get replicas + * @param range the token range for which to get replicas + * @return ordered list of replica nodes for the given token range + */ + @NonNull + default List getReplicasList(@NonNull String keyspaceName, @NonNull TokenRange range) { + return getReplicasList(CqlIdentifier.fromCql(keyspaceName), range); + } + /** The name of the partitioner class in use, as reported by the Cassandra nodes. */ @NonNull String getPartitionerName(); diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java b/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java index 3d1694cd2c3..3ce0f7d08d2 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicy.java @@ -57,12 +57,12 @@ import com.datastax.oss.driver.internal.core.util.collection.QueryPlan; import com.datastax.oss.driver.internal.core.util.collection.SimpleQueryPlan; import com.datastax.oss.driver.shaded.guava.common.base.Predicates; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import com.datastax.oss.driver.shaded.guava.common.collect.Lists; import com.datastax.oss.driver.shaded.guava.common.collect.Sets; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; import java.nio.ByteBuffer; -import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -263,7 +263,7 @@ public Queue newQueryPlan(@Nullable Request request, @Nullable Session ses // Take a snapshot since the set is concurrent: Object[] currentNodes = liveNodes.dc(localDc).toArray(); - Set allReplicas = getReplicas(request, session); + List allReplicas = getReplicas(request, session); int replicaCount = 0; // in currentNodes if (!allReplicas.isEmpty()) { @@ -295,9 +295,9 @@ public Queue newQueryPlan(@Nullable Request request, @Nullable Session ses } @NonNull - protected Set getReplicas(@Nullable Request request, @Nullable Session session) { + protected List getReplicas(@Nullable Request request, @Nullable Session session) { if (request == null || session == null) { - return Collections.emptySet(); + return ImmutableList.of(); } Optional maybeTokenMap = context.getMetadataManager().getMetadata().getTokenMap(); @@ -321,7 +321,7 @@ protected Set getReplicas(@Nullable Request request, @Nullable Session ses keyspace = session.getKeyspace().get(); } if (keyspace == null) { - return Collections.emptySet(); + return ImmutableList.of(); } table = request.getRoutingTable(); @@ -329,7 +329,7 @@ protected Set getReplicas(@Nullable Request request, @Nullable Session ses token = request.getRoutingToken(); key = (token == null) ? request.getRoutingKey() : null; if (token == null && key == null) { - return Collections.emptySet(); + return ImmutableList.of(); } partitioner = request.getPartitioner(); @@ -339,7 +339,7 @@ protected Set getReplicas(@Nullable Request request, @Nullable Session ses } catch (Exception e) { // Protect against poorly-implemented Request instances LOG.error("Unexpected error while trying to compute query plan", e); - return Collections.emptySet(); + return ImmutableList.of(); } if (token == null && partitioner != null) { @@ -350,25 +350,25 @@ protected Set getReplicas(@Nullable Request request, @Nullable Session ses context.getMetadataManager().getMetadata().getKeyspace(keyspace); if (ksMetadata.isPresent() && ksMetadata.get().isUsingTablets() && maybeTabletMap.isPresent()) { if (table == null) { - return Collections.emptySet(); + return ImmutableList.of(); } if (token instanceof TokenLong64) { Tablet targetTablet = maybeTabletMap.get().getTablet(keyspace, table, ((TokenLong64) token).getValue()); if (targetTablet != null) { - return targetTablet.getReplicaNodes(); + return targetTablet.getReplicaNodesList(); } } - return Collections.emptySet(); + return ImmutableList.of(); } if (!maybeTokenMap.isPresent()) { - return Collections.emptySet(); + return ImmutableList.of(); } TokenMap tokenMap = maybeTokenMap.get(); return token != null - ? tokenMap.getReplicas(keyspace, token) - : tokenMap.getReplicas(keyspace, partitioner, key); + ? tokenMap.getReplicasList(keyspace, token) + : tokenMap.getReplicasList(keyspace, partitioner, key); } @NonNull diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicy.java b/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicy.java index 469e54c56fb..0b78141227a 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicy.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicy.java @@ -38,12 +38,12 @@ import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; import java.util.BitSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.OptionalLong; import java.util.Queue; -import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -138,7 +138,7 @@ public Queue newQueryPlan(@Nullable Request request, @Nullable Session ses // Take a snapshot since the set is concurrent: Object[] currentNodes = getLiveNodes().dc(getLocalDatacenter()).toArray(); - Set allReplicas = getReplicas(request, session); + List allReplicas = getReplicas(request, session); int replicaCount = 0; // in currentNodes int localRackReplicaCount = 0; // in currentNodes String localRack = getLocalRack(); diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTabletMap.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTabletMap.java index f349df7b98a..c6467a62ca8 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTabletMap.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTabletMap.java @@ -8,7 +8,9 @@ import com.datastax.oss.driver.api.core.metadata.TabletMap; import com.datastax.oss.driver.shaded.guava.common.annotations.Beta; import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import edu.umd.cs.findbugs.annotations.NonNull; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -130,12 +132,7 @@ public void addTablet(CqlIdentifier keyspace, CqlIdentifier table, Tablet tablet @Override public void removeByNode(Node node) { for (ConcurrentSkipListSet tabletSet : mapping.values()) { - Iterator it = tabletSet.iterator(); - while (it.hasNext()) { - if (it.next().getReplicaNodes().contains(node)) { - it.remove(); - } - } + tabletSet.removeIf(tablet -> tablet.getReplicaNodesList().contains(node)); } } @@ -162,18 +159,18 @@ public void removeByTable(CqlIdentifier table) { public static class DefaultTablet implements Tablet { private final long firstToken; private final long lastToken; - @NonNull private final Set replicaNodes; + @NonNull private final ImmutableList replicaNodes; @NonNull private final Map replicaShards; @VisibleForTesting DefaultTablet( long firstToken, long lastToken, - @NonNull Set replicaNodes, + @NonNull List replicaNodes, @NonNull Map replicaShards) { this.firstToken = firstToken; this.lastToken = lastToken; - this.replicaNodes = replicaNodes; + this.replicaNodes = ImmutableList.copyOf(replicaNodes); this.replicaShards = replicaShards; } @@ -189,7 +186,7 @@ public static DefaultTablet parseTabletPayloadV1(TupleValue tupleValue, Map replicaNodes = new HashSet<>(); + List replicaNodes = new ArrayList<>(); Map replicaShards = new HashMap<>(); List list = tupleValue.getList(2, TupleValue.class); assert list != null; @@ -197,7 +194,9 @@ public static DefaultTablet parseTabletPayloadV1(TupleValue tupleValue, Map getReplicaNodes() { + return new HashSet<>(replicaNodes); + } + + /** + * Return replicas in the tablet. + * + * @return a list of replicas in the tablet + */ + @Override + public List getReplicaNodesList() { return replicaNodes; } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/CanonicalNodeListBuilder.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/CanonicalNodeListBuilder.java new file mode 100644 index 00000000000..197e031eab4 --- /dev/null +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/CanonicalNodeListBuilder.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.datastax.oss.driver.internal.core.metadata.token; + +import com.datastax.oss.driver.api.core.metadata.Node; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import net.jcip.annotations.NotThreadSafe; + +/** + * A reusable set builder that guarantees that identical sets (same elements in the same order) will + * be represented by the same instance. + */ +@NotThreadSafe +class CanonicalNodeListBuilder { + + private final Map, List> canonicalLists = new HashMap<>(); + private final List elements = new ArrayList<>(); + + void add(Node node) { + // This is O(n), but the cardinality is low (max possible size is the replication factor). + if (!elements.contains(node)) { + elements.add(node); + } + } + + int size() { + return elements.size(); + } + + List build() { + return canonicalLists.computeIfAbsent(elements, ImmutableList::copyOf); + } + + void clear() { + elements.clear(); + } +} diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/DefaultTokenMap.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/DefaultTokenMap.java index b9f752a7bca..571123cfdf0 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/DefaultTokenMap.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/DefaultTokenMap.java @@ -184,21 +184,27 @@ public Set getTokenRanges(@NonNull CqlIdentifier keyspace, @NonNull @NonNull @Override - public Set getReplicas( + public List getReplicasList( @NonNull CqlIdentifier keyspace, @Nullable Partitioner partitioner, @NonNull ByteBuffer partitionKey) { KeyspaceTokenMap keyspaceMap = getKeyspaceMap(keyspace); return (keyspaceMap == null) - ? Collections.emptySet() + ? ImmutableList.of() : keyspaceMap.getReplicas(partitioner, partitionKey); } @NonNull @Override public Set getReplicas(@NonNull CqlIdentifier keyspace, @NonNull Token token) { + return ImmutableSet.copyOf(getReplicasList(keyspace, token)); + } + + @NonNull + @Override + public List getReplicasList(@NonNull CqlIdentifier keyspace, @NonNull Token token) { KeyspaceTokenMap keyspaceMap = getKeyspaceMap(keyspace); - return (keyspaceMap == null) ? Collections.emptySet() : keyspaceMap.getReplicas(token); + return (keyspaceMap == null) ? ImmutableList.of() : keyspaceMap.getReplicas(token); } @NonNull diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/EverywhereReplicationStrategy.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/EverywhereReplicationStrategy.java index 1973c07f5f8..cddc05cbf7e 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/EverywhereReplicationStrategy.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/EverywhereReplicationStrategy.java @@ -19,8 +19,9 @@ import com.datastax.oss.driver.api.core.metadata.Node; import com.datastax.oss.driver.api.core.metadata.token.Token; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; -import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -30,12 +31,16 @@ public class EverywhereReplicationStrategy implements ReplicationStrategy { @Override - public Map> computeReplicasByToken( + public Map> computeReplicasListByToken( Map tokenToPrimary, List ring) { - ImmutableMap.Builder> result = ImmutableMap.builder(); - Set allNodes = ImmutableSet.copyOf(tokenToPrimary.values()); + ImmutableMap.Builder> result = ImmutableMap.builder(); + Set uniqueNodes = new LinkedHashSet<>(); + for (Token token : ring) { + uniqueNodes.add(tokenToPrimary.get(token)); + } + ImmutableList allNodes = ImmutableList.copyOf(uniqueNodes); for (Token token : tokenToPrimary.keySet()) { - result = result.put(token, allNodes); + result.put(token, allNodes); } return result.build(); } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/KeyspaceTokenMap.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/KeyspaceTokenMap.java index 5940c63bdd2..e1873fb9516 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/KeyspaceTokenMap.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/KeyspaceTokenMap.java @@ -61,7 +61,8 @@ static KeyspaceTokenMap build( try { ReplicationStrategy strategy = replicationStrategyFactory.newInstance(replicationConfig); - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); SetMultimap tokenRangesByNode; if (ring.size() == 1) { // We forced the single range to ]minToken,minToken], make sure to use that instead of @@ -87,13 +88,13 @@ static KeyspaceTokenMap build( private final List ring; private final SetMultimap tokenRangesByNode; - private final Map> replicasByToken; + private final Map> replicasByToken; private final TokenFactory tokenFactory; private KeyspaceTokenMap( List ring, SetMultimap tokenRangesByNode, - Map> replicasByToken, + Map> replicasByToken, TokenFactory tokenFactory) { this.ring = ring; this.tokenRangesByNode = tokenRangesByNode; @@ -105,16 +106,16 @@ Set getTokenRanges(Node replica) { return tokenRangesByNode.get(replica); } - Set getReplicas(Partitioner partitioner, ByteBuffer partitionKey) { + List getReplicas(Partitioner partitioner, ByteBuffer partitionKey) { if (partitioner == null) { partitioner = tokenFactory; } return getReplicas(partitioner.hash(partitionKey)); } - Set getReplicas(Token token) { + List getReplicas(Token token) { // If the token happens to be one of the "primary" tokens, get result directly - Set nodes = replicasByToken.get(token); + List nodes = replicasByToken.get(token); if (nodes != null) { return nodes; } @@ -130,7 +131,7 @@ Set getReplicas(Token token) { } private static SetMultimap buildTokenRangesByNode( - Set tokenRanges, Map> replicasByToken) { + Set tokenRanges, Map> replicasByToken) { ImmutableSetMultimap.Builder result = ImmutableSetMultimap.builder(); for (TokenRange range : tokenRanges) { for (Node node : replicasByToken.get(range.getEnd())) { diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/LocalReplicationStrategy.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/LocalReplicationStrategy.java index 916947e598c..fe67b717d46 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/LocalReplicationStrategy.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/LocalReplicationStrategy.java @@ -19,23 +19,22 @@ import com.datastax.oss.driver.api.core.metadata.Node; import com.datastax.oss.driver.api.core.metadata.token.Token; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; -import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; import java.util.List; import java.util.Map; -import java.util.Set; import net.jcip.annotations.ThreadSafe; @ThreadSafe class LocalReplicationStrategy implements ReplicationStrategy { @Override - public Map> computeReplicasByToken( + public Map> computeReplicasListByToken( Map tokenToPrimary, List ring) { - ImmutableMap.Builder> result = ImmutableMap.builder(); + ImmutableMap.Builder> result = ImmutableMap.builder(); // Each token maps to exactly one node for (Map.Entry entry : tokenToPrimary.entrySet()) { - result.put(entry.getKey(), ImmutableSet.of(entry.getValue())); + result.put(entry.getKey(), ImmutableList.of(entry.getValue())); } return result.build(); } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/NetworkTopologyReplicationStrategy.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/NetworkTopologyReplicationStrategy.java index 0ed81083ad6..effb8437548 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/NetworkTopologyReplicationStrategy.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/NetworkTopologyReplicationStrategy.java @@ -56,17 +56,17 @@ class NetworkTopologyReplicationStrategy implements ReplicationStrategy { } @Override - public Map> computeReplicasByToken( + public Map> computeReplicasListByToken( Map tokenToPrimary, List ring) { // The implementation of this method was adapted from // org.apache.cassandra.locator.NetworkTopologyStrategy - ImmutableMap.Builder> result = ImmutableMap.builder(); + ImmutableMap.Builder> result = ImmutableMap.builder(); Map> racks = getRacksInDcs(tokenToPrimary.values()); Map dcNodeCount = Maps.newHashMapWithExpectedSize(replicationFactors.size()); Set warnedDcs = Sets.newHashSetWithExpectedSize(replicationFactors.size()); - CanonicalNodeSetBuilder replicasBuilder = new CanonicalNodeSetBuilder(); + CanonicalNodeListBuilder replicasBuilder = new CanonicalNodeListBuilder(); // find maximum number of nodes in each DC for (Node node : Sets.newHashSet(tokenToPrimary.values())) { diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationStrategy.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationStrategy.java index e16841e5107..ba329160a58 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationStrategy.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationStrategy.java @@ -19,10 +19,37 @@ import com.datastax.oss.driver.api.core.metadata.Node; import com.datastax.oss.driver.api.core.metadata.token.Token; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; public interface ReplicationStrategy { - Map> computeReplicasByToken(Map tokenToPrimary, List ring); + + /** + * Computes the replicas for each token in the ring. + * + * @param tokenToPrimary mapping from token to its primary replica node + * @param ring ordered list of tokens representing the ring + * @return a map where keys are tokens and values are the sets of replica nodes for each token + * @deprecated Use {@link #computeReplicasListByToken(Map, List)} instead. + */ + @Deprecated + default Map> computeReplicasByToken( + Map tokenToPrimary, List ring) { + return computeReplicasListByToken(tokenToPrimary, ring).entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, e -> new HashSet<>(e.getValue()))); + } + + /** + * Computes the replicas for each token in the ring. + * + * @param tokenToPrimary mapping from token to its primary replica node + * @param ring ordered list of tokens representing the ring + * @return a map where keys are tokens and values are the ordered lists of replica nodes for each + * token + */ + Map> computeReplicasListByToken( + Map tokenToPrimary, List ring); } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/SimpleReplicationStrategy.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/SimpleReplicationStrategy.java index db2c16112a1..c6e7b6dfe7f 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/SimpleReplicationStrategy.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/SimpleReplicationStrategy.java @@ -24,7 +24,6 @@ import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; import java.util.List; import java.util.Map; -import java.util.Set; import net.jcip.annotations.ThreadSafe; @ThreadSafe @@ -42,13 +41,13 @@ class SimpleReplicationStrategy implements ReplicationStrategy { } @Override - public Map> computeReplicasByToken( + public Map> computeReplicasListByToken( Map tokenToPrimary, List ring) { int rf = Math.min(replicationFactor.fullReplicas(), ring.size()); - ImmutableMap.Builder> result = ImmutableMap.builder(); - CanonicalNodeSetBuilder replicasBuilder = new CanonicalNodeSetBuilder(); + ImmutableMap.Builder> result = ImmutableMap.builder(); + CanonicalNodeListBuilder replicasBuilder = new CanonicalNodeListBuilder(); for (int i = 0; i < ring.size(); i++) { replicasBuilder.clear(); diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyDcFailoverTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyDcFailoverTest.java index 82d70ac583d..e57cc7824d2 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyDcFailoverTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyDcFailoverTest.java @@ -37,8 +37,8 @@ import com.datastax.oss.driver.api.core.config.DriverExecutionProfile; import com.datastax.oss.driver.api.core.metadata.Node; import com.datastax.oss.driver.internal.core.metadata.DefaultNode; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; -import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; import java.util.Map; import java.util.UUID; import org.junit.Test; @@ -60,7 +60,7 @@ public class BasicLoadBalancingPolicyDcFailoverTest extends BasicLoadBalancingPo public void should_prioritize_single_replica() { when(request.getRoutingKeyspace()).thenReturn(KEYSPACE); when(request.getRoutingKey()).thenReturn(ROUTING_KEY); - when(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)).thenReturn(ImmutableSet.of(node3)); + when(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)).thenReturn(ImmutableList.of(node3)); // node3 always first, round-robin on the rest, then remote nodes assertThat(policy.newQueryPlan(request, session)) @@ -81,8 +81,8 @@ public void should_prioritize_single_replica() { public void should_prioritize_and_shuffle_replicas() { when(request.getRoutingKeyspace()).thenReturn(KEYSPACE); when(request.getRoutingKey()).thenReturn(ROUTING_KEY); - when(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .thenReturn(ImmutableSet.of(node2, node3, node5, node8)); + when(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .thenReturn(ImmutableList.of(node2, node3, node5, node8)); // node 5 and 8 being in a remote DC, they don't get a boost for being a replica assertThat(policy.newQueryPlan(request, session)) diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyPreferredRemoteDcsTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyPreferredRemoteDcsTest.java index 1ac63bf9357..df198a13987 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyPreferredRemoteDcsTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyPreferredRemoteDcsTest.java @@ -34,7 +34,6 @@ import com.datastax.oss.driver.internal.core.metadata.DefaultNode; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; -import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; import java.util.Map; import java.util.UUID; import org.junit.Test; @@ -53,10 +52,10 @@ public class BasicLoadBalancingPolicyPreferredRemoteDcsTest public void should_prioritize_single_replica() { when(request.getRoutingKeyspace()).thenReturn(KEYSPACE); when(request.getRoutingKey()).thenReturn(ROUTING_KEY); - when(tokenMap.getReplicas(KEYSPACE, ROUTING_KEY)).thenReturn(ImmutableSet.of(node3)); + when(tokenMap.getReplicasList(KEYSPACE, ROUTING_KEY)).thenReturn(ImmutableList.of(node3)); // Additional mock. Implementation diverges from upstream enough to call this method instead. - when(tokenMap.getReplicas(eq(KEYSPACE), isNull(), eq(ROUTING_KEY))) - .thenReturn(ImmutableSet.of(node3)); + when(tokenMap.getReplicasList(eq(KEYSPACE), isNull(), eq(ROUTING_KEY))) + .thenReturn(ImmutableList.of(node3)); // node3 always first, round-robin on the rest assertThat(policy.newQueryPlan(request, session)) @@ -83,11 +82,11 @@ public void should_prioritize_single_replica() { public void should_prioritize_and_shuffle_replicas() { when(request.getRoutingKeyspace()).thenReturn(KEYSPACE); when(request.getRoutingKey()).thenReturn(ROUTING_KEY); - when(tokenMap.getReplicas(KEYSPACE, ROUTING_KEY)) - .thenReturn(ImmutableSet.of(node1, node2, node3, node6, node9)); + when(tokenMap.getReplicasList(KEYSPACE, ROUTING_KEY)) + .thenReturn(ImmutableList.of(node1, node2, node3, node6, node9)); // Additional mock. Implementation diverges from upstream enough to call this method instead. - when(tokenMap.getReplicas(eq(KEYSPACE), isNull(), eq(ROUTING_KEY))) - .thenReturn(ImmutableSet.of(node1, node2, node3, node6, node9)); + when(tokenMap.getReplicasList(eq(KEYSPACE), isNull(), eq(ROUTING_KEY))) + .thenReturn(ImmutableList.of(node1, node2, node3, node6, node9)); // node 6 and 9 being in a remote DC, they don't get a boost for being a replica assertThat(policy.newQueryPlan(request, session)) diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyQueryPlanTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyQueryPlanTest.java index 30d2e680295..c2e89cdf07c 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyQueryPlanTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/BasicLoadBalancingPolicyQueryPlanTest.java @@ -23,7 +23,6 @@ */ package com.datastax.oss.driver.internal.core.loadbalancing; -import static java.util.Collections.emptySet; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; @@ -45,6 +44,7 @@ import com.datastax.oss.driver.api.core.metadata.token.Token; import com.datastax.oss.driver.api.core.session.Request; import com.datastax.oss.driver.internal.core.session.DefaultSession; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; import com.datastax.oss.protocol.internal.util.Bytes; @@ -93,10 +93,10 @@ public void should_use_round_robin_when_no_request() { assertRoundRobinQueryPlans(); // Then - then(tokenMap).should(never()).getReplicas(any(CqlIdentifier.class), any(Token.class)); + then(tokenMap).should(never()).getReplicasList(any(CqlIdentifier.class), any(Token.class)); then(tokenMap) .should(never()) - .getReplicas(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); + .getReplicasList(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); } @Test @@ -110,10 +110,10 @@ public void should_use_round_robin_when_no_session() { // Then then(request).should(never()).getRoutingKey(); then(request).should(never()).getRoutingToken(); - then(tokenMap).should(never()).getReplicas(any(CqlIdentifier.class), any(Token.class)); + then(tokenMap).should(never()).getReplicasList(any(CqlIdentifier.class), any(Token.class)); then(tokenMap) .should(never()) - .getReplicas(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); + .getReplicasList(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); } @Test @@ -126,10 +126,10 @@ public void should_use_round_robin_when_request_has_no_routing_keyspace() { then(request).should(never()).getRoutingKey(); then(request).should(never()).getRoutingToken(); - then(tokenMap).should(never()).getReplicas(any(CqlIdentifier.class), any(Token.class)); + then(tokenMap).should(never()).getReplicasList(any(CqlIdentifier.class), any(Token.class)); then(tokenMap) .should(never()) - .getReplicas(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); + .getReplicasList(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); } @Test @@ -140,10 +140,10 @@ public void should_use_round_robin_when_request_has_no_routing_key_or_token() { assertRoundRobinQueryPlans(); - then(tokenMap).should(never()).getReplicas(any(CqlIdentifier.class), any(Token.class)); + then(tokenMap).should(never()).getReplicasList(any(CqlIdentifier.class), any(Token.class)); then(tokenMap) .should(never()) - .getReplicas(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); + .getReplicasList(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); } @Test @@ -152,10 +152,10 @@ public void should_use_round_robin_when_token_map_absent() { assertRoundRobinQueryPlans(); - then(tokenMap).should(never()).getReplicas(any(CqlIdentifier.class), any(Token.class)); + then(tokenMap).should(never()).getReplicasList(any(CqlIdentifier.class), any(Token.class)); then(tokenMap) .should(never()) - .getReplicas(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); + .getReplicasList(any(CqlIdentifier.class), isNull(), any(ByteBuffer.class)); } @Test @@ -163,11 +163,11 @@ public void should_use_round_robin_when_token_map_absent() { should_use_round_robin_when_token_map_returns_no_replicas_using_request_keyspace_and_routing_key() { when(request.getRoutingKeyspace()).thenReturn(KEYSPACE); when(request.getRoutingKey()).thenReturn(ROUTING_KEY); - when(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)).thenReturn(Collections.emptySet()); + when(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)).thenReturn(Collections.emptyList()); assertRoundRobinQueryPlans(); - then(tokenMap).should(atLeast(1)).getReplicas(KEYSPACE, null, ROUTING_KEY); + then(tokenMap).should(atLeast(1)).getReplicasList(KEYSPACE, null, ROUTING_KEY); } @Test @@ -178,11 +178,12 @@ public void should_use_round_robin_when_token_map_absent() { given(request.getRoutingKeyspace()).willReturn(null); given(session.getKeyspace()).willReturn(Optional.of(KEYSPACE)); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)).willReturn(emptySet()); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(Collections.emptyList()); // When assertRoundRobinQueryPlans(); // Then - then(tokenMap).should(atLeast(1)).getReplicas(KEYSPACE, null, ROUTING_KEY); + then(tokenMap).should(atLeast(1)).getReplicasList(KEYSPACE, null, ROUTING_KEY); } @Test @@ -192,11 +193,11 @@ public void should_use_round_robin_when_token_map_absent() { given(request.getKeyspace()).willReturn(null); given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingToken()).willReturn(routingToken); - given(tokenMap.getReplicas(KEYSPACE, routingToken)).willReturn(emptySet()); + given(tokenMap.getReplicasList(KEYSPACE, routingToken)).willReturn(Collections.emptyList()); // When assertRoundRobinQueryPlans(); // Then - then(tokenMap).should(atLeast(1)).getReplicas(KEYSPACE, routingToken); + then(tokenMap).should(atLeast(1)).getReplicasList(KEYSPACE, routingToken); } @Test @@ -230,7 +231,7 @@ protected void assertRoundRobinQueryPlans() { public void should_prioritize_single_replica() { when(request.getRoutingKeyspace()).thenReturn(KEYSPACE); when(request.getRoutingKey()).thenReturn(ROUTING_KEY); - when(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)).thenReturn(ImmutableSet.of(node3)); + when(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)).thenReturn(ImmutableList.of(node3)); // node3 always first, round-robin on the rest assertThat(policy.newQueryPlan(request, session)) @@ -250,8 +251,8 @@ public void should_prioritize_single_replica() { public void should_prioritize_and_shuffle_replicas() { when(request.getRoutingKeyspace()).thenReturn(KEYSPACE); when(request.getRoutingKey()).thenReturn(ROUTING_KEY); - when(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .thenReturn(ImmutableSet.of(node3, node5)); + when(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .thenReturn(ImmutableList.of(node3, node5)); assertThat(policy.newQueryPlan(request, session)) .containsExactly(node3, node5, node1, node2, node4); diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyQueryPlanTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyQueryPlanTest.java index e68dfdaba05..f016323c16b 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyQueryPlanTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyQueryPlanTest.java @@ -35,6 +35,7 @@ import com.datastax.oss.driver.api.core.metadata.Node; import com.datastax.oss.driver.internal.core.pool.ChannelPool; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; import java.util.Optional; @@ -95,8 +96,8 @@ public void should_prioritize_and_shuffle_2_replicas() { // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node3, node5)); // When Queue plan1 = dsePolicy.newQueryPlan(request, session); @@ -119,8 +120,8 @@ public void should_prioritize_and_shuffle_3_or_more_replicas_when_all_healthy_an // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); dsePolicy.upTimes.put(node1, T1); dsePolicy.upTimes.put(node3, T2); dsePolicy.upTimes.put(node5, T3); // newest up replica @@ -148,8 +149,8 @@ public void should_prioritize_and_shuffle_3_or_more_replicas_when_all_healthy_an // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); dsePolicy.upTimes.put(node1, T2); // newest up replica dsePolicy.upTimes.put(node3, T1); given(pool3.getInFlight()).willReturn(0); @@ -176,8 +177,8 @@ public void should_prioritize_and_shuffle_3_or_more_replicas_when_all_healthy_an // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); dsePolicy.upTimes.put(node1, T2); // newest up replica dsePolicy.upTimes.put(node3, T1); given(pool1.getInFlight()).willReturn(0); @@ -204,8 +205,8 @@ public void should_prioritize_and_shuffle_3_or_more_replicas_when_first_unhealth // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); given(pool1.getInFlight()).willReturn(100); // unhealthy given(pool3.getInFlight()).willReturn(0); given(pool5.getInFlight()).willReturn(0); @@ -236,8 +237,8 @@ public void should_prioritize_and_shuffle_3_or_more_replicas_when_first_unhealth // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); given(pool1.getInFlight()).willReturn(100); // unhealthy given(pool3.getInFlight()).willReturn(0); given(pool5.getInFlight()).willReturn(0); @@ -266,8 +267,8 @@ public void should_prioritize_and_shuffle_3_or_more_replicas_when_last_unhealthy // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); given(pool1.getInFlight()).willReturn(0); given(pool3.getInFlight()).willReturn(0); given(pool5.getInFlight()).willReturn(100); // unhealthy @@ -292,8 +293,8 @@ public void should_prioritize_and_shuffle_3_or_more_replicas_when_majority_unhea // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); given(pool1.getInFlight()).willReturn(100); given(pool3.getInFlight()).willReturn(100); given(pool5.getInFlight()).willReturn(0); @@ -318,8 +319,8 @@ public void should_reorder_first_two_replicas_when_first_has_more_in_flight_than // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); given(pool1.getInFlight()).willReturn(200); given(pool3.getInFlight()).willReturn(100); @@ -343,8 +344,8 @@ public void should_not_shuffle_local_rack_replica_with_all_replicas_healthy() { // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); String localRack = "rack1"; given(dsePolicy.getLocalRack()).willReturn(localRack); given(node1.getRack()).willReturn(localRack); @@ -367,8 +368,8 @@ public void should_prefer_local_rack_replica_with_less_inflight_requests() { // Given given(request.getRoutingKeyspace()).willReturn(KEYSPACE); given(request.getRoutingKey()).willReturn(ROUTING_KEY); - given(tokenMap.getReplicas(KEYSPACE, null, ROUTING_KEY)) - .willReturn(ImmutableSet.of(node1, node3, node5)); + given(tokenMap.getReplicasList(KEYSPACE, null, ROUTING_KEY)) + .willReturn(ImmutableList.of(node1, node3, node5)); String localRack = "rack1"; given(dsePolicy.getLocalRack()).willReturn(localRack); given(node3.getRack()).willReturn(localRack); diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultTabletMapTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultTabletMapTest.java index 7253301d9b8..d87be9fe43e 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultTabletMapTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultTabletMapTest.java @@ -7,11 +7,11 @@ import com.datastax.oss.driver.api.core.metadata.Node; import com.datastax.oss.driver.api.core.metadata.Tablet; import com.datastax.oss.driver.api.core.metadata.TabletMap; +import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; +import java.util.List; import java.util.Map; -import java.util.Set; import org.junit.Test; import org.testng.Assert; @@ -21,19 +21,20 @@ public class DefaultTabletMapTest { public void should_remove_overlapping_tablets() { TabletMap tabletMap = DefaultTabletMap.emptyMap(); Tablet tablet1 = - new DefaultTabletMap.DefaultTablet(0, 1, Collections.emptySet(), Collections.emptyMap()); + new DefaultTabletMap.DefaultTablet(0, 1, Collections.emptyList(), Collections.emptyMap()); Tablet tablet2 = - new DefaultTabletMap.DefaultTablet(1, 2, Collections.emptySet(), Collections.emptyMap()); + new DefaultTabletMap.DefaultTablet(1, 2, Collections.emptyList(), Collections.emptyMap()); Tablet tablet3 = - new DefaultTabletMap.DefaultTablet(2, 3, Collections.emptySet(), Collections.emptyMap()); + new DefaultTabletMap.DefaultTablet(2, 3, Collections.emptyList(), Collections.emptyMap()); Tablet tablet4 = new DefaultTabletMap.DefaultTablet( - -100, 100, Collections.emptySet(), Collections.emptyMap()); + -100, 100, Collections.emptyList(), Collections.emptyMap()); Tablet tablet5 = - new DefaultTabletMap.DefaultTablet(-10, 10, Collections.emptySet(), Collections.emptyMap()); + new DefaultTabletMap.DefaultTablet( + -10, 10, Collections.emptyList(), Collections.emptyMap()); Tablet tablet6 = - new DefaultTabletMap.DefaultTablet(9, 20, Collections.emptySet(), Collections.emptyMap()); + new DefaultTabletMap.DefaultTablet(9, 20, Collections.emptyList(), Collections.emptyMap()); KeyspaceTableNamePair key1 = new KeyspaceTableNamePair(CqlIdentifier.fromCql("ks"), CqlIdentifier.fromCql("tab")); @@ -75,7 +76,7 @@ public void tablet_range_should_not_include_first_token() { TabletMap tabletMap = DefaultTabletMap.emptyMap(); Tablet tablet1 = new DefaultTabletMap.DefaultTablet( - -123, 123, Collections.emptySet(), Collections.emptyMap()); + -123, 123, Collections.emptyList(), Collections.emptyMap()); tabletMap.addTablet(CqlIdentifier.fromCql("ks"), CqlIdentifier.fromCql("tab"), tablet1); Tablet result = tabletMap.getTablet(CqlIdentifier.fromCql("ks"), CqlIdentifier.fromCql("tab"), -123); @@ -87,7 +88,7 @@ public void tablet_range_should_include_last_token() { TabletMap tabletMap = DefaultTabletMap.emptyMap(); Tablet tablet1 = new DefaultTabletMap.DefaultTablet( - -123, 456, Collections.emptySet(), Collections.emptyMap()); + -123, 456, Collections.emptyList(), Collections.emptyMap()); tabletMap.addTablet(CqlIdentifier.fromCql("ks"), CqlIdentifier.fromCql("tab"), tablet1); Tablet result = tabletMap.getTablet(CqlIdentifier.fromCql("ks"), CqlIdentifier.fromCql("tab"), 456); @@ -98,9 +99,7 @@ public void tablet_range_should_include_last_token() { public void should_return_correct_shard() { Node node1 = mock(DefaultNode.class); Node node2 = mock(DefaultNode.class); - Set replicaNodes = new HashSet(); - replicaNodes.add(node1); - replicaNodes.add(node2); + List replicaNodes = ImmutableList.of(node1, node2); Map replicaShards = new HashMap<>(); replicaShards.put(node1, 1); replicaShards.put(node2, 2); diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/DefaultTokenMapTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/DefaultTokenMapTest.java index 3170e2dd6b2..8fd985ce84f 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/DefaultTokenMapTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/DefaultTokenMapTest.java @@ -113,15 +113,15 @@ public void should_build_token_map() { assertThat(tokenMap.getTokenRanges(KS1, node3)).containsOnly(RANGE23, RANGE12); assertThat(tokenMap.getTokenRanges(KS1, node4)).containsOnly(RANGE34, RANGE23); - assertThat(tokenMap.getReplicas(KS1, RANGE12)).containsOnly(node2, node3); - assertThat(tokenMap.getReplicas(KS1, RANGE23)).containsOnly(node3, node4); - assertThat(tokenMap.getReplicas(KS1, RANGE34)).containsOnly(node1, node4); - assertThat(tokenMap.getReplicas(KS1, RANGE41)).containsOnly(node1, node2); + assertThat(tokenMap.getReplicasList(KS1, RANGE12)).containsOnly(node2, node3); + assertThat(tokenMap.getReplicasList(KS1, RANGE23)).containsOnly(node3, node4); + assertThat(tokenMap.getReplicasList(KS1, RANGE34)).containsOnly(node1, node4); + assertThat(tokenMap.getReplicasList(KS1, RANGE41)).containsOnly(node1, node2); - assertThat(tokenMap.getReplicas(KS1, ROUTING_KEY12)).containsOnly(node2, node3); - assertThat(tokenMap.getReplicas(KS1, ROUTING_KEY23)).containsOnly(node3, node4); - assertThat(tokenMap.getReplicas(KS1, ROUTING_KEY34)).containsOnly(node1, node4); - assertThat(tokenMap.getReplicas(KS1, ROUTING_KEY41)).containsOnly(node1, node2); + assertThat(tokenMap.getReplicasList(KS1, ROUTING_KEY12)).containsOnly(node2, node3); + assertThat(tokenMap.getReplicasList(KS1, ROUTING_KEY23)).containsOnly(node3, node4); + assertThat(tokenMap.getReplicasList(KS1, ROUTING_KEY34)).containsOnly(node1, node4); + assertThat(tokenMap.getReplicasList(KS1, ROUTING_KEY41)).containsOnly(node1, node2); // KS2 is only replicated on DC1 assertThat(tokenMap.getTokenRanges(KS2, node1)).containsOnly(RANGE41, RANGE34); @@ -129,15 +129,15 @@ public void should_build_token_map() { assertThat(tokenMap.getTokenRanges(KS2, node2)).isEmpty(); assertThat(tokenMap.getTokenRanges(KS2, node4)).isEmpty(); - assertThat(tokenMap.getReplicas(KS2, RANGE12)).containsOnly(node3); - assertThat(tokenMap.getReplicas(KS2, RANGE23)).containsOnly(node3); - assertThat(tokenMap.getReplicas(KS2, RANGE34)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS2, RANGE41)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS2, RANGE12)).containsOnly(node3); + assertThat(tokenMap.getReplicasList(KS2, RANGE23)).containsOnly(node3); + assertThat(tokenMap.getReplicasList(KS2, RANGE34)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS2, RANGE41)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS2, ROUTING_KEY12)).containsOnly(node3); - assertThat(tokenMap.getReplicas(KS2, ROUTING_KEY23)).containsOnly(node3); - assertThat(tokenMap.getReplicas(KS2, ROUTING_KEY34)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS2, ROUTING_KEY41)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS2, ROUTING_KEY12)).containsOnly(node3); + assertThat(tokenMap.getReplicasList(KS2, ROUTING_KEY23)).containsOnly(node3); + assertThat(tokenMap.getReplicasList(KS2, ROUTING_KEY34)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS2, ROUTING_KEY41)).containsOnly(node1); } @Test @@ -157,18 +157,18 @@ public void should_build_token_map_with_single_node() { assertThat(tokenMap.getTokenRanges()).containsExactly(FULL_RING); assertThat(tokenMap.getTokenRanges(KS1, node1)).containsOnly(FULL_RING); - assertThat(tokenMap.getReplicas(KS1, FULL_RING)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS1, ROUTING_KEY12)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS1, ROUTING_KEY23)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS1, ROUTING_KEY34)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS1, ROUTING_KEY41)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS1, FULL_RING)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS1, ROUTING_KEY12)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS1, ROUTING_KEY23)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS1, ROUTING_KEY34)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS1, ROUTING_KEY41)).containsOnly(node1); assertThat(tokenMap.getTokenRanges(KS2, node1)).containsOnly(FULL_RING); - assertThat(tokenMap.getReplicas(KS2, FULL_RING)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS2, ROUTING_KEY12)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS2, ROUTING_KEY23)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS2, ROUTING_KEY34)).containsOnly(node1); - assertThat(tokenMap.getReplicas(KS2, ROUTING_KEY41)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS2, FULL_RING)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS2, ROUTING_KEY12)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS2, ROUTING_KEY23)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS2, ROUTING_KEY34)).containsOnly(node1); + assertThat(tokenMap.getReplicasList(KS2, ROUTING_KEY41)).containsOnly(node1); } @Test diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/NetworkTopologyReplicationStrategyTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/NetworkTopologyReplicationStrategyTest.java index 42dc5e69199..758e54cd5b9 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/NetworkTopologyReplicationStrategyTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/NetworkTopologyReplicationStrategyTest.java @@ -35,7 +35,6 @@ import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Test; import org.junit.runner.RunWith; @@ -95,7 +94,8 @@ public void should_compute_for_simple_layout() { new NetworkTopologyReplicationStrategy(ImmutableMap.of(DC1, "1", DC2, "1"), "test"); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -131,7 +131,8 @@ public void should_compute_for_simple_layout_with_multiple_nodes_per_rack() { new NetworkTopologyReplicationStrategy(ImmutableMap.of(DC1, "1", DC2, "1"), "test"); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -167,7 +168,8 @@ public void should_compute_for_simple_layout_with_3_dcs() { ImmutableMap.of(DC1, "1", DC2, "1", DC3, "1"), "test"); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -208,7 +210,8 @@ public void should_compute_for_unbalanced_ring() { new NetworkTopologyReplicationStrategy(ImmutableMap.of(DC1, "2", DC2, "2"), "test"); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -264,7 +267,8 @@ public void should_compute_with_multiple_racks_per_dc() { new NetworkTopologyReplicationStrategy(ImmutableMap.of(DC1, "2", DC2, "2"), "test"); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -332,7 +336,8 @@ public void should_pick_dc_replicas_in_different_racks_first() { new NetworkTopologyReplicationStrategy(ImmutableMap.of(DC1, "3", DC2, "3"), "test"); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -371,7 +376,7 @@ public void should_pick_dc_replicas_in_different_racks_first() { @Test public void should_pick_dc_replicas_in_different_racks_first_when_nodes_own_consecutive_tokens() { // When - Map> replicasByToken = computeWithDifferentRacksAndConsecutiveTokens(3); + Map> replicasByToken = computeWithDifferentRacksAndConsecutiveTokens(3); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(16); @@ -411,7 +416,7 @@ public void should_pick_dc_replicas_in_different_racks_first_when_nodes_own_cons @Test public void should_pick_dc_replicas_in_different_racks_first_when_all_nodes_contain_all_data() { // When - Map> replicasByToken = computeWithDifferentRacksAndConsecutiveTokens(4); + Map> replicasByToken = computeWithDifferentRacksAndConsecutiveTokens(4); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(16); @@ -441,7 +446,7 @@ public void should_pick_dc_replicas_in_different_racks_first_when_all_nodes_cont assertThat(replicasByToken.get(TOKEN19)).isSameAs(replicasByToken.get(TOKEN18)); } - private Map> computeWithDifferentRacksAndConsecutiveTokens( + private Map> computeWithDifferentRacksAndConsecutiveTokens( int replicationFactor) { List ring = ImmutableList.of( @@ -480,7 +485,7 @@ private Map> computeWithDifferentRacksAndConsecutiveTokens( DC1, Integer.toString(replicationFactor), DC2, Integer.toString(replicationFactor)), "test"); - return strategy.computeReplicasByToken(tokenToPrimary, ring); + return strategy.computeReplicasListByToken(tokenToPrimary, ring); } /** @@ -491,7 +496,7 @@ private Map> computeWithDifferentRacksAndConsecutiveTokens( @Test public void should_compute_complex_layout() { // When - Map> replicasByToken = computeComplexLayout(2); + Map> replicasByToken = computeComplexLayout(2); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(18); @@ -524,7 +529,7 @@ public void should_compute_complex_layout() { @Test public void should_compute_complex_layout_with_rf_too_high() { // When - Map> replicasByToken = computeComplexLayout(4); + Map> replicasByToken = computeComplexLayout(4); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(18); @@ -564,7 +569,7 @@ public void should_compute_complex_layout_with_rf_too_high() { .containsExactly(node6, node1, node5, node3, node2, node4); } - private Map> computeComplexLayout(int replicationFactor) { + private Map> computeComplexLayout(int replicationFactor) { List ring = ImmutableList.of( TOKEN01, TOKEN02, TOKEN03, TOKEN04, TOKEN05, TOKEN06, TOKEN07, TOKEN08, TOKEN09, @@ -602,7 +607,7 @@ private Map> computeComplexLayout(int replicationFactor) { DC1, Integer.toString(replicationFactor), DC2, Integer.toString(replicationFactor)), "test"); - return strategy.computeReplicasByToken(tokenToPrimary, ring); + return strategy.computeReplicasListByToken(tokenToPrimary, ring); } /** @@ -663,7 +668,7 @@ private int countTraversedTokens( return invocation.callRealMethod(); }); new NetworkTopologyReplicationStrategy(replicationConfig, "test") - .computeReplicasByToken(tokenToPrimary, ringSpy); + .computeReplicasListByToken(tokenToPrimary, ringSpy); return count.get(); } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/SimpleReplicationStrategyTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/SimpleReplicationStrategyTest.java index 517d8cfdb84..6a70b28800d 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/SimpleReplicationStrategyTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/token/SimpleReplicationStrategyTest.java @@ -25,7 +25,6 @@ import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; import java.util.List; import java.util.Map; -import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -66,7 +65,8 @@ public void should_compute_for_simple_layout() { SimpleReplicationStrategy strategy = new SimpleReplicationStrategy(new ReplicationFactor(2)); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -87,7 +87,8 @@ public void should_compute_when_nodes_own_consecutive_tokens() { SimpleReplicationStrategy strategy = new SimpleReplicationStrategy(new ReplicationFactor(2)); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -107,7 +108,8 @@ public void should_compute_when_ring_unbalanced() { SimpleReplicationStrategy strategy = new SimpleReplicationStrategy(new ReplicationFactor(2)); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -127,7 +129,8 @@ public void should_compute_when_replication_factor_is_larger_than_cluster_size() SimpleReplicationStrategy strategy = new SimpleReplicationStrategy(new ReplicationFactor(6)); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); @@ -186,7 +189,8 @@ public void should_compute_for_complex_layout() { SimpleReplicationStrategy strategy = new SimpleReplicationStrategy(new ReplicationFactor(3)); // When - Map> replicasByToken = strategy.computeReplicasByToken(tokenToPrimary, ring); + Map> replicasByToken = + strategy.computeReplicasListByToken(tokenToPrimary, ring); // Then assertThat(replicasByToken.keySet().size()).isEqualTo(ring.size()); diff --git a/examples/src/main/java/com/datastax/oss/driver/examples/basic/TokenMapAndShardIdLookup.java b/examples/src/main/java/com/datastax/oss/driver/examples/basic/TokenMapAndShardIdLookup.java index d22dd080f5d..25b31b49fc6 100644 --- a/examples/src/main/java/com/datastax/oss/driver/examples/basic/TokenMapAndShardIdLookup.java +++ b/examples/src/main/java/com/datastax/oss/driver/examples/basic/TokenMapAndShardIdLookup.java @@ -12,7 +12,7 @@ import com.datastax.oss.driver.api.core.metadata.token.Token; import com.datastax.oss.driver.api.core.type.codec.TypeCodecs; import java.nio.ByteBuffer; -import java.util.Set; +import java.util.List; /** * Demonstrates usage of TokenMap and NodeShardingInfo Needs a Scylla cluster to be running locally @@ -58,12 +58,12 @@ public static void main(String[] args) { TokenMap tokenMap = metadata.getTokenMap().get(); - Set nodes = - tokenMap.getReplicas(CqlIdentifier.fromCql("tokenmap_example_ks"), PARTITION_KEY); + List nodes = + tokenMap.getReplicasList(CqlIdentifier.fromCql("tokenmap_example_ks"), PARTITION_KEY); System.out.println("Replica set size: " + nodes.size()); Token token = tokenMap.newToken(PARTITION_KEY); - assert nodes.size() > 0; + assert !nodes.isEmpty(); Node node = nodes.iterator().next(); assert node.getShardingInfo() != null; diff --git a/integration-tests/src/test/java/com/datastax/oss/driver/core/loadbalancing/DefaultLoadBalancingPolicyIT.java b/integration-tests/src/test/java/com/datastax/oss/driver/core/loadbalancing/DefaultLoadBalancingPolicyIT.java index 79765ef8bcf..ebd41de6e89 100644 --- a/integration-tests/src/test/java/com/datastax/oss/driver/core/loadbalancing/DefaultLoadBalancingPolicyIT.java +++ b/integration-tests/src/test/java/com/datastax/oss/driver/core/loadbalancing/DefaultLoadBalancingPolicyIT.java @@ -171,7 +171,7 @@ public void should_prioritize_replicas_when_routing_information_present() { ByteBuffer routingKey = TypeCodecs.INT.encodePrimitive(1, ProtocolVersion.DEFAULT); TokenMap tokenMap = SESSION_RULE.session().getMetadata().getTokenMap().get(); Set localReplicas = new HashSet<>(); - for (Node replica : tokenMap.getReplicas(keyspace, routingKey)) { + for (Node replica : tokenMap.getReplicasList(keyspace, routingKey)) { if (replica.getDatacenter().equals(LOCAL_DC)) { localReplicas.add(replica); } @@ -215,7 +215,7 @@ public void should_hit_non_replicas_when_routing_information_present_but_all_rep InternalDriverContext context = (InternalDriverContext) SESSION_RULE.session().getContext(); Set localReplicas = new HashSet<>(); - for (Node replica : tokenMap.getReplicas(keyspace, routingKey)) { + for (Node replica : tokenMap.getReplicasList(keyspace, routingKey)) { if (replica.getDatacenter().equals(LOCAL_DC)) { localReplicas.add(replica); context.getEventBus().fire(TopologyEvent.forceDown(replica.getBroadcastRpcAddress().get())); diff --git a/integration-tests/src/test/java/com/datastax/oss/driver/core/loadbalancing/LWTLoadBalancingIT.java b/integration-tests/src/test/java/com/datastax/oss/driver/core/loadbalancing/LWTLoadBalancingIT.java index 81cfa3c2b5a..9e2d034a19f 100644 --- a/integration-tests/src/test/java/com/datastax/oss/driver/core/loadbalancing/LWTLoadBalancingIT.java +++ b/integration-tests/src/test/java/com/datastax/oss/driver/core/loadbalancing/LWTLoadBalancingIT.java @@ -83,7 +83,8 @@ public void should_use_only_one_node_when_lwt_detected() { int pk = 1234; ByteBuffer routingKey = TypeCodecs.INT.encodePrimitive(pk, ProtocolVersion.DEFAULT); TokenMap tokenMap = SESSION_RULE.session().getMetadata().getTokenMap().get(); - Node owner = tokenMap.getReplicas(session.getKeyspace().get(), routingKey).iterator().next(); + Node owner = + tokenMap.getReplicasList(session.getKeyspace().get(), routingKey).iterator().next(); PreparedStatement statement = SESSION_RULE .session() @@ -122,7 +123,8 @@ public void should_use_only_one_node_when_lwt_batch_detected() { int pk = 1234; ByteBuffer routingKey = TypeCodecs.INT.encodePrimitive(pk, ProtocolVersion.DEFAULT); TokenMap tokenMap = SESSION_RULE.session().getMetadata().getTokenMap().get(); - Node owner = tokenMap.getReplicas(session.getKeyspace().get(), routingKey).iterator().next(); + Node owner = + tokenMap.getReplicasList(session.getKeyspace().get(), routingKey).iterator().next(); PreparedStatement statement = SESSION_RULE .session() diff --git a/integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/SchemaIT.java b/integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/SchemaIT.java index d4b04d287b7..c6a0b1d0434 100644 --- a/integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/SchemaIT.java +++ b/integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/SchemaIT.java @@ -335,7 +335,7 @@ public void should_exclude_virtual_keyspaces_from_token_map() { assertThat(tokenMap.getReplicas("system_views", partitionKey)).isEmpty(); assertThat(tokenMap.getReplicas("system_virtual_schema", partitionKey)).isEmpty(); // Check that a non-virtual keyspace is present - assertThat(tokenMap.getReplicas(sessionRule.keyspace(), partitionKey)).isNotEmpty(); + assertThat(tokenMap.getReplicasList(sessionRule.keyspace(), partitionKey)).isNotEmpty(); } } diff --git a/integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/TokenITBase.java b/integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/TokenITBase.java index 057461a1bd7..c97a2279efd 100644 --- a/integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/TokenITBase.java +++ b/integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/TokenITBase.java @@ -105,7 +105,7 @@ public void should_be_consistent_with_range_queries() { ProtocolVersion protocolVersion = session().getContext().getProtocolVersion(); ByteBuffer serializedKey = TypeCodecs.INT.encodePrimitive(key, protocolVersion); assertThat(serializedKey).isNotNull(); - Set replicas = tokenMap.getReplicas(KS1, serializedKey); + List replicas = tokenMap.getReplicasList(KS1, serializedKey); assertThat(replicas).hasSize(1); Node replica = replicas.iterator().next(); @@ -126,7 +126,7 @@ public void should_be_consistent_with_range_queries() { foundRange = range; // That range should be managed by the replica - assertThat(tokenMap.getReplicas(KS1, range)).contains(replica); + assertThat(tokenMap.getReplicasList(KS1, range)).contains(replica); } } } diff --git a/pom.xml b/pom.xml index 321f3fec2d5..ae3b2358d00 100644 --- a/pom.xml +++ b/pom.xml @@ -100,7 +100,7 @@ ${skipTests} false false - +