Don't hop DCs for LOCAL_* CLs by default

Sylvain Lebresne · Sylvain Lebresne · commit 5a4b45ea9014 · 2014-03-17T20:24:32.000+01:00
JAVA-289 #fixes
diff --git a/driver-core/CHANGELOG.rst b/driver-core/CHANGELOG.rst
@@ -7,6 +7,7 @@ CHANGELOG
 - [new] Add method to check if a Cluster/Session instance has been closed already (JAVA-114)
 - [new] Expose the name of the partitioner in use in the cluster metadata (JAVA-179)
 - [new] Add new WhiteListPolicy to limit the nodes connected to a particular list
+- [improvement] Do not hop DC for LOCAL_* CL in DCAwareRoundRobinPolicy (JAVA-289)
 
 
 1.0.6:
diff --git a/driver-core/src/main/java/com/datastax/driver/core/ConsistencyLevel.java b/driver-core/src/main/java/com/datastax/driver/core/ConsistencyLevel.java
@@ -61,4 +61,13 @@ static org.apache.cassandra.db.ConsistencyLevel toCassandraCL(ConsistencyLevel c
         }
         throw new AssertionError();
     }
+
+    /**
+     * Whether or not the the consistency level applies to the local data-center only.
+     *
+     * @return whether this consistency level is {@code LOCAL_ONE} or {@code LOCAL_QUORUM}.
+     */
+    public boolean isDCLocal() {
+        return this == LOCAL_ONE || this == LOCAL_QUORUM;
+    }
 }
diff --git a/driver-core/src/main/java/com/datastax/driver/core/policies/DCAwareRoundRobinPolicy.java b/driver-core/src/main/java/com/datastax/driver/core/policies/DCAwareRoundRobinPolicy.java
@@ -49,6 +49,7 @@ public class DCAwareRoundRobinPolicy implements LoadBalancingPolicy {
     private final AtomicInteger index = new AtomicInteger();
     private final String localDc;
     private final int usedHostsPerRemoteDc;
+    private final boolean dontHopForLocalCL;
 
     /**
      * Creates a new datacenter aware round robin policy given the name of
@@ -74,6 +75,15 @@ public DCAwareRoundRobinPolicy(String localDc) {
      * <p>
      * The name of the local datacenter provided must be the local
      * datacenter name as known by Cassandra.
+     * <p>
+     * If {@code usedHostsPerRemoteDc > 0}, then if for a query no host
+     * in the local datacenter can be reached and if the consistency
+     * level of the query is not {@code LOCAL_ONE} or {@code LOCAL_QUORUM},
+     * then up to {@code usedHostsPerRemoteDc} host per remote data-center
+     * will be tried by the policy as a fallback. Please note that no
+     * remote host will be used for {@code LOCAL_ONE} and {@code LOCAL_QUORUM}
+     * since this would change the meaning of the consistency level (and
+     * thus somewhat break the consistency contract).
      *
      * @param localDc the name of the local datacenter (as known by
      * Cassandra).
@@ -86,8 +96,41 @@ public DCAwareRoundRobinPolicy(String localDc) {
      * connections to them will be maintained).
      */
     public DCAwareRoundRobinPolicy(String localDc, int usedHostsPerRemoteDc) {
+        this(localDc, usedHostsPerRemoteDc, false);
+    }
+
+    /**
+     * Creates a new DCAwareRoundRobin policy given the name of the local
+     * datacenter and that uses the provided number of host per remote
+     * datacenter as failover for the local hosts.
+     * <p>
+     * This constructor is equivalent to {@link DCAwareRoundRobinPolicy(String, int)}
+     * but allows to override the policy of never using remote data-center
+     * nodes for {@code LOCAL_ONE} and {@code LOCAL_QUORUM} queries. It is
+     * however inadvisable to do so in almost all cases, as this would
+     * potentially break consistency guarantees and if you are fine with that,
+     * it's probably better to use a weaker consitency like {@code ONE}, {@code
+     * TWO} or {@code THREE}. As such, this constructor should generally
+     * be avoided in favor of {@link DCAwareRoundRobinPolicy(String, int)}.
+     * Use it only if you know and understand what you do.
+     *
+     * @param localDc the name of the local datacenter (as known by
+     * Cassandra).
+     * @param usedHostsPerRemoteDc the number of host per remote
+     * datacenter that policies created by the returned factory should
+     * consider. Created policies {@code distance} method will return a
+     * {@code HostDistance.REMOTE} distance for only {@code
+     * usedHostsPerRemoteDc} hosts per remote datacenter. Other hosts
+     * of the remote datacenters will be ignored (and thus no
+     * connections to them will be maintained).
+     * @param allowRemoteDCsForLocalConsistencyLevel whether or not the
+     * policy may return remote host when building query plan for query
+     * having consitency {@code LOCAL_ONE} and {@code LOCAL_QUORUM}.
+     */
+    public DCAwareRoundRobinPolicy(String localDc, int usedHostsPerRemoteDc, boolean allowRemoteDCsForLocalConsistencyLevel) {
         this.localDc = localDc;
         this.usedHostsPerRemoteDc = usedHostsPerRemoteDc;
+        this.dontHopForLocalCL = !allowRemoteDCsForLocalConsistencyLevel;
     }
 
     @Override
@@ -158,7 +201,7 @@ public HostDistance distance(Host host) {
      * try first for querying, which one to use as failover, etc...
      */
     @Override
-    public Iterator<Host> newQueryPlan(Query query) {
+    public Iterator<Host> newQueryPlan(final Query query) {
 
         CopyOnWriteArrayList<Host> localLiveHosts = perDcLiveHosts.get(localDc);
         final List<Host> hosts = localLiveHosts == null ? Collections.<Host>emptyList() : cloneList(localLiveHosts);
@@ -192,6 +235,9 @@ protected Host computeNext() {
                     return currentDcHosts.get(c);
                 }
 
+                if (dontHopForLocalCL && query.getConsistencyLevel().isDCLocal())
+                    return endOfData();
+
                 if (remoteDcs == null) {
                     Set<String> copy = new HashSet<String>(perDcLiveHosts.keySet());
                     copy.remove(localDc);
diff --git a/driver-core/src/main/java/com/datastax/driver/core/policies/RoundRobinPolicy.java b/driver-core/src/main/java/com/datastax/driver/core/policies/RoundRobinPolicy.java
@@ -23,6 +23,8 @@
 import java.util.concurrent.atomic.AtomicInteger;
 
 import com.google.common.collect.AbstractIterator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.datastax.driver.core.Cluster;
 import com.datastax.driver.core.Host;
@@ -43,9 +45,13 @@
  */
 public class RoundRobinPolicy implements LoadBalancingPolicy {
 
+    private static final Logger logger = LoggerFactory.getLogger(RoundRobinPolicy.class);
+
     private final CopyOnWriteArrayList<Host> liveHosts = new CopyOnWriteArrayList<Host>();
     private final AtomicInteger index = new AtomicInteger();
 
+    private volatile boolean hasLoggedLocalCLUse;
+
     /**
      * Creates a load balancing policy that picks host to query in a round robin
      * fashion (on all the hosts of the Cassandra cluster).
@@ -87,6 +93,13 @@ public HostDistance distance(Host host) {
     @Override
     public Iterator<Host> newQueryPlan(Query query) {
 
+        if (!hasLoggedLocalCLUse && query.getConsistencyLevel().isDCLocal()) {
+            hasLoggedLocalCLUse = true;
+            logger.warn("Detected request at Consistency Level {} but the non-DC aware RoundRobinPolicy is in use. "
+                      + "It is strongly advised to use DCAwareRoundRobinPolicy if you have multiple DCs/use DC-aware consistency levels "
+                      + "(note: this message will only be logged once)", query.getConsistencyLevel());
+        }
+
         // We clone liveHosts because we want a version of the list that
         // cannot change concurrently of the query plan iterator (this
         // would be racy). We use clone() as it don't involve a copy of the

Original file line number	Diff line number	Diff line change
`@@ -61,4 +61,13 @@ static org.apache.cassandra.db.ConsistencyLevel toCassandraCL(ConsistencyLevel c`
`61`	`61`	`}`
`62`	`62`	`throw new AssertionError();`
`63`	`63`	`}`
	`64`	`+`
	`65`	`+ /**`
	`66`	`+ * Whether or not the the consistency level applies to the local data-center only.`
	`67`	`+ *`
	`68`	`+ * @return whether this consistency level is {@code LOCAL_ONE} or {@code LOCAL_QUORUM}.`
	`69`	`+ */`
	`70`	`+ public boolean isDCLocal() {`
	`71`	`+ return this == LOCAL_ONE \|\| this == LOCAL_QUORUM;`
	`72`	`+ }`
`64`	`73`	`}`