cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject git commit: Re-enable unknown options in compaction/replication strategy
Date Tue, 05 Mar 2013 10:42:37 GMT
Updated Branches:
  refs/heads/cassandra-1.2 2d16e2811 -> c315745c9


Re-enable unknown options in compaction/replication strategy

patch by slebresne; reviewed by xedin 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/c315745c
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/c315745c
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/c315745c

Branch: refs/heads/cassandra-1.2
Commit: c315745c92dcee3954dad830bf8756307e3c6bf5
Parents: 2d16e28
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Tue Mar 5 11:41:07 2013 +0100
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Tue Mar 5 11:42:29 2013 +0100

----------------------------------------------------------------------
 CHANGES.txt                                        |    2 +
 .../org/apache/cassandra/config/CFMetaData.java    |    4 +-
 .../org/apache/cassandra/config/KSMetaData.java    |    2 +-
 src/java/org/apache/cassandra/cql/CFPropDefs.java  |    2 +-
 src/java/org/apache/cassandra/cql3/CFPropDefs.java |    2 +-
 .../cql3/statements/AlterKeyspaceStatement.java    |   14 ++-
 .../cql3/statements/CreateKeyspaceStatement.java   |   14 ++-
 src/java/org/apache/cassandra/db/DefsTable.java    |   14 +--
 src/java/org/apache/cassandra/db/Table.java        |   11 +--
 .../locator/AbstractReplicationStrategy.java       |   84 ++++++++++++---
 .../apache/cassandra/locator/LocalStrategy.java    |    7 +-
 .../cassandra/locator/NetworkTopologyStrategy.java |    8 ++
 .../locator/OldNetworkTopologyStrategy.java        |    7 ++
 .../apache/cassandra/locator/SimpleStrategy.java   |    9 ++-
 .../apache/cassandra/thrift/CassandraServer.java   |   11 +--
 .../ReplicationStrategyEndpointCacheTest.java      |    2 +-
 16 files changed, 127 insertions(+), 66 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index a135262..f4e854b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -12,6 +12,8 @@
  * Include type arguments in Thrift CQLPreparedResult (CASSANDRA-5311)
  * Fix compaction not removing columns when bf_fp_ratio is 1 (CASSANDRA-5182)
  * cli: Warn about missing CQL3 tables in schema descriptions (CASSANDRA-5309)
+ * Re-enable unknown option in replication/compaction strategies option for
+   backward compatibility (CASSANDRA-4795)
 Merged from 1.1:
  * nodetool: ability to repair specific range (CASSANDRA-5280)
  * Fix possible assertion triggered in SliceFromReadCommand (CASSANDRA-5284)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/src/java/org/apache/cassandra/config/CFMetaData.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/CFMetaData.java b/src/java/org/apache/cassandra/config/CFMetaData.java
index ae91d23..18cdd93 100644
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@ -855,7 +855,7 @@ public final class CFMetaData
             throw new ConfigurationException("subcolumncomparators do not match or are note
compatible.");
     }
 
