Updated Branches: refs/heads/trunk 8142a2fe9 -> 86637d43c
Make CFMetaData conversion to/from thrift/native schema inverses patch by slebresne; reviewed by xedin for CASSANDRA-3559 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/86637d43 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/86637d43 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/86637d43 Branch: refs/heads/trunk Commit: 86637d43cb4019d5f6747349d9d7315ff2236017 Parents: 8142a2f Author: Sylvain Lebresne <[email protected]> Authored: Wed Jan 25 11:24:54 2012 +0100 Committer: Sylvain Lebresne <[email protected]> Committed: Wed Jan 25 12:11:56 2012 +0100 ---------------------------------------------------------------------- CHANGES.txt | 2 + .../org/apache/cassandra/config/CFMetaData.java | 28 +++++++--- .../apache/cassandra/config/ColumnDefinition.java | 22 +++++--- .../org/apache/cassandra/config/KSMetaData.java | 2 +- .../cassandra/db/migration/MigrationHelper.java | 28 +++++++++- .../apache/cassandra/config/CFMetaDataTest.java | 42 ++++++++++++++- 6 files changed, 104 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/86637d43/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index c0ea71c..66ab562 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -53,6 +53,8 @@ throttling to prevent an NPE when there is no yaml config (CASSANDRA-3752) * Allow concurrent schema migrations (CASSANDRA-1391) * Add SnapshotCommand to trigger snapshot on remote node (CASSANDRA-3721) + * Make CFMetaData conversions to/from thrift/native schema inverses + (CASSANDRA_3559) 1.0.8 http://git-wip-us.apache.org/repos/asf/cassandra/blob/86637d43/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 1afa04f..9bd731a 100644 --- a/src/java/org/apache/cassandra/config/CFMetaData.java +++ b/src/java/org/apache/cassandra/config/CFMetaData.java @@ -825,7 +825,7 @@ public final class CFMetaData def.setMin_compaction_threshold(minCompactionThreshold); def.setMax_compaction_threshold(maxCompactionThreshold); def.setMerge_shards_chance(mergeShardsChance); - def.setKey_alias(getKeyName()); + def.setKey_alias(keyAlias); List<org.apache.cassandra.thrift.ColumnDef> column_meta = new ArrayList<org.apache.cassandra.thrift.ColumnDef>(column_metadata.size()); for (ColumnDefinition cd : column_metadata.values()) { @@ -975,8 +975,8 @@ public final class CFMetaData if (field.equals(CfDef._Fields.COLUMN_METADATA)) continue; // deal with columns after main attributes - Object curValue = curState.getFieldValue(field); - Object newValue = newState.getFieldValue(field); + Object curValue = curState.isSet(field) ? curState.getFieldValue(field) : null; + Object newValue = newState.isSet(field) ? newState.getFieldValue(field) : null; if (Objects.equal(curValue, newValue)) continue; @@ -1064,8 +1064,9 @@ public final class CFMetaData if (field.equals(CfDef._Fields.COLUMN_METADATA)) continue; + Object value = cfDef.isSet(field) ? cfDef.getFieldValue(field) : null; mutation.add(new QueryPath(SystemTable.SCHEMA_COLUMNFAMILIES_CF, null, compositeNameFor(cfDef.name, field.getFieldName())), - valueAsBytes(cfDef.getFieldValue(field)), + valueAsBytes(value), timestamp); } @@ -1091,6 +1092,15 @@ public final class CFMetaData */ public static CfDef fromSchema(ColumnFamily serializedCfDef) throws IOException { + CfDef cfDef = fromSchemaNoColumnDefinition(serializedCfDef); + + ColumnFamily serializedColumnDefinitions = ColumnDefinition.readSchema(cfDef.keyspace, cfDef.name); + return addColumnDefinitionSchema(cfDef, serializedColumnDefinitions); + } + + // Package protected for use by tests + static CfDef fromSchemaNoColumnDefinition(ColumnFamily serializedCfDef) + { assert serializedCfDef != null; CfDef cfDef = new CfDef(); @@ -1109,10 +1119,14 @@ public final class CFMetaData CfDef._Fields field = CfDef._Fields.findByName(attr[1]); cfDef.setFieldValue(field, deserializeValue(cfAttr.value(), getValueClass(CfDef.class, field.getFieldName()))); } + return cfDef; + } - for (ColumnDef columnDef : ColumnDefinition.fromSchema(cfDef.keyspace, cfDef.name)) + // Package protected for use by tests + static CfDef addColumnDefinitionSchema(CfDef cfDef, ColumnFamily serializedColumnDefinitions) + { + for (ColumnDef columnDef : ColumnDefinition.fromSchema(serializedColumnDefinitions)) cfDef.addToColumn_metadata(columnDef); - return cfDef; } @@ -1144,4 +1158,4 @@ public final class CFMetaData .append("caching", caching) .toString(); } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/cassandra/blob/86637d43/src/java/org/apache/cassandra/config/ColumnDefinition.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/config/ColumnDefinition.java b/src/java/org/apache/cassandra/config/ColumnDefinition.java index d8c9db9..927ca39 100644 --- a/src/java/org/apache/cassandra/config/ColumnDefinition.java +++ b/src/java/org/apache/cassandra/config/ColumnDefinition.java @@ -208,6 +208,18 @@ public class ColumnDefinition } } + public static ColumnFamily readSchema(String ksName, String cfName) + { + DecoratedKey key = StorageService.getPartitioner().decorateKey(SystemTable.getSchemaKSKey(ksName)); + ColumnFamilyStore columnsStore = SystemTable.schemaCFS(SystemTable.SCHEMA_COLUMNS_CF); + return columnsStore.getColumnFamily(key, + new QueryPath(SystemTable.SCHEMA_COLUMNS_CF), + MigrationHelper.searchComposite(cfName, true), + MigrationHelper.searchComposite(cfName, false), + false, + Integer.MAX_VALUE); + } + /** * Deserialize columns from low-level representation * @@ -216,16 +228,8 @@ public class ColumnDefinition * * @return Thrift-based deserialized representation of the column */ - public static List<ColumnDef> fromSchema(String ksName, String cfName) + public static List<ColumnDef> fromSchema(ColumnFamily columns) { - DecoratedKey key = StorageService.getPartitioner().decorateKey(SystemTable.getSchemaKSKey(ksName)); - ColumnFamilyStore columnsStore = SystemTable.schemaCFS(SystemTable.SCHEMA_COLUMNS_CF); - ColumnFamily columns = columnsStore.getColumnFamily(key, - new QueryPath(SystemTable.SCHEMA_COLUMNS_CF), - MigrationHelper.searchComposite(cfName, true), - MigrationHelper.searchComposite(cfName, false), - false, - Integer.MAX_VALUE); if (columns == null || columns.isEmpty()) return Collections.emptyList(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/86637d43/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 2196156..728b9f9 100644 --- a/src/java/org/apache/cassandra/config/KSMetaData.java +++ b/src/java/org/apache/cassandra/config/KSMetaData.java @@ -406,7 +406,7 @@ public final class KSMetaData for (CfDef cfDef : cfs.values()) { - for (ColumnDef columnDef : ColumnDefinition.fromSchema(cfDef.keyspace, cfDef.name)) + for (ColumnDef columnDef : ColumnDefinition.fromSchema(ColumnDefinition.readSchema(cfDef.keyspace, cfDef.name))) cfDef.addToColumn_metadata(columnDef); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/86637d43/src/java/org/apache/cassandra/db/migration/MigrationHelper.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/migration/MigrationHelper.java b/src/java/org/apache/cassandra/db/migration/MigrationHelper.java index 981e02b..d6a498e 100644 --- a/src/java/org/apache/cassandra/db/migration/MigrationHelper.java +++ b/src/java/org/apache/cassandra/db/migration/MigrationHelper.java @@ -21,7 +21,9 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.Future; import com.google.common.collect.Iterables; @@ -47,6 +49,18 @@ import org.codehaus.jackson.map.ObjectMapper; public class MigrationHelper { private static final ObjectMapper jsonMapper = new ObjectMapper(); + private static final Map<Class<?>, Class<?>> primitiveToWrapper = new HashMap<Class<?>, Class<?>>(); + static + { + primitiveToWrapper.put(boolean.class, Boolean.class); + primitiveToWrapper.put(byte.class, Byte.class); + primitiveToWrapper.put(short.class, Short.class); + primitiveToWrapper.put(char.class, Character.class); + primitiveToWrapper.put(int.class, Integer.class); + primitiveToWrapper.put(long.class, Long.class); + primitiveToWrapper.put(float.class, Float.class); + primitiveToWrapper.put(double.class, Double.class); + } public static ByteBuffer readableColumnName(ByteBuffer columnName, AbstractType comparator) { @@ -71,7 +85,10 @@ public class MigrationHelper { // because jackson serialized ByteBuffer as byte[] and needs help with deserialization later if (valueClass.equals(ByteBuffer.class)) - return ByteBuffer.wrap((byte[]) deserializeValue(value, byte[].class)); + { + byte[] bvalue = (byte[]) deserializeValue(value, byte[].class); + return bvalue == null ? null : ByteBuffer.wrap(bvalue); + } return jsonMapper.readValue(ByteBufferUtil.getArray(value), valueClass); } @@ -85,7 +102,8 @@ public class MigrationHelper { try { - return klass.getField(name).getType(); + // We want to keep null values, so we must not return a primitive type + return maybeConvertToWrapperClass(klass.getField(name).getType()); } catch (NoSuchFieldException e) { @@ -93,6 +111,12 @@ public class MigrationHelper } } + private static Class<?> maybeConvertToWrapperClass(Class<?> klass) + { + Class<?> cl = primitiveToWrapper.get(klass); + return cl == null ? klass : cl; + } + public static ByteBuffer searchComposite(String comp1, boolean start) { return compositeNameFor(comp1, !start, null, false, null, false); http://git-wip-us.apache.org/repos/asf/cassandra/blob/86637d43/test/unit/org/apache/cassandra/config/CFMetaDataTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/config/CFMetaDataTest.java b/test/unit/org/apache/cassandra/config/CFMetaDataTest.java index 1a67c04..76d1c4b 100644 --- a/test/unit/org/apache/cassandra/config/CFMetaDataTest.java +++ b/test/unit/org/apache/cassandra/config/CFMetaDataTest.java @@ -20,9 +20,18 @@ package org.apache.cassandra.config; import java.util.ArrayList; import java.util.List; +import java.util.HashMap; +import org.apache.cassandra.CleanupHelper; +import org.apache.cassandra.config.Schema; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.ColumnFamily; +import org.apache.cassandra.db.RowMutation; +import org.apache.cassandra.db.SystemTable; +import org.apache.cassandra.db.Table; import org.apache.cassandra.db.marshal.AsciiType; import org.apache.cassandra.db.marshal.UTF8Type; +import org.apache.cassandra.io.compress.*; import org.apache.cassandra.thrift.CfDef; import org.apache.cassandra.thrift.ColumnDef; import org.apache.cassandra.thrift.IndexType; @@ -32,7 +41,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; -public class CFMetaDataTest +public class CFMetaDataTest extends CleanupHelper { private static String KEYSPACE = "Keyspace1"; private static String COLUMN_FAMILY = "Standard1"; @@ -87,4 +96,35 @@ public class CFMetaDataTest assertEquals(thriftCfDef.comment, converted.comment); assertEquals(thriftCfDef.column_metadata, converted.column_metadata); } + + @Test + public void testConversionsInverses() throws Exception + { + for (String table : Schema.instance.getNonSystemTables()) + { + for (ColumnFamilyStore cfs : Table.open(table).getColumnFamilyStores()) + { + CFMetaData cfm = cfs.metadata; + checkInverses(cfm); + + // Testing with compression to catch #3558 + CFMetaData withCompression = CFMetaData.rename(cfm, cfm.cfName); // basically a clone + withCompression.compressionParameters(new CompressionParameters(SnappyCompressor.instance, 32768, new HashMap<String, String>())); + checkInverses(withCompression); + } + } + } + + private void checkInverses(CFMetaData cfm) throws Exception + { + // Test thrift conversion + assert cfm.equals(CFMetaData.fromThrift(cfm.toThrift())) : String.format("\n%s\n!=\n%s", cfm, CFMetaData.fromThrift(cfm.toThrift())); + + // Test schema conversion + RowMutation rm = cfm.toSchema(System.currentTimeMillis()); + ColumnFamily serializedCf = rm.getColumnFamily(Schema.instance.getId(Table.SYSTEM_TABLE, SystemTable.SCHEMA_COLUMNFAMILIES_CF)); + ColumnFamily serializedCD = rm.getColumnFamily(Schema.instance.getId(Table.SYSTEM_TABLE, SystemTable.SCHEMA_COLUMNS_CF)); + CfDef cfDef = CFMetaData.addColumnDefinitionSchema(CFMetaData.fromSchema(serializedCf), serializedCD); + assert cfm.equals(CFMetaData.fromThrift(cfDef)) : String.format("\n%s\n!=\n%s", cfm, CFMetaData.fromThrift(cfDef)); + } }
