You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2012/03/07 18:56:12 UTC

[3/6] git commit: Fix race leading to super column assertion failure

Fix race leading to super column assertion failure

patch by slebresne; reviewed by jbellis for CASSANDRA-3957


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/f2399074
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/f2399074
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/f2399074

Branch: refs/heads/cassandra-1.1
Commit: f2399074465357fefd60521da644d65adc54651d
Parents: 048c8a9
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Mar 7 18:37:48 2012 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Mar 7 18:37:48 2012 +0100

----------------------------------------------------------------------
 CHANGES.txt                                        |    1 +
 src/java/org/apache/cassandra/db/ColumnFamily.java |   20 +++++++++++++++
 .../org/apache/cassandra/db/ColumnFamilyStore.java |    5 +++-
 3 files changed, 25 insertions(+), 1 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2399074/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index c453943..925a4a9 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -9,6 +9,7 @@
  * Fix division-by-zero error on get_slice (CASSANDRA-4000)
  * don't change manifest level for cleanup, scrub, and upgradesstables
    operations under LeveledCompactionStrategy (CASSANDRA-3989)
+ * fix race leading to super columns assertion failure (CASSANDRA-3957)
 
 
 1.0.8

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2399074/src/java/org/apache/cassandra/db/ColumnFamily.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/ColumnFamily.java b/src/java/org/apache/cassandra/db/ColumnFamily.java
index cb715de..d27a963 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamily.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamily.java
@@ -140,6 +140,26 @@ public class ColumnFamily extends AbstractColumnContainer
         return getType() == ColumnFamilyType.Super;
     }
 
+    /**
+     * Same as addAll() but do a cloneMeShallow of SuperColumn if necessary to
+     * avoid keeping references to the structure (see #3957).
+     */
+    public void addAllWithSCCopy(ColumnFamily cf, Allocator allocator)
+    {
+        if (cf.isSuper())
+        {
+            for (IColumn c : cf)
+            {
+                columns.addColumn(((SuperColumn)c).cloneMeShallow(), allocator);
+            }
+            delete(cf);
+        }
+        else
+        {
+            addAll(cf, allocator);
+        }
+    }
+
     public void addColumn(QueryPath path, ByteBuffer value, long timestamp)
     {
         addColumn(path, value, timestamp, 0);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f2399074/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index b50662a..60a3487 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -787,7 +787,10 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
         {
             ColumnFamily cachedRow = getRawCachedRow(key);
             if (cachedRow != null)
-                cachedRow.addAll(columnFamily, HeapAllocator.instance);
+                // columnFamily is what is written in the commit log. Because of the PeriodicCommitLog, this can be done in concurrency
+                // with this. So columnFamily shouldn't be modified and if it contains super columns, neither should they. So for super
+                // columns, we must make sure to clone them when adding to the cache. That's what addAllWithSCCopy does (see #3957)
+                cachedRow.addAllWithSCCopy(columnFamily, HeapAllocator.instance);
         }
     }