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/08 09:48:09 UTC
[1/4] git commit: Fix race leading to super column assertion failure
Updated Branches:
refs/heads/trunk 6b2ea2647 -> 008cc7ebd
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/008cc7eb
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/008cc7eb
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/008cc7eb
Branch: refs/heads/trunk
Commit: 008cc7ebdf39b844f130403400b111ef6d7d58a4
Parents: 65059cf
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Mar 7 18:37:48 2012 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Thu Mar 8 09:46:59 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/008cc7eb/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index bf8d9fc..e4018c1 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -47,6 +47,7 @@ Merged from 1.0:
* 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.1-beta1
http://git-wip-us.apache.org/repos/asf/cassandra/blob/008cc7eb/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 f7398ee..ecbb936 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamily.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamily.java
@@ -136,6 +136,26 @@ public class ColumnFamily extends AbstractColumnContainer implements IRowCacheEn
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/008cc7eb/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 48cdb8c..0aa45a5 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -732,7 +732,10 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
if (cachedRow instanceof RowCacheSentinel)
invalidateCachedRow(cacheKey);
else
- ((ColumnFamily) 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)
+ ((ColumnFamily) cachedRow).addAllWithSCCopy(columnFamily, HeapAllocator.instance);
}
}