-    public static void validateCompactionOptions(Class<? extends AbstractCompactionStrategy>
strategyClass, Map<String, String> options) throws ConfigurationException
+    public static void validateCompactionOptions(Class<? extends AbstractCompactionStrategy>
strategyClass, Map<String, String> options, boolean checkUnexpected) throws ConfigurationException
     {
         try
         {
@@ -864,7 +864,7 @@ public final class CFMetaData
 
             Method validateMethod = strategyClass.getMethod("validateOptions", Map.class);
             Map<String, String> unknownOptions = (Map<String, String>) validateMethod.invoke(null,
options);
-            if (!unknownOptions.isEmpty())
+            if (checkUnexpected && !unknownOptions.isEmpty())
                 throw new ConfigurationException(String.format("Properties specified %s are
not understood by %s", unknownOptions.keySet(), strategyClass.getSimpleName()));
         }
         catch (NoSuchMethodException e)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/src/java/org/apache/cassandra/config/KSMetaData.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/KSMetaData.java b/src/java/org/apache/cassandra/config/KSMetaData.java
index 9522aa0..138e24b 100644
--- a/src/java/org/apache/cassandra/config/KSMetaData.java
+++ b/src/java/org/apache/cassandra/config/KSMetaData.java
@@ -206,7 +206,7 @@ public final class KSMetaData
         // Attempt to instantiate the ARS, which will throw a ConfigException if the strategy_options
aren't fully formed
         TokenMetadata tmd = StorageService.instance.getTokenMetadata();
         IEndpointSnitch eps = DatabaseDescriptor.getEndpointSnitch();
-        AbstractReplicationStrategy.createReplicationStrategy(name, strategyClass, tmd, eps,
strategyOptions);
+        AbstractReplicationStrategy.validateReplicationStrategyIgnoreUnexpected(name, strategyClass,
tmd, eps, strategyOptions);
 
         for (CFMetaData cfm : cfMetaData.values())
             cfm.validate();

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/src/java/org/apache/cassandra/cql/CFPropDefs.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql/CFPropDefs.java b/src/java/org/apache/cassandra/cql/CFPropDefs.java
index 7185b52..7178995 100644
--- a/src/java/org/apache/cassandra/cql/CFPropDefs.java
+++ b/src/java/org/apache/cassandra/cql/CFPropDefs.java
@@ -174,7 +174,7 @@ public class CFPropDefs {
                         CFMetaData.DEFAULT_MIN_COMPACTION_THRESHOLD));
         }
 
-        CFMetaData.validateCompactionOptions(compactionStrategyClass, compactionStrategyOptions);
+        CFMetaData.validateCompactionOptions(compactionStrategyClass, compactionStrategyOptions,
false);
     }
 
     /** Map a keyword to the corresponding value */

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/src/java/org/apache/cassandra/cql3/CFPropDefs.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/CFPropDefs.java b/src/java/org/apache/cassandra/cql3/CFPropDefs.java
index ccef5f8..8ad29fd 100644
--- a/src/java/org/apache/cassandra/cql3/CFPropDefs.java
+++ b/src/java/org/apache/cassandra/cql3/CFPropDefs.java
@@ -88,7 +88,7 @@ public class CFPropDefs extends PropertyDefinitions
             compactionStrategyClass = CFMetaData.createCompactionStrategy(strategy);
             compactionOptions.remove(COMPACTION_STRATEGY_CLASS_KEY);
 
-            CFMetaData.validateCompactionOptions(compactionStrategyClass, compactionOptions);
+            CFMetaData.validateCompactionOptions(compactionStrategyClass, compactionOptions,
true);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/src/java/org/apache/cassandra/cql3/statements/AlterKeyspaceStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterKeyspaceStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterKeyspaceStatement.java
index 52c422a..ef96997 100644
--- a/src/java/org/apache/cassandra/cql3/statements/AlterKeyspaceStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterKeyspaceStatement.java
@@ -72,12 +72,14 @@ public class AlterKeyspaceStatement extends SchemaAlteringStatement
         }
         else if (attrs.getReplicationStrategyClass() != null)
         {
-            // trial run to let ARS validate class + per-class options
-            AbstractReplicationStrategy.createReplicationStrategy(name,
-                                                                  AbstractReplicationStrategy.getClass(attrs.getReplicationStrategyClass()),
-                                                                  StorageService.instance.getTokenMetadata(),
-                                                                  DatabaseDescriptor.getEndpointSnitch(),
-                                                                  attrs.getReplicationOptions());
+            // The strategy is validated through KSMetaData.validate() in announceKeyspaceUpdate
below.
+            // However, for backward compatibility with thrift, this doesn't validate unexpected
options yet,
+            // so doing proper validation here.
+            AbstractReplicationStrategy.validateReplicationStrategy(name,
+                                                                    attrs.getReplicationStrategyClass(),
+                                                                    StorageService.instance.getTokenMetadata(),
+                                                                    DatabaseDescriptor.getEndpointSnitch(),
+                                                                    attrs.getReplicationOptions());
         }
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java
index 0a12f97..26e255d 100644
--- a/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java
@@ -87,12 +87,14 @@ public class CreateKeyspaceStatement extends SchemaAlteringStatement
         if (attrs.getReplicationStrategyClass() == null)
             throw new ConfigurationException("Missing mandatory replication strategy class");
 
