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/trunk
Commit: c315745c92dcee3954dad830bf8756307e3c6bf5
Parents: 2d16e28
Author: Sylvain Lebresne <[email protected]>
Authored: Tue Mar 5 11:41:07 2013 +0100
Committer: Sylvain Lebresne <[email protected]>
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);

Reply via email to