From f711da8ced03b563579c9b13cbd512e5978d6ccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20B=C4=85czkowski?= Date: Tue, 26 Nov 2024 21:16:56 +0100 Subject: [PATCH 1/2] Make TabletMap return empty replica set when stale replica is found Makes TabletMap return empty collection for some calls to `getReplicas()`. If the current replica list for a tablet turns out to contain a host that is not present in current Metadata then empty collection is returned in an effort to misroute the query. This query will cause the tablet information to be updated when the result with the up to date information is received. It is possible that the query will be randomly routed correctly, but this case is significantly less likely with each subsequent query, although this can depend on underlying load balancing policy. This covers the node removal/replacement case of #378. --- .../java/com/datastax/driver/core/TabletMap.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/driver-core/src/main/java/com/datastax/driver/core/TabletMap.java b/driver-core/src/main/java/com/datastax/driver/core/TabletMap.java index baecc5959b..95136a19d8 100644 --- a/driver-core/src/main/java/com/datastax/driver/core/TabletMap.java +++ b/driver-core/src/main/java/com/datastax/driver/core/TabletMap.java @@ -57,7 +57,9 @@ public Map getMapping() { } /** - * Finds hosts that have replicas for a given table and token combination + * Finds hosts that have replicas for a given table and token combination. Meant for use in query + * planning. Can return empty collection if internal replica list information is determined not up + * to date. * * @param keyspace the keyspace that table is in * @param table the table name @@ -93,8 +95,14 @@ public Set getReplicas(String keyspace, String table, long token) { HashSet uuidSet = new HashSet<>(); for (HostShardPair hostShardPair : row.replicas) { - if (cluster.metadata.getHost(hostShardPair.getHost()) != null) + if (cluster.metadata.getHost(hostShardPair.getHost()) == null) { + // We've encountered a stale host. Return an empty set to + // misroute the request. If misrouted then response will + // contain up to date tablet information that will be processed. + return Collections.emptySet(); + } else { uuidSet.add(hostShardPair.getHost()); + } } return uuidSet; } finally { From f05ba8640c8828dce9654660bc9c9354eb3e2fdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20B=C4=85czkowski?= Date: Wed, 27 Nov 2024 17:30:14 +0100 Subject: [PATCH 2/2] Change return type of `TabletMap#getReplicas()` to `Set` Having this method return set of hosts instead saves one `Map#get()` query per host. In theory, previously it was also possible that between `TabletMap#getReplicas()` returning and `Metadata#getReplicas()` translating uuids to hosts, some hosts could become invalid, making the translation represent them as nulls. --- .../java/com/datastax/driver/core/Metadata.java | 7 +++---- .../java/com/datastax/driver/core/TabletMap.java | 13 +++++++------ .../com/datastax/driver/core/TabletMapListener.java | 2 ++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/driver-core/src/main/java/com/datastax/driver/core/Metadata.java b/driver-core/src/main/java/com/datastax/driver/core/Metadata.java index 5bbcb3a4db..4c0ab210e1 100644 --- a/driver-core/src/main/java/com/datastax/driver/core/Metadata.java +++ b/driver-core/src/main/java/com/datastax/driver/core/Metadata.java @@ -44,7 +44,6 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; -import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -570,9 +569,9 @@ public Set getReplicas( if (keyspace != null && table != null) { Token token = partitioner.hash(partitionKey); assert (token instanceof Token.TokenLong64); - Set hostUuids = tabletMap.getReplicas(keyspace, table, (long) token.getValue()); - if (!hostUuids.isEmpty()) { - return hostUuids.stream().map(this::getHost).collect(Collectors.toSet()); + Set replicas = tabletMap.getReplicas(keyspace, table, (long) token.getValue()); + if (!replicas.isEmpty()) { + return replicas; } } // Fall back to tokenMap diff --git a/driver-core/src/main/java/com/datastax/driver/core/TabletMap.java b/driver-core/src/main/java/com/datastax/driver/core/TabletMap.java index 95136a19d8..a73e06b6c4 100644 --- a/driver-core/src/main/java/com/datastax/driver/core/TabletMap.java +++ b/driver-core/src/main/java/com/datastax/driver/core/TabletMap.java @@ -64,9 +64,9 @@ public Map getMapping() { * @param keyspace the keyspace that table is in * @param table the table name * @param token the token to look for - * @return Set of host UUIDS that do have a tablet for given token for a given table. + * @return Set of Host instances that do have a tablet for the given token and table combination. */ - public Set getReplicas(String keyspace, String table, long token) { + public Set getReplicas(String keyspace, String table, long token) { TabletMap.KeyspaceTableNamePair key = new TabletMap.KeyspaceTableNamePair(keyspace, table); if (mapping == null) { @@ -93,18 +93,19 @@ public Set getReplicas(String keyspace, String table, long token) { return Collections.emptySet(); } - HashSet uuidSet = new HashSet<>(); + HashSet replicaSet = new HashSet<>(); for (HostShardPair hostShardPair : row.replicas) { - if (cluster.metadata.getHost(hostShardPair.getHost()) == null) { + Host replica = cluster.metadata.getHost(hostShardPair.getHost()); + if (replica == null) { // We've encountered a stale host. Return an empty set to // misroute the request. If misrouted then response will // contain up to date tablet information that will be processed. return Collections.emptySet(); } else { - uuidSet.add(hostShardPair.getHost()); + replicaSet.add(replica); } } - return uuidSet; + return replicaSet; } finally { readLock.unlock(); } diff --git a/driver-core/src/main/java/com/datastax/driver/core/TabletMapListener.java b/driver-core/src/main/java/com/datastax/driver/core/TabletMapListener.java index 7b3ffa5901..788b55fae6 100644 --- a/driver-core/src/main/java/com/datastax/driver/core/TabletMapListener.java +++ b/driver-core/src/main/java/com/datastax/driver/core/TabletMapListener.java @@ -1,5 +1,7 @@ package com.datastax.driver.core; +// This class does not handle topology changes. Node removal is covered by TabletMap#getReplicas() +// implementation and tablet overlap is resolved when adding new tablets. public class TabletMapListener extends SchemaChangeListenerBase { private final TabletMap tabletMap;