-        // trial run to let ARS validate class + per-class options
-        AbstractReplicationStrategy.createReplicationStrategy(name,
-                                                              AbstractReplicationStrategy.getClass(attrs.getReplicationStrategyClass()),
-                                                              StorageService.instance.getTokenMetadata(),
-                                                              DatabaseDescriptor.getEndpointSnitch(),
-                                                              attrs.getReplicationOptions());
+        // The strategy is validated through KSMetaData.validate() in announceNewKeyspace
below.
+        // However, for backward compatibility with thrift, this doesn't validate unexpected
options yet,
+        // so doing proper validation here.
+        AbstractReplicationStrategy.validateReplicationStrategy(name,
+                                                                attrs.getReplicationStrategyClass(),
+                                                                StorageService.instance.getTokenMetadata(),
+                                                                DatabaseDescriptor.getEndpointSnitch(),
+                                                                attrs.getReplicationOptions());
     }
 
     public void announceMigration() throws RequestValidationException

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/src/java/org/apache/cassandra/db/DefsTable.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/DefsTable.java b/src/java/org/apache/cassandra/db/DefsTable.java
index 30614b7..914789b 100644
--- a/src/java/org/apache/cassandra/db/DefsTable.java
+++ b/src/java/org/apache/cassandra/db/DefsTable.java
@@ -532,18 +532,10 @@ public class DefsTable
 
         Schema.instance.setTableDefinition(newKsm);
 
-        try
-        {
-            if (!StorageService.instance.isClientMode())
-            {
-                Table.open(newState.name).createReplicationStrategy(newKsm);
-                MigrationManager.instance.notifyUpdateKeyspace(newKsm);
-            }
-        }
-        catch (ConfigurationException e)
+        if (!StorageService.instance.isClientMode())
         {
-            // It's too late to throw a configuration exception, we should have catch those
previously
-            throw new RuntimeException(e);
+            Table.open(newState.name).createReplicationStrategy(newKsm);
+            MigrationManager.instance.notifyUpdateKeyspace(newKsm);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/src/java/org/apache/cassandra/db/Table.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/Table.java b/src/java/org/apache/cassandra/db/Table.java
index d923081..3a73f43 100644
--- a/src/java/org/apache/cassandra/db/Table.java
+++ b/src/java/org/apache/cassandra/db/Table.java
@@ -261,14 +261,7 @@ public class Table
         name = table;
         KSMetaData ksm = Schema.instance.getKSMetaData(table);
         assert ksm != null : "Unknown keyspace " + table;
-        try
-        {
-            createReplicationStrategy(ksm);
-        }
-        catch (ConfigurationException e)
-        {
-            throw new RuntimeException(e);
-        }
+        createReplicationStrategy(ksm);
 
         indexLocks = new Object[DatabaseDescriptor.getConcurrentWriters() * 128];
         for (int i = 0; i < indexLocks.length; i++)
@@ -281,7 +274,7 @@ public class Table
         }
     }
 
