From 145010997609f1ed8e629b0996c06a367fdde588 Mon Sep 17 00:00:00 2001 From: Sylvain Lebresne Date: Tue, 24 May 2011 16:23:27 +0000 Subject: [PATCH 1/3] Clone super column to avoid modifying them mid-flush patch by slebresne; reviewed by jbellis for CASSANDRA-2675 git-svn-id: https://svn.apache.org/repos/asf/cassandra/branches/cassandra-0.7@1127130 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 1 + src/java/org/apache/cassandra/db/ColumnFamily.java | 5 +++++ .../apache/cassandra/db/filter/QueryFilter.java | 14 +++++++++++++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 996f227856..2b99fb669b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -8,6 +8,7 @@ * avoid replaying hints to dropped columnfamilies (CASSANDRA-2685) * add placeholders for missing rows in range query pseudo-RR (CASSANDRA-2680) * remove no-op HHOM.renameHints (CASSANDRA-2693) + * clone super columns to avoid modifying them during flush (CASSANDRA-2675) 0.7.6 diff --git a/src/java/org/apache/cassandra/db/ColumnFamily.java b/src/java/org/apache/cassandra/db/ColumnFamily.java index f1018ee6ab..8c94f29ed9 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamily.java +++ b/src/java/org/apache/cassandra/db/ColumnFamily.java @@ -146,6 +146,11 @@ public class ColumnFamily implements IColumnContainer, IIterableColumns return columns.size(); } + public boolean isEmpty() + { + return columns.isEmpty(); + } + public boolean isSuper() { return type == ColumnFamilyType.Super; diff --git a/src/java/org/apache/cassandra/db/filter/QueryFilter.java b/src/java/org/apache/cassandra/db/filter/QueryFilter.java index 35d6b11212..d4cccda4a5 100644 --- a/src/java/org/apache/cassandra/db/filter/QueryFilter.java +++ b/src/java/org/apache/cassandra/db/filter/QueryFilter.java @@ -103,7 +103,19 @@ public class QueryFilter public void reduce(IColumn current) { - curCF.addColumn(current); + if (curCF.isSuper() && curCF.isEmpty()) + { + // If it is the first super column we add, we must clone it since other super column may modify + // it otherwise and it could be aliased in a memtable somewhere. We'll also don't have to care about what + // consumers make of the result (for instance CFS.getColumnFamily() call removeDeleted() on the + // result which removes column; which shouldn't be done on the original super column). + assert current instanceof SuperColumn; + curCF.addColumn(((SuperColumn)current).cloneMe()); + } + else + { + curCF.addColumn(current); + } } protected IColumn getReduced() From 85e5c375731c7b74630bc9a0fbdc6ffe4c0e635d Mon Sep 17 00:00:00 2001 From: Jonathan Ellis Date: Tue, 24 May 2011 16:29:12 +0000 Subject: [PATCH 2/3] make DefsTest less fragile patch by jbellis git-svn-id: https://svn.apache.org/repos/asf/cassandra/branches/cassandra-0.7@1127132 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/cassandra/config/DefsTest.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/test/unit/org/apache/cassandra/config/DefsTest.java b/test/unit/org/apache/cassandra/config/DefsTest.java index b724a6f78e..d99f2f81c7 100644 --- a/test/unit/org/apache/cassandra/config/DefsTest.java +++ b/test/unit/org/apache/cassandra/config/DefsTest.java @@ -436,15 +436,15 @@ public class DefsTest extends CleanupHelper assert !success : "This mutation should have failed since the CF no longer exists."; // reads should fail too. + boolean opened = false; try { Table.open(ks.name); + opened = true; } - catch (Throwable th) - { - // this is what has historically happened when you try to open a table that doesn't exist. - assert th instanceof NullPointerException; - } + catch (Throwable th) {} + if (opened) + throw new AssertionError("Opened dropped keyspace"); } @Test @@ -500,14 +500,15 @@ public class DefsTest extends CleanupHelper assert DefsTable.getFiles(newKs.name, cfName).size() > 0; // read on old should fail. + boolean opened = false; try { Table.open(oldKs.name); + opened = true; } - catch (Throwable th) - { - assert th instanceof NullPointerException; - } + catch (Throwable th) {} + if (opened) + throw new AssertionError("Opened dropped keyspace"); // write on old should fail. rm = new RowMutation(oldKs.name, ByteBufferUtil.bytes("any key will do")); From 5841048f62f8b53a08f7cfaf717094ab97e083ea Mon Sep 17 00:00:00 2001 From: Jonathan Ellis Date: Tue, 24 May 2011 16:45:26 +0000 Subject: [PATCH 3/3] add more descriptive errors for ConfigurationException when updating a CF definition patch by jbellis git-svn-id: https://svn.apache.org/repos/asf/cassandra/branches/cassandra-0.7@1127137 13f79535-47bb-0310-9956-ffa450edef68 --- src/java/org/apache/cassandra/config/CFMetaData.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/java/org/apache/cassandra/config/CFMetaData.java b/src/java/org/apache/cassandra/config/CFMetaData.java index 4e6158dde4..07bb3f08e6 100644 --- a/src/java/org/apache/cassandra/config/CFMetaData.java +++ b/src/java/org/apache/cassandra/config/CFMetaData.java @@ -628,12 +628,16 @@ public final class CFMetaData public void apply(org.apache.cassandra.avro.CfDef cf_def) throws ConfigurationException { // validate - if (!cf_def.id.equals(cfId)) - throw new ConfigurationException("ids do not match."); if (!cf_def.keyspace.toString().equals(tableName)) - throw new ConfigurationException("keyspaces do not match."); + throw new ConfigurationException(String.format("Keyspace mismatch (found %s; expected %s)", + cf_def.keyspace, tableName)); if (!cf_def.name.toString().equals(cfName)) - throw new ConfigurationException("names do not match."); + throw new ConfigurationException(String.format("Column family mismatch (found %s; expected %s)", + cf_def.name, cfName)); + if (!cf_def.id.equals(cfId)) + throw new ConfigurationException(String.format("Column family ID mismatch (found %s; expected %s)", + cf_def.id, cfId)); + if (!cf_def.column_type.toString().equals(cfType.name())) throw new ConfigurationException("types do not match."); if (comparator != DatabaseDescriptor.getComparator(cf_def.comparator_type))