You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2013/01/09 12:08:24 UTC
[1/4] git commit: Refuse unrecognized replication strategy options
Refuse unrecognized replication strategy options
patch by dbrosius; reviewed by slebresne for CASSANDRA-4795
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/dc94f373
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/dc94f373
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/dc94f373
Branch: refs/heads/trunk
Commit: dc94f3730f50005524edf946772e00bdb7c80327
Parents: 8349fce
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Jan 9 11:17:37 2013 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Jan 9 11:17:37 2013 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../locator/AbstractReplicationStrategy.java | 13 +++++++++----
.../apache/cassandra/locator/LocalStrategy.java | 2 +-
.../cassandra/locator/NetworkTopologyStrategy.java | 1 -
.../apache/cassandra/locator/SimpleStrategy.java | 9 ++++-----
.../apache/cassandra/thrift/CassandraServer.java | 8 ++++++++
6 files changed, 23 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/dc94f373/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 733c3a3..3567362 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -24,6 +24,7 @@
* Ensure CL guarantees on digest mismatch (CASSANDRA-5113)
* Validate correctly selects on composite partition key (CASSANDRA-5122)
* Fix exception when adding collection (CASSANDRA-5117)
+ * Refuse unrecognized replication strategy options (CASSANDRA-4795)
Merged from 1.1:
* Pig: correctly decode row keys in widerow mode (CASSANDRA-5098)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/dc94f373/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java
index c6adad3..3c30baa 100644
--- a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java
+++ b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java
@@ -60,7 +60,7 @@ public abstract class AbstractReplicationStrategy
this.tokenMetadata = tokenMetadata;
this.snitch = snitch;
this.tokenMetadata.register(this);
- this.configOptions = configOptions;
+ this.configOptions = configOptions == null ? Collections.<String, String>emptyMap() : configOptions;
this.table = table;
}
@@ -238,7 +238,12 @@ public abstract class AbstractReplicationStrategy
public static Class<AbstractReplicationStrategy> getClass(String cls) throws ConfigurationException
{
String className = cls.contains(".") ? cls : "org.apache.cassandra.locator." + cls;
- return FBUtilities.classForName(className, "replication strategy");
+ Class<AbstractReplicationStrategy> strategyClass = FBUtilities.classForName(className, "replication strategy");
+ if (!AbstractReplicationStrategy.class.isAssignableFrom(strategyClass))
+ {
+ throw new ConfigurationException(String.format("Specified replication strategy class (%s) is not derived from AbstractReplicationStrategy", className));
+ }
+ return strategyClass;
}
protected void validateReplicationFactor(String rf) throws ConfigurationException
@@ -256,12 +261,12 @@ public abstract class AbstractReplicationStrategy
}
}
- protected void warnOnUnexpectedOptions(Collection<String> expectedOptions)
+ protected void validateExpectedOptions(Collection<String> expectedOptions) throws ConfigurationException
{
for (String key : configOptions.keySet())
{
if (!expectedOptions.contains(key))
- logger.warn("Unrecognized strategy option {" + key + "} passed to " + getClass().getSimpleName() + " for keyspace " + table);
+ throw new ConfigurationException(String.format("Unrecognized strategy option {%s} passed to %s for keyspace %s", key, getClass().getSimpleName(), table));
}
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/dc94f373/src/java/org/apache/cassandra/locator/LocalStrategy.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/locator/LocalStrategy.java b/src/java/org/apache/cassandra/locator/LocalStrategy.java
index 17ad181..4aafa01 100644
--- a/src/java/org/apache/cassandra/locator/LocalStrategy.java
+++ b/src/java/org/apache/cassandra/locator/LocalStrategy.java
@@ -61,6 +61,6 @@ public class LocalStrategy extends AbstractReplicationStrategy
public void validateOptions() throws ConfigurationException
{
// LocalStrategy doesn't expect any options.
- warnOnUnexpectedOptions(Collections.<String>emptySet());
+ validateExpectedOptions(Collections.<String>emptySet());
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/dc94f373/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java b/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java
index c779dc2..6291da0 100644
--- a/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java
+++ b/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java
@@ -189,6 +189,5 @@ public class NetworkTopologyStrategy extends AbstractReplicationStrategy
{
validateReplicationFactor(e.getValue());
}
-
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/dc94f373/src/java/org/apache/cassandra/locator/SimpleStrategy.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/locator/SimpleStrategy.java b/src/java/org/apache/cassandra/locator/SimpleStrategy.java
index 7f27334..5d8a723 100644
--- a/src/java/org/apache/cassandra/locator/SimpleStrategy.java
+++ b/src/java/org/apache/cassandra/locator/SimpleStrategy.java
@@ -68,11 +68,10 @@ public class SimpleStrategy extends AbstractReplicationStrategy
public void validateOptions() throws ConfigurationException
{
- if (configOptions == null || configOptions.get("replication_factor") == null)
- {
+ validateExpectedOptions(Arrays.<String>asList("replication_factor"));
+ String rf = configOptions.get("replication_factor");
+ if (rf == null)
throw new ConfigurationException("SimpleStrategy requires a replication_factor strategy option.");
- }
- warnOnUnexpectedOptions(Arrays.<String>asList("replication_factor"));
- validateReplicationFactor(configOptions.get("replication_factor"));
+ validateReplicationFactor(rf);
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/dc94f373/src/java/org/apache/cassandra/thrift/CassandraServer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/thrift/CassandraServer.java b/src/java/org/apache/cassandra/thrift/CassandraServer.java
index 49fda60..dfeccb7 100644
--- a/src/java/org/apache/cassandra/thrift/CassandraServer.java
+++ b/src/java/org/apache/cassandra/thrift/CassandraServer.java
@@ -51,6 +51,7 @@ import org.apache.cassandra.exceptions.ReadTimeoutException;
import org.apache.cassandra.exceptions.RequestExecutionException;
import org.apache.cassandra.exceptions.RequestValidationException;
import org.apache.cassandra.io.util.DataOutputBuffer;
+import org.apache.cassandra.locator.AbstractReplicationStrategy;
import org.apache.cassandra.locator.DynamicEndpointSnitch;
import org.apache.cassandra.scheduler.IRequestScheduler;
import org.apache.cassandra.service.*;
@@ -1324,6 +1325,13 @@ public class CassandraServer implements Cassandra.Iface
state().hasAllKeyspacesAccess(Permission.CREATE);
ThriftValidation.validateKeyspaceNotYetExisting(ks_def.name);
+ // trial run to let ARS validate class + per-class options
+ AbstractReplicationStrategy.createReplicationStrategy(ks_def.name,
+ AbstractReplicationStrategy.getClass(ks_def.strategy_class),
+ StorageService.instance.getTokenMetadata(),
+ DatabaseDescriptor.getEndpointSnitch(),
+ ks_def.getStrategy_options());
+
// generate a meaningful error if the user setup keyspace and/or column definition incorrectly
for (CfDef cf : ks_def.cf_defs)
{