-    public void createReplicationStrategy(KSMetaData ksm) throws ConfigurationException
+    public void createReplicationStrategy(KSMetaData ksm)
     {
         if (replicationStrategy != null)
             StorageService.instance.getTokenMetadata().unregister(replicationStrategy);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/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 4b54d94..5ebebcd 100644
--- a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java
+++ b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java
@@ -211,12 +211,23 @@ public abstract class AbstractReplicationStrategy
 
     public abstract void validateOptions() throws ConfigurationException;
 
-    public static AbstractReplicationStrategy createReplicationStrategy(String table,
-                                                                        Class<? extends
AbstractReplicationStrategy> strategyClass,
-                                                                        TokenMetadata tokenMetadata,
-                                                                        IEndpointSnitch snitch,
-                                                                        Map<String, String>
strategyOptions)
-            throws ConfigurationException
+    /*
+     * The options recognized by the strategy.
+     * The empty collection means that no options are accepted, but null means
+     * that any option is accepted.
+     */
+    public Collection<String> recognizedOptions()
+    {
+        // We default to null for backward compatibility sake
+        return null;
+    }
+
+    private static AbstractReplicationStrategy createInternal(String table,
+                                                              Class<? extends AbstractReplicationStrategy>
strategyClass,
+                                                              TokenMetadata tokenMetadata,
+                                                              IEndpointSnitch snitch,
+                                                              Map<String, String> strategyOptions)
+        throws ConfigurationException
     {
         AbstractReplicationStrategy strategy;
         Class [] parameterTypes = new Class[] {String.class, TokenMetadata.class, IEndpointSnitch.class,
Map.class};
@@ -227,24 +238,61 @@ public abstract class AbstractReplicationStrategy
         }
         catch (Exception e)
         {
-            throw new RuntimeException(e);
+            throw new ConfigurationException("Error constructing replication strategy class",
e);
         }
-
-        // Throws Config Exception if strat_opts don't contain required info
-        strategy.validateOptions();
-
         return strategy;
     }
 
     public static AbstractReplicationStrategy createReplicationStrategy(String table,
-                                                                        String strategyClassName,
+                                                                        Class<? extends
AbstractReplicationStrategy> strategyClass,
                                                                         TokenMetadata tokenMetadata,
                                                                         IEndpointSnitch snitch,
                                                                         Map<String, String>
strategyOptions)
-            throws ConfigurationException
     {
-        Class<AbstractReplicationStrategy> c = getClass(strategyClassName);
-        return createReplicationStrategy(table, c, tokenMetadata, snitch, strategyOptions);
+        try
+        {
+            AbstractReplicationStrategy strategy = createInternal(table, strategyClass, tokenMetadata,
snitch, strategyOptions);
+
+            // Because we used to not properly validate unrecognized options, we only log
a warning if we find one.
+            try
+            {
+                strategy.validateExpectedOptions();
+            }
+            catch (ConfigurationException e)
+            {
+                logger.warn("Ignoring {}", e.getMessage());
+            }
+
+            strategy.validateOptions();
+            return strategy;
+        }
+        catch (ConfigurationException e)
+        {
+            // If that happens at this point, there is nothing we can do about it.
+            throw new RuntimeException();
+        }
+    }
+
+    public static void validateReplicationStrategy(String table,
+                                                   String strategyClassName,
+                                                   TokenMetadata tokenMetadata,
+                                                   IEndpointSnitch snitch,
+                                                   Map<String, String> strategyOptions)
throws ConfigurationException
+    {
+        AbstractReplicationStrategy strategy = createInternal(table, getClass(strategyClassName),
tokenMetadata, snitch, strategyOptions);
+        strategy.validateExpectedOptions();
+        strategy.validateOptions();
+    }
+
+    // For backward compatibility sake on the thrift side
+    public static void validateReplicationStrategyIgnoreUnexpected(String table,
+                                                                   Class<? extends AbstractReplicationStrategy>
strategyClass,
+                                                                   TokenMetadata tokenMetadata,
+                                                                   IEndpointSnitch snitch,
+                                                                   Map<String, String>
strategyOptions) throws ConfigurationException
+    {
+        AbstractReplicationStrategy strategy = createInternal(table, strategyClass, tokenMetadata,
snitch, strategyOptions);
+        strategy.validateOptions();
     }
 
     public static Class<AbstractReplicationStrategy> getClass(String cls) throws ConfigurationException
@@ -273,8 +321,12 @@ public abstract class AbstractReplicationStrategy
         }
     }
 
-    protected void validateExpectedOptions(Collection<String> expectedOptions) throws
ConfigurationException
+    private void validateExpectedOptions() throws ConfigurationException
     {
+        Collection expectedOptions = recognizedOptions();
+        if (expectedOptions == null)
+            return;
+
         for (String key : configOptions.keySet())
         {
             if (!expectedOptions.contains(key))

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/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 ab580f1..0e95820 100644
--- a/src/java/org/apache/cassandra/locator/LocalStrategy.java
+++ b/src/java/org/apache/cassandra/locator/LocalStrategy.java
@@ -20,6 +20,7 @@ package org.apache.cassandra.locator;
 import java.net.InetAddress;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
@@ -61,7 +62,11 @@ public class LocalStrategy extends AbstractReplicationStrategy
 
     public void validateOptions() throws ConfigurationException
     {
+    }
+
+    public Collection<String> recognizedOptions()
+    {
         // LocalStrategy doesn't expect any options.
-        validateExpectedOptions(Collections.<String>emptySet());
+        return Collections.<String>emptySet();
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/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 ad43197..c64c792 100644
--- a/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java
+++ b/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java
@@ -188,7 +188,15 @@ public class NetworkTopologyStrategy extends AbstractReplicationStrategy
     {
         for (Entry<String, String> e : this.configOptions.entrySet())
         {
+            if (e.getKey().equalsIgnoreCase("replication_factor"))
+                throw new ConfigurationException("replication_factor is an option for SimpleStrategy,
not NetworkTopologyStrategy");
             validateReplicationFactor(e.getValue());
         }
     }
+
+    public Collection<String> recognizedOptions()
+    {
+        // We explicitely allow all options
+        return null;
+    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/src/java/org/apache/cassandra/locator/OldNetworkTopologyStrategy.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/locator/OldNetworkTopologyStrategy.java b/src/java/org/apache/cassandra/locator/OldNetworkTopologyStrategy.java
index 2e3e3e8..a46197e 100644
--- a/src/java/org/apache/cassandra/locator/OldNetworkTopologyStrategy.java
+++ b/src/java/org/apache/cassandra/locator/OldNetworkTopologyStrategy.java
@@ -19,6 +19,8 @@ package org.apache.cassandra.locator;
 
 import java.net.InetAddress;
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -113,4 +115,9 @@ public class OldNetworkTopologyStrategy extends AbstractReplicationStrategy
         }
         validateReplicationFactor(configOptions.get("replication_factor"));
     }
+
+    public Collection<String> recognizedOptions()
+    {
+        return Collections.<String>singleton("replication_factor");
+    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/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 17d171e..c310875 100644
--- a/src/java/org/apache/cassandra/locator/SimpleStrategy.java
+++ b/src/java/org/apache/cassandra/locator/SimpleStrategy.java
@@ -19,7 +19,8 @@ package org.apache.cassandra.locator;
 
 import java.net.InetAddress;
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -69,10 +70,14 @@ public class SimpleStrategy extends AbstractReplicationStrategy
 
     public void validateOptions() throws ConfigurationException
     {
-        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.");
         validateReplicationFactor(rf);
     }
+
+    public Collection<String> recognizedOptions()
+    {
+        return Collections.<String>singleton("replication_factor");
+    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/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 7edaf2b..7002436 100644
--- a/src/java/org/apache/cassandra/thrift/CassandraServer.java
+++ b/src/java/org/apache/cassandra/thrift/CassandraServer.java
@@ -1295,7 +1295,7 @@ public class CassandraServer implements Cassandra.Iface
             cState.hasKeyspaceAccess(keyspace, Permission.CREATE);
             cf_def.unsetId(); // explicitly ignore any id set by client (Hector likes to
set zero)
             CFMetaData cfm = CFMetaData.fromThrift(cf_def);
-            CFMetaData.validateCompactionOptions(cfm.compactionStrategyClass, cfm.compactionStrategyOptions);
+            CFMetaData.validateCompactionOptions(cfm.compactionStrategyClass, cfm.compactionStrategyOptions,
false);
 
             cfm.addDefaultIndexNames();
             MigrationManager.announceNewColumnFamily(cfm);
@@ -1338,13 +1338,6 @@ 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)
             {
@@ -1432,7 +1425,7 @@ public class CassandraServer implements Cassandra.Iface
 
             CFMetaData.applyImplicitDefaults(cf_def);
             CFMetaData cfm = CFMetaData.fromThrift(cf_def);
-            CFMetaData.validateCompactionOptions(cfm.compactionStrategyClass, cfm.compactionStrategyOptions);
+            CFMetaData.validateCompactionOptions(cfm.compactionStrategyClass, cfm.compactionStrategyOptions,
false);
             cfm.addDefaultIndexNames();
             MigrationManager.announceColumnFamilyUpdate(cfm);
             return Schema.instance.getVersion().toString();

http://git-wip-us.apache.org/repos/asf/cassandra/blob/c315745c/test/unit/org/apache/cassandra/locator/ReplicationStrategyEndpointCacheTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/locator/ReplicationStrategyEndpointCacheTest.java
b/test/unit/org/apache/cassandra/locator/ReplicationStrategyEndpointCacheTest.java
index 8212468..277677f 100644
--- a/test/unit/org/apache/cassandra/locator/ReplicationStrategyEndpointCacheTest.java
+++ b/test/unit/org/apache/cassandra/locator/ReplicationStrategyEndpointCacheTest.java
@@ -165,7 +165,7 @@ public class ReplicationStrategyEndpointCacheTest extends SchemaLoader
     {
         return AbstractReplicationStrategy.createReplicationStrategy(
                 strategy.tableName,
-                strategy.getClass().getName(),
+                AbstractReplicationStrategy.getClass(strategy.getClass().getName()),
                 newTmd,
                 strategy.snitch,
                 strategy.configOptions);


Mime
View raw message