You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by gd...@apache.org on 2010/11/19 16:28:01 UTC

svn commit: r1036894 - in /cassandra/trunk: src/avro/ src/java/org/apache/cassandra/avro/ src/java/org/apache/cassandra/config/ src/java/org/apache/cassandra/db/ src/java/org/apache/cassandra/db/migration/ src/java/org/apache/cassandra/service/ src/jav...

Author: gdusbabek
Date: Fri Nov 19 15:28:01 2010
New Revision: 1036894

URL: http://svn.apache.org/viewvc?rev=1036894&view=rev
Log:
updateColumnFamily uses reload, remove unneccesary structures, fix bugs. patch by gdusbabek, reviewe by jbellis. CASSANDRA-1715

Modified:
    cassandra/trunk/src/avro/internode.genavro
    cassandra/trunk/src/java/org/apache/cassandra/avro/CassandraServer.java
    cassandra/trunk/src/java/org/apache/cassandra/config/CFMetaData.java
    cassandra/trunk/src/java/org/apache/cassandra/config/ColumnDefinition.java
    cassandra/trunk/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
    cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
    cassandra/trunk/src/java/org/apache/cassandra/db/migration/UpdateColumnFamily.java
    cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java
    cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java
    cassandra/trunk/test/unit/org/apache/cassandra/db/DefsTest.java

Modified: cassandra/trunk/src/avro/internode.genavro
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/avro/internode.genavro?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/src/avro/internode.genavro (original)
+++ cassandra/trunk/src/avro/internode.genavro Fri Nov 19 15:28:01 2010
@@ -73,8 +73,7 @@ protocol InterNode {
     
     @namespace("org.apache.cassandra.db.migration.avro")
     record UpdateColumnFamily {
-        org.apache.cassandra.avro.CfDef oldCf;
-        org.apache.cassandra.avro.CfDef newCf;
+        org.apache.cassandra.avro.CfDef metadata;
     }
 
     @namespace("org.apache.cassandra.db.migration.avro")

Modified: cassandra/trunk/src/java/org/apache/cassandra/avro/CassandraServer.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/avro/CassandraServer.java?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/avro/CassandraServer.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/avro/CassandraServer.java Fri Nov 19 15:28:01 2010
@@ -700,8 +700,8 @@ public class CassandraServer implements 
         
         try
         {
-            CFMetaData newCfm = oldCfm.apply(cf_def);
-            UpdateColumnFamily update = new UpdateColumnFamily(oldCfm, newCfm);
+            oldCfm.apply(cf_def);
+            UpdateColumnFamily update = new UpdateColumnFamily(CFMetaData.convertToThrift(cf_def));
             applyMigrationOnStage(update);
             return DatabaseDescriptor.getDefsVersion().toString();
         }

Modified: cassandra/trunk/src/java/org/apache/cassandra/config/CFMetaData.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/config/CFMetaData.java?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/config/CFMetaData.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/config/CFMetaData.java Fri Nov 19 15:28:01 2010
@@ -21,9 +21,10 @@ package org.apache.cassandra.config;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -275,7 +276,7 @@ public final class CFMetaData
     public static CFMetaData newIndexMetadata(String table, String parentCf, ColumnDefinition info, AbstractType columnComparator)
     {
         return new CFMetaData(table,
-                              parentCf + "." + (info.index_name == null ? FBUtilities.bytesToHex(info.name) : info.index_name),
+                              parentCf + "." + (info.getIndexName() == null ? FBUtilities.bytesToHex(info.name) : info.getIndexName()),
                               ColumnFamilyType.Standard,
                               columnComparator,
                               null,
@@ -582,55 +583,13 @@ public final class CFMetaData
         return validator;
     }
     
-    public CFMetaData apply(org.apache.cassandra.avro.CfDef cf_def) throws ConfigurationException
+    public void apply(org.apache.cassandra.avro.CfDef cf_def) throws ConfigurationException
     {
-        // validate.
-        if (!cf_def.id.equals(cfId))
-            throw new ConfigurationException(String.format("ids do not match. %d, %d", cf_def.id, cfId));
-        if (!cf_def.keyspace.toString().equals(tableName))
-            throw new ConfigurationException(String.format("keyspaces do not match. %s, %s", cf_def.keyspace, tableName));
-        if (!cf_def.name.toString().equals(cfName))
-            throw new ConfigurationException("names do not match.");
-        if (!cf_def.column_type.toString().equals(cfType.name()))
-            throw new ConfigurationException("types do not match.");
-        if (comparator != DatabaseDescriptor.getComparator(cf_def.comparator_type.toString()))
-            throw new ConfigurationException("comparators do not match.");
-        if (cf_def.subcomparator_type == null || cf_def.subcomparator_type.equals(""))
-        {
-            if (subcolumnComparator != null)
-                throw new ConfigurationException("subcolumncomparators do not match.");
-            // else, it's null and we're good.
-        }
-        else if (subcolumnComparator != DatabaseDescriptor.getComparator(cf_def.subcomparator_type.toString()))
-            throw new ConfigurationException("subcolumncomparators do not match.");
-
-        validateMinMaxCompactionThresholds(cf_def);
-        validateMemtableSettings(cf_def);
-        
-        return new CFMetaData(tableName, 
-                              cfName, 
-                              cfType, 
-                              comparator, 
-                              subcolumnComparator, 
-                              cf_def.comment == null ? "" : cf_def.comment.toString(), 
-                              cf_def.row_cache_size, 
-                              cf_def.key_cache_size,
-                              cf_def.read_repair_chance, 
-                              cf_def.gc_grace_seconds, 
-                              DatabaseDescriptor.getComparator(cf_def.default_validation_class == null ? null : cf_def.default_validation_class.toString()),
-                              cf_def.min_compaction_threshold,
-                              cf_def.max_compaction_threshold,
-                              cf_def.row_cache_save_period_in_seconds,
-                              cf_def.key_cache_save_period_in_seconds,
-                              cf_def.memtable_flush_after_mins,
-                              cf_def.memtable_throughput_in_mb,
-                              cf_def.memtable_operations_in_millions,
-                              cfId,
-                              column_metadata);
+        apply(convertToThrift(cf_def));
     }
     
     // merges some final fields from this CFM with modifiable fields from CfDef into a new CFMetaData.
-    public CFMetaData apply(org.apache.cassandra.thrift.CfDef cf_def) throws ConfigurationException
+    public void apply(org.apache.cassandra.thrift.CfDef cf_def) throws ConfigurationException
     {
         // validate
         if (cf_def.id != cfId)
@@ -655,40 +614,49 @@ public final class CFMetaData
         validateMinMaxCompactionThresholds(cf_def);
         validateMemtableSettings(cf_def);
 
-        Map<ByteBuffer, ColumnDefinition> metadata = new HashMap<ByteBuffer, ColumnDefinition>();
-        if (cf_def.column_metadata == null)
+        comment = cf_def.comment == null ? "" : cf_def.comment;
+        rowCacheSize = cf_def.row_cache_size;
+        keyCacheSize = cf_def.key_cache_size;
+        readRepairChance = cf_def.read_repair_chance;
+        gcGraceSeconds = cf_def.gc_grace_seconds;
+        defaultValidator = DatabaseDescriptor.getComparator(cf_def.default_validation_class);
+        minCompactionThreshold = cf_def.min_compaction_threshold;
+        maxCompactionThreshold = cf_def.max_compaction_threshold;
+        rowCacheSavePeriodInSeconds = cf_def.row_cache_save_period_in_seconds;
+        keyCacheSavePeriodInSeconds = cf_def.key_cache_save_period_in_seconds;
+        memtableFlushAfterMins = cf_def.memtable_flush_after_mins;
+        memtableThroughputInMb = cf_def.memtable_throughput_in_mb;
+        memtableOperationsInMillions = cf_def.memtable_operations_in_millions;
+        
+        // adjust secondary indexes. figure out who is coming and going.
+        Set<ByteBuffer> toRemove = new HashSet<ByteBuffer>();
+        Set<ByteBuffer> newIndexNames = new HashSet<ByteBuffer>();
+        Set<org.apache.cassandra.thrift.ColumnDef> toAdd = new HashSet<org.apache.cassandra.thrift.ColumnDef>();
+        for (org.apache.cassandra.thrift.ColumnDef def : cf_def.column_metadata)
+        {
+            newIndexNames.add(def.name);
+            if (!column_metadata.containsKey(def.name))
+                toAdd.add(def);
+        }
+        for (ByteBuffer indexName : column_metadata.keySet())
+            if (!newIndexNames.contains(indexName))
+                toRemove.add(indexName);
+        
+        // remove the ones leaving.
+        for (ByteBuffer indexName : toRemove)
+            column_metadata.remove(indexName);
+        // update the ones staying
+        for (org.apache.cassandra.thrift.ColumnDef def : cf_def.column_metadata)
         {
-            metadata = column_metadata;
+            column_metadata.get(def.name).setIndexType(def.index_type);
+            column_metadata.get(def.name).setIndexName(def.index_name);
         }
-        else
+        // add the new ones coming in.
+        for (org.apache.cassandra.thrift.ColumnDef def : toAdd)
         {
-            for (org.apache.cassandra.thrift.ColumnDef def : cf_def.column_metadata)
-            {
-                ColumnDefinition cd = new ColumnDefinition(def.name, def.validation_class, def.index_type, def.index_name);
-                metadata.put(cd.name, cd);
-            }
+            ColumnDefinition cd = new ColumnDefinition(def.name, def.validation_class, def.index_type, def.index_name);
+            column_metadata.put(cd.name, cd);
         }
-
-        return new CFMetaData(tableName, 
-                              cfName, 
-                              cfType, 
-                              comparator, 
-                              subcolumnComparator, 
-                              cf_def.comment, 
-                              cf_def.row_cache_size, 
-                              cf_def.key_cache_size,
-                              cf_def.read_repair_chance, 
-                              cf_def.gc_grace_seconds, 
-                              DatabaseDescriptor.getComparator(cf_def.default_validation_class == null ? null : cf_def.default_validation_class),
-                              cf_def.min_compaction_threshold,
-                              cf_def.max_compaction_threshold,
-                              cf_def.row_cache_save_period_in_seconds,
-                              cf_def.key_cache_save_period_in_seconds,
-                              cf_def.memtable_flush_after_mins,
-                              cf_def.memtable_throughput_in_mb,
-                              cf_def.memtable_operations_in_millions,
-                              cfId,
-                              metadata);
     }
     
     // converts CFM to thrift CfDef
@@ -720,8 +688,8 @@ public final class CFMetaData
         for (ColumnDefinition cd : cfm.column_metadata.values())
         {
             org.apache.cassandra.thrift.ColumnDef tcd = new org.apache.cassandra.thrift.ColumnDef();
-            tcd.setIndex_name(cd.index_name);
-            tcd.setIndex_type(cd.index_type);
+            tcd.setIndex_name(cd.getIndexName());
+            tcd.setIndex_type(cd.getIndexType());
             tcd.setName(cd.name);
             tcd.setValidation_class(cd.validator.getClass().getName());
             column_meta.add(tcd);
@@ -761,8 +729,8 @@ public final class CFMetaData
         for (ColumnDefinition cd : cfm.column_metadata.values())
         {
             org.apache.cassandra.avro.ColumnDef tcd = new org.apache.cassandra.avro.ColumnDef();
-            tcd.index_name = cd.index_name;
-            tcd.index_type = org.apache.cassandra.avro.IndexType.valueOf(cd.index_type.name());
+            tcd.index_name = cd.getIndexName();
+            tcd.index_type = org.apache.cassandra.avro.IndexType.valueOf(cd.getIndexType().name());
             tcd.name = cd.name;
             tcd.validation_class = cd.validator.getClass().getName();
             column_meta.add(tcd);
@@ -770,6 +738,46 @@ public final class CFMetaData
         def.column_metadata = column_meta;   
         return def;
     }
+    
+    private static String stringOrNull(CharSequence cs)
+    {
+        return cs == null ? null : cs.toString();
+    }
+    
+    public static org.apache.cassandra.thrift.CfDef convertToThrift(org.apache.cassandra.avro.CfDef def)
+    {
+        org.apache.cassandra.thrift.CfDef newDef = new org.apache.cassandra.thrift.CfDef(def.keyspace.toString(), def.name.toString());
+        newDef.setColumn_type(def.column_type.toString());
+        newDef.setComment(def.comment.toString());
+        newDef.setComparator_type(stringOrNull(def.comparator_type));
+        newDef.setDefault_validation_class(stringOrNull(def.default_validation_class));
+        newDef.setGc_grace_seconds(def.gc_grace_seconds);
+        newDef.setId(def.id);
+        newDef.setKey_cache_save_period_in_seconds(def.key_cache_save_period_in_seconds);
+        newDef.setKey_cache_size(def.key_cache_size);
+        newDef.setKeyspace(def.keyspace.toString());
+        newDef.setMax_compaction_threshold(def.max_compaction_threshold);
+        newDef.setMemtable_flush_after_mins(def.memtable_flush_after_mins);
+        newDef.setMemtable_operations_in_millions(def.memtable_operations_in_millions);
+        newDef.setMemtable_throughput_in_mb(def.memtable_throughput_in_mb);
+        newDef.setMin_compaction_threshold(def.min_compaction_threshold);
+        newDef.setName(def.name.toString());
+        newDef.setRead_repair_chance(def.read_repair_chance);
+        newDef.setRow_cache_save_period_in_seconds(def.row_cache_save_period_in_seconds);
+        newDef.setRow_cache_size(def.row_cache_size);
+        newDef.setSubcomparator_type(stringOrNull(def.subcomparator_type));
+        List<org.apache.cassandra.thrift.ColumnDef> columnMeta = new ArrayList<org.apache.cassandra.thrift.ColumnDef>();
+        for (org.apache.cassandra.avro.ColumnDef cdef : def.column_metadata)
+        {
+            org.apache.cassandra.thrift.ColumnDef tdef = new org.apache.cassandra.thrift.ColumnDef(cdef.name, stringOrNull(cdef.validation_class));
+            tdef.setIndex_name(stringOrNull(cdef.index_name));
+            if (cdef.index_type != null)
+                tdef.setIndex_type(org.apache.cassandra.thrift.IndexType.valueOf(cdef.index_type.name()));
+            columnMeta.add(tdef);
+        }
+        newDef.setColumn_metadata(columnMeta);
+        return newDef;
+    }
 
     public static void validateMinMaxCompactionThresholds(org.apache.cassandra.thrift.CfDef cf_def) throws ConfigurationException
     {

Modified: cassandra/trunk/src/java/org/apache/cassandra/config/ColumnDefinition.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/config/ColumnDefinition.java?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/config/ColumnDefinition.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/config/ColumnDefinition.java Fri Nov 19 15:28:01 2010
@@ -36,8 +36,8 @@ import org.apache.cassandra.utils.FBUtil
 public class ColumnDefinition {
     public final ByteBuffer name;
     public final AbstractType validator;
-    public final IndexType index_type;
-    public final String index_name;
+    private IndexType index_type;
+    private String index_name;
 
     public ColumnDefinition(ByteBuffer name, String validation_class, IndexType index_type, String index_name) throws ConfigurationException
     {
@@ -152,4 +152,25 @@ public class ColumnDefinition {
                ", index_name='" + index_name + '\'' +
                '}';
     }
+
+    public String getIndexName()
+    {
+        return index_name;
+    }
+    
+    public void setIndexName(String s)
+    {
+        index_name = s;
+    }
+
+
+    public IndexType getIndexType()
+    {
+        return index_type;
+    }
+
+    public void setIndexType(IndexType index_type)
+    {
+        this.index_type = index_type;
+    }
 }

Modified: cassandra/trunk/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/config/DatabaseDescriptor.java?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/config/DatabaseDescriptor.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/config/DatabaseDescriptor.java Fri Nov 19 15:28:01 2010
@@ -968,7 +968,8 @@ public class    DatabaseDescriptor
     // process of mutating an individual keyspace, rather than setting manually here.
     public static void setTableDefinition(KSMetaData ksm, UUID newVersion)
     {
-        tables.put(ksm.name, ksm);
+        if (ksm != null)
+            tables.put(ksm.name, ksm);
         DatabaseDescriptor.defsVersion = newVersion;
     }
     

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java Fri Nov 19 15:28:01 2010
@@ -185,13 +185,23 @@ public class ColumnFamilyStore implement
             throw new IOError(ex.getCause());
         }
         
-        // todo: update cache sizes, etc. see SSTableTracker
+        ssTables.updateCacheSizes();
         
-        // drop indexes no longer needed
+        // figure out what needs to be added and dropped.
         Set<ByteBuffer> indexesToDrop = new HashSet<ByteBuffer>();
+        Set<ColumnDefinition> indexesToAdd = new HashSet<ColumnDefinition>();
+        
+        for (ColumnDefinition cdef : metadata.getColumn_metadata().values())
+        {
+            if (!indexedColumns.containsKey(cdef.name))
+                indexesToAdd.add(cdef);
+        }
         for (ByteBuffer indexName : indexedColumns.keySet())
-               if (!metadata.getColumn_metadata().containsKey(indexName))
-                   indexesToDrop.add(indexName);
+        {
+            if (!metadata.getColumn_metadata().containsKey(indexName))
+                indexesToDrop.add(indexName);
+        }
+        // drop indexes no longer needed.
         for (ByteBuffer indexName : indexesToDrop)
         {
             ColumnFamilyStore indexCfs = indexedColumns.remove(indexName);
@@ -199,14 +209,10 @@ public class ColumnFamilyStore implement
             SystemTable.setIndexRemoved(metadata.tableName, metadata.cfName);
             indexCfs.removeAllSSTables();
         }
-        
-        // there isn't a valid way to update existing indexes at this point (nothing you can change),
-        // so don't bother with them.
-        
-        // add indexes that are new
-        for (Map.Entry<ByteBuffer, ColumnDefinition> entry : metadata.getColumn_metadata().entrySet())
-            if (!indexedColumns.containsKey(entry.getKey()) && entry.getValue().index_type != null)
-                addIndex(entry.getValue());
+        // add new indexes.
+        for (ColumnDefinition info : indexesToAdd)
+            if (info.getIndexType() != null)
+                addIndex(info);
     }
 
     private ColumnFamilyStore(Table table, String columnFamilyName, IPartitioner partitioner, int generation, CFMetaData metadata)
@@ -258,7 +264,7 @@ public class ColumnFamilyStore implement
         indexedColumns = new ConcurrentSkipListMap<ByteBuffer, ColumnFamilyStore>(getComparator());
         for (ColumnDefinition info : metadata.getColumn_metadata().values())
         {
-            if (info.index_type != null)
+            if (info.getIndexType() != null)
                 addIndex(info);
         }
 
@@ -311,7 +317,7 @@ public class ColumnFamilyStore implement
 
     public void addIndex(final ColumnDefinition info)
     {
-        assert info.index_type != null;
+        assert info.getIndexType() != null;
         IPartitioner rowPartitioner = StorageService.getPartitioner();
         AbstractType columnComparator = (rowPartitioner instanceof OrderPreservingPartitioner || rowPartitioner instanceof ByteOrderedPartitioner)
                                         ? BytesType.instance

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/migration/UpdateColumnFamily.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/migration/UpdateColumnFamily.java?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/migration/UpdateColumnFamily.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/migration/UpdateColumnFamily.java Fri Nov 19 15:28:01 2010
@@ -14,6 +14,7 @@ import org.apache.cassandra.config.KSMet
 import org.apache.cassandra.db.ColumnFamilyStore;
 import org.apache.cassandra.db.SystemTable;
 import org.apache.cassandra.db.Table;
+import org.apache.cassandra.thrift.CfDef;
 import org.apache.cassandra.utils.FBUtilities;
 import org.apache.cassandra.utils.UUIDGen;
 
@@ -38,41 +39,32 @@ import org.apache.cassandra.utils.UUIDGe
 /** todo: doesn't work with secondary indices yet. See CASSANDRA-1415. */
 public class UpdateColumnFamily extends Migration
 {
-    private CFMetaData oldCfm;
-    private CFMetaData newCfm;
+    private CFMetaData metadata;
     
     protected UpdateColumnFamily() { }
     
     /** assumes validation has already happened. That is, replacing oldCfm with newCfm is neither illegal or totally whackass. */
-    public UpdateColumnFamily(CFMetaData oldCfm, CFMetaData newCfm) throws ConfigurationException, IOException
+    public UpdateColumnFamily(CfDef cf_def) throws ConfigurationException, IOException
     {
         super(UUIDGen.makeType1UUIDFromHost(FBUtilities.getLocalAddress()), DatabaseDescriptor.getDefsVersion());
         
-        KSMetaData ksm = DatabaseDescriptor.getTableDefinition(newCfm.tableName);
+        KSMetaData ksm = DatabaseDescriptor.getTableDefinition(cf_def.keyspace);
         if (ksm == null)
             throw new ConfigurationException("Keyspace does not already exist.");
         
-        this.oldCfm = oldCfm;
-        this.newCfm = newCfm;
+        CFMetaData oldCfm = DatabaseDescriptor.getCFMetaData(CFMetaData.getId(cf_def.keyspace, cf_def.name));
+        oldCfm.apply(cf_def); 
+        this.metadata = oldCfm;
         
         // clone ksm but include the new cf def.
-        KSMetaData newKsm = makeNewKeyspaceDefinition(ksm);
-        rm = Migration.makeDefinitionMutation(newKsm, null, newVersion);
-    }
-    
-    private KSMetaData makeNewKeyspaceDefinition(KSMetaData ksm)
-    {
-        List<CFMetaData> newCfs = new ArrayList<CFMetaData>(ksm.cfMetaData().values());
-        newCfs.remove(oldCfm);
-        newCfs.add(newCfm);
-        return new KSMetaData(ksm.name, ksm.strategyClass, ksm.strategyOptions, ksm.replicationFactor, newCfs.toArray(new CFMetaData[newCfs.size()]));
+        rm = Migration.makeDefinitionMutation(ksm, null, newVersion);
     }
     
     public void beforeApplyModels()
     {
         if (clientMode)
             return;
-        ColumnFamilyStore cfs = Table.open(oldCfm.tableName).getColumnFamilyStore(oldCfm.cfName);
+        ColumnFamilyStore cfs = Table.open(metadata.tableName).getColumnFamilyStore(metadata.cfName);
         cfs.snapshot(Table.getTimestampedSnapshotName(null));
     }
 
@@ -81,29 +73,16 @@ public class UpdateColumnFamily extends 
         acquireLocks();
         try
         {
-            logger.debug("Updating " + oldCfm + " to " + newCfm);
-            KSMetaData newKsm = makeNewKeyspaceDefinition(DatabaseDescriptor.getTableDefinition(newCfm.tableName));
-            DatabaseDescriptor.setTableDefinition(newKsm, newVersion);
+            logger.debug("Updating " + metadata + " to " + metadata);
+            
+            DatabaseDescriptor.setTableDefinition(null, newVersion);
             
             if (!clientMode)
             {
-                Table table = Table.open(oldCfm.tableName);
-                ColumnFamilyStore oldCfs = table.getColumnFamilyStore(oldCfm.cfName);
-                table.reloadCf(newCfm.cfId);
-    
-                // clean up obsolete index data files
-                for (Map.Entry<ByteBuffer, ColumnDefinition> entry : oldCfm.getColumn_metadata().entrySet())
-                {
-                    ByteBuffer column = entry.getKey();
-                    ColumnDefinition def = entry.getValue();
-                    if (def.index_type != null
-                        && (!newCfm.getColumn_metadata().containsKey(column) || newCfm.getColumn_metadata().get(column).index_type == null))
-                    {
-                        ColumnFamilyStore indexCfs = oldCfs.getIndexedColumnFamilyStore(column);
-                        SystemTable.setIndexRemoved(table.name, indexCfs.columnFamily);
-                        indexCfs.removeAllSSTables();
-                    }
-                }
+                Table table = Table.open(metadata.tableName);
+                
+                ColumnFamilyStore oldCfs = table.getColumnFamilyStore(metadata.cfName);
+                oldCfs.reload();
             }
         }
         finally
@@ -115,15 +94,13 @@ public class UpdateColumnFamily extends 
     public void subdeflate(org.apache.cassandra.db.migration.avro.Migration mi)
     {
         org.apache.cassandra.db.migration.avro.UpdateColumnFamily update = new org.apache.cassandra.db.migration.avro.UpdateColumnFamily();
-        update.newCf = newCfm.deflate();
-        update.oldCf = oldCfm.deflate();
+        update.metadata = metadata.deflate();
         mi.migration = update;
     }
 
     public void subinflate(org.apache.cassandra.db.migration.avro.Migration mi)
     {
         org.apache.cassandra.db.migration.avro.UpdateColumnFamily update = (org.apache.cassandra.db.migration.avro.UpdateColumnFamily)mi.migration;
-        newCfm = CFMetaData.inflate(update.newCf);
-        oldCfm = CFMetaData.inflate(update.oldCf);
+        metadata = CFMetaData.inflate(update.metadata);
     }
 }

Modified: cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/service/StorageService.java Fri Nov 19 15:28:01 2010
@@ -1995,8 +1995,8 @@ public class StorageService implements I
                 for (ColumnDefinition cd : cfm.getColumn_metadata().values())
                 {
                     RawColumnDefinition rcd = new RawColumnDefinition();
-                    rcd.index_name = cd.index_name;
-                    rcd.index_type = cd.index_type;
+                    rcd.index_name = cd.getIndexName();
+                    rcd.index_type = cd.getIndexType();
                     rcd.name = ByteBufferUtil.string(cd.name, Charsets.UTF_8);
                     rcd.validator_class = cd.validator.getClass().getName();
                     rcf.column_metadata[j++] = rcd;

Modified: cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java Fri Nov 19 15:28:01 2010
@@ -857,8 +857,8 @@ public class CassandraServer implements 
         
         try
         {
-            CFMetaData newCfm = oldCfm.apply(cf_def);
-            UpdateColumnFamily update = new UpdateColumnFamily(oldCfm, newCfm);
+            // ideally, apply() would happen on the stage with the
+            UpdateColumnFamily update = new UpdateColumnFamily(cf_def);
             applyMigrationOnStage(update);
             return DatabaseDescriptor.getDefsVersion().toString();
         }

Modified: cassandra/trunk/test/unit/org/apache/cassandra/db/DefsTest.java
URL: http://svn.apache.org/viewvc/cassandra/trunk/test/unit/org/apache/cassandra/db/DefsTest.java?rev=1036894&r1=1036893&r2=1036894&view=diff
==============================================================================
--- cassandra/trunk/test/unit/org/apache/cassandra/db/DefsTest.java (original)
+++ cassandra/trunk/test/unit/org/apache/cassandra/db/DefsTest.java Fri Nov 19 15:28:01 2010
@@ -545,6 +545,7 @@ public class DefsTest extends CleanupHel
         
         assert DatabaseDescriptor.getTableDefinition(cf.tableName) != null;
         assert DatabaseDescriptor.getTableDefinition(cf.tableName) == ksm;
+        assert DatabaseDescriptor.getCFMetaData(cf.tableName, cf.cfName) != null;
         
         // updating certain fields should fail.
         CfDef cf_def = new CfDef();
@@ -566,44 +567,28 @@ public class DefsTest extends CleanupHel
         
         // test valid operations.
         cf_def.setComment("Modified comment");
-        CFMetaData updateCfm = cf.apply(cf_def);
-        new UpdateColumnFamily(cf, updateCfm).apply();
-        cf = updateCfm;
+        new UpdateColumnFamily(cf_def).apply(); // doesn't get set back here.
         
         cf_def.setRow_cache_size(2d);
-        updateCfm = cf.apply(cf_def);
-        new UpdateColumnFamily(cf, updateCfm).apply();
-        cf = updateCfm;
+        new UpdateColumnFamily(cf_def).apply();
         
         cf_def.setKey_cache_size(3d);
-        updateCfm = cf.apply(cf_def);
-        new UpdateColumnFamily(cf, updateCfm).apply();
-        cf = updateCfm;
+        new UpdateColumnFamily(cf_def).apply();
         
         cf_def.setRead_repair_chance(0.23);
-        updateCfm = cf.apply(cf_def);
-        new UpdateColumnFamily(cf, updateCfm).apply();
-        cf = updateCfm;
+        new UpdateColumnFamily(cf_def).apply();
         
         cf_def.setGc_grace_seconds(12);
-        updateCfm = cf.apply(cf_def);
-        new UpdateColumnFamily(cf, updateCfm).apply();
-        cf = updateCfm;
+        new UpdateColumnFamily(cf_def).apply();
         
         cf_def.setDefault_validation_class("UTF8Type");
-        updateCfm = cf.apply(cf_def);
-        new UpdateColumnFamily(cf, updateCfm).apply();
-        cf = updateCfm;
+        new UpdateColumnFamily(cf_def).apply();
 
         cf_def.setMin_compaction_threshold(3);
-        updateCfm = cf.apply(cf_def);
-        new UpdateColumnFamily(cf, updateCfm).apply();
-        cf = updateCfm;
+        new UpdateColumnFamily(cf_def).apply();
 
         cf_def.setMax_compaction_threshold(33);
-        updateCfm = cf.apply(cf_def);
-        new UpdateColumnFamily(cf, updateCfm).apply();
-        cf = updateCfm;
+        new UpdateColumnFamily(cf_def).apply();
 
         // can't test changing the reconciler because there is only one impl.
         
@@ -615,12 +600,13 @@ public class DefsTest extends CleanupHel
         assert DatabaseDescriptor.getCFMetaData(cf.tableName, cf.cfName).getGcGraceSeconds() == cf_def.gc_grace_seconds;
         assert DatabaseDescriptor.getCFMetaData(cf.tableName, cf.cfName).getDefaultValidator() == UTF8Type.instance;
         
+        // todo: we probably don't need to reset old values in the catches anymore.
         // make sure some invalid operations fail.
         int oldId = cf_def.id;
         try
         {
             cf_def.setId(cf_def.getId() + 1);
-            updateCfm = cf.apply(cf_def);
+            cf.apply(cf_def);
             throw new AssertionError("Should have blown up when you used a different id.");
         }
         catch (ConfigurationException expected) 
@@ -632,7 +618,7 @@ public class DefsTest extends CleanupHel
         try
         {
             cf_def.setName(cf_def.getName() + "_renamed");
-            updateCfm = cf.apply(cf_def);
+            cf.apply(cf_def);
             throw new AssertionError("Should have blown up when you used a different name.");
         }
         catch (ConfigurationException expected)
@@ -644,7 +630,7 @@ public class DefsTest extends CleanupHel
         try
         {
             cf_def.setKeyspace(oldStr + "_renamed");
-            updateCfm = cf.apply(cf_def);
+            cf.apply(cf_def);
             throw new AssertionError("Should have blown up when you used a different keyspace.");
         }
         catch (ConfigurationException expected)
@@ -655,7 +641,7 @@ public class DefsTest extends CleanupHel
         try
         {
             cf_def.setColumn_type(ColumnFamilyType.Super.name());
-            updateCfm = cf.apply(cf_def);
+            cf.apply(cf_def);
             throw new AssertionError("Should have blwon up when you used a different cf type.");
         }
         catch (ConfigurationException expected)
@@ -667,7 +653,7 @@ public class DefsTest extends CleanupHel
         try 
         {
             cf_def.setComparator_type(BytesType.class.getSimpleName());
-            updateCfm = cf.apply(cf_def);
+            cf.apply(cf_def);
             throw new AssertionError("Should have blown up when you used a different comparator.");
         }
         catch (ConfigurationException expected)
@@ -678,7 +664,7 @@ public class DefsTest extends CleanupHel
         try
         {
             cf_def.setMin_compaction_threshold(34);
-            updateCfm = cf.apply(cf_def);
+            cf.apply(cf_def);
             throw new AssertionError("Should have blown up when min > max.");
         }
         catch (ConfigurationException expected)
@@ -689,7 +675,7 @@ public class DefsTest extends CleanupHel
         try
         {
             cf_def.setMax_compaction_threshold(2);
-            updateCfm = cf.apply(cf_def);
+            cf.apply(cf_def);
             throw new AssertionError("Should have blown up when max > min.");
         }
         catch (ConfigurationException expected)