Author: jbellis
Date: Wed Nov 4 17:07:49 2009
New Revision: 832798
URL: http://svn.apache.org/viewvc?rev=832798&view=rev
Log:
return clones of supercolumns from memtable so caller can't accidentally mutate
them, fixing the failing test.
convert removeDeleted on SC to remove-oriented instead of clone-then-add-back
to make this hurt performance less.
patch by jbellis; reviewed by gdusbabek for CASSANDRA-510
Modified:
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/Memtable.java
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/RemoveSuperColumnTest.java
Modified:
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
URL:
http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java?rev=832798&r1=832797&r2=832798&view=diff
==============================================================================
---
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
(original)
+++
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
Wed Nov 4 17:07:49 2009
@@ -499,45 +499,55 @@
return null;
}
+ if (cf.isSuper())
+ removeDeletedSuper(cf, gcBefore);
+ else
+ removeDeletedStandard(cf, gcBefore);
+
// in case of a timestamp tie, tombstones get priority over
non-tombstones.
// (we want this to be deterministic to avoid confusion.)
+ if (cf.getColumnCount() == 0 && cf.getLocalDeletionTime() <= gcBefore)
+ {
+ return null;
+ }
+ return cf;
+ }
+
+ private static void removeDeletedStandard(ColumnFamily cf, int gcBefore)
+ {
for (byte[] cname : cf.getColumnNames())
{
IColumn c = cf.getColumnsMap().get(cname);
- if (c instanceof SuperColumn)
+ if ((c.isMarkedForDelete() && c.getLocalDeletionTime() <= gcBefore)
+ || c.timestamp() <= cf.getMarkedForDeleteAt())
{
- long minTimestamp = Math.max(c.getMarkedForDeleteAt(),
cf.getMarkedForDeleteAt());
- // don't operate directly on the supercolumn, it could be the
one in the memtable.
- // instead, create a new SC and add in the subcolumns that
qualify.
cf.remove(cname);
- SuperColumn sc = ((SuperColumn)c).cloneMeShallow();
- for (IColumn subColumn : c.getSubColumns())
- {
- if (subColumn.timestamp() > minTimestamp)
- {
- if (!subColumn.isMarkedForDelete() ||
subColumn.getLocalDeletionTime() > gcBefore)
- {
- sc.addColumn(subColumn);
- }
- }
- }
- if (sc.getSubColumns().size() > 0 || sc.getLocalDeletionTime()
> gcBefore)
+ }
+ }
+ }
+
+ private static void removeDeletedSuper(ColumnFamily cf, int gcBefore)
+ {
+ // TODO assume deletion means "most are deleted?" and add to clone,
instead of remove from original?
+ // this could be improved by having compaction, or possibly even
removeDeleted, r/m the tombstone
+ // once gcBefore has passed, so if new stuff is added in it doesn't
used the wrong algorithm forever
+ for (byte[] cname : cf.getColumnNames())
+ {
+ IColumn c = cf.getColumnsMap().get(cname);
+ long minTimestamp = Math.max(c.getMarkedForDeleteAt(),
cf.getMarkedForDeleteAt());
+ for (IColumn subColumn : c.getSubColumns())
+ {
+ if (subColumn.timestamp() <= minTimestamp
+ || (subColumn.isMarkedForDelete() &&
subColumn.getLocalDeletionTime() <= gcBefore))
{
- cf.addColumn(sc);
+ ((SuperColumn)c).remove(subColumn.name());
}
}
- else if ((c.isMarkedForDelete() && c.getLocalDeletionTime() <=
gcBefore)
- || c.timestamp() <= cf.getMarkedForDeleteAt())
+ if (c.getSubColumns().isEmpty() && c.getLocalDeletionTime() <=
gcBefore)
{
- cf.remove(cname);
+ cf.remove(c.name());
}
}
-
- if (cf.getColumnCount() == 0 && cf.getLocalDeletionTime() <= gcBefore)
- {
- return null;
- }
- return cf;
}
/*
Modified:
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/Memtable.java
URL:
http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/Memtable.java?rev=832798&r1=832797&r2=832798&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/db/Memtable.java
(original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/db/Memtable.java
Wed Nov 4 17:07:49 2009
@@ -262,7 +262,8 @@
if (filter.reversed)
ArrayUtils.reverse(columns);
IColumn startIColumn;
- if (DatabaseDescriptor.getColumnFamilyType(table_,
filter.getColumnFamilyName()).equals("Standard"))
+ final boolean isStandard =
DatabaseDescriptor.getColumnFamilyType(table_,
filter.getColumnFamilyName()).equals("Standard");
+ if (isStandard)
startIColumn = new Column(filter.start);
else
startIColumn = new SuperColumn(filter.start, null); // ok to not
have subcolumnComparator since we won't be adding columns to this object
@@ -298,7 +299,8 @@
public IColumn next()
{
- return columns[curIndex_++];
+ // clone supercolumns so caller can freely removeDeleted or
otherwise mutate it
+ return isStandard ? columns[curIndex_++] :
((SuperColumn)columns[curIndex_++]).cloneMe();
}
};
}
@@ -307,6 +309,7 @@
{
final ColumnFamily cf =
columnFamilies_.get(partitioner_.decorateKey(filter.key));
final ColumnFamily columnFamily = cf == null ?
ColumnFamily.create(table_, filter.getColumnFamilyName()) : cf.cloneMeShallow();
+ final boolean isStandard =
DatabaseDescriptor.getColumnFamilyType(table_,
filter.getColumnFamilyName()).equals("Standard");
return new SimpleAbstractColumnIterator()
{
@@ -329,7 +332,8 @@
current = iter.next();
IColumn column = cf.getColumn(current);
if (column != null)
- return column;
+ // clone supercolumns so caller can freely
removeDeleted or otherwise mutate it
+ return isStandard ? column :
((SuperColumn)column).cloneMe();
}
return endOfData();
}
Modified:
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java
URL:
http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java?rev=832798&r1=832797&r2=832798&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java
(original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java
Wed Nov 4 17:07:49 2009
@@ -49,10 +49,15 @@
SuperColumn(byte[] name, AbstractType comparator)
{
+ this(name, new ConcurrentSkipListMap<byte[], IColumn>(comparator));
+ }
+
+ private SuperColumn(byte[] name, ConcurrentSkipListMap<byte[], IColumn>
columns)
+ {
assert name != null;
assert name.length <= IColumn.MAX_NAME_LENGTH;
name_ = name;
- columns_ = new ConcurrentSkipListMap<byte[], IColumn>(comparator);
+ columns_ = columns;
}
public AbstractType getComparator()
@@ -67,6 +72,14 @@
return sc;
}
+
+ public IColumn cloneMe()
+ {
+ SuperColumn sc = new SuperColumn(name_, new
ConcurrentSkipListMap<byte[], IColumn>(columns_));
+ sc.markForDeleteAt(localDeletionTime, markedForDeleteAt);
+ return sc;
+ }
+
public boolean isMarkedForDelete()
{
return markedForDeleteAt > Long.MIN_VALUE;
Modified:
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
URL:
http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java?rev=832798&r1=832797&r2=832798&view=diff
==============================================================================
---
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
(original)
+++
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
Wed Nov 4 17:07:49 2009
@@ -66,6 +66,8 @@
public SuperColumn filterSuperColumn(SuperColumn superColumn, int gcBefore)
{
+ // we clone shallow, then add, under the theory that generally we're
interested in a relatively small number of subcolumns.
+ // this may be a poor assumption.
SuperColumn scFiltered = superColumn.cloneMeShallow();
Iterator<IColumn> subcolumns;
if (reversed)
Modified:
incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/RemoveSuperColumnTest.java
URL:
http://svn.apache.org/viewvc/incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/RemoveSuperColumnTest.java?rev=832798&r1=832797&r2=832798&view=diff
==============================================================================
---
incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/RemoveSuperColumnTest.java
(original)
+++
incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/RemoveSuperColumnTest.java
Wed Nov 4 17:07:49 2009
@@ -100,8 +100,8 @@
{
ColumnFamilyStore store =
Table.open("Keyspace1").getColumnFamilyStore("Super1");
ColumnFamily resolved = store.getColumnFamily(new
NamesQueryFilter("key1", new QueryPath("Super1"), "SC1".getBytes()));
- assert
resolved.getSortedColumns().iterator().next().getMarkedForDeleteAt() == 1;
- assert
resolved.getSortedColumns().iterator().next().getSubColumns().size() == 0;
+ assert
resolved.getSortedColumns().iterator().next().getMarkedForDeleteAt() == 1 :
resolved;
+ assert
resolved.getSortedColumns().iterator().next().getSubColumns().size() == 0 :
resolved;
assertNull(ColumnFamilyStore.removeDeleted(resolved,
Integer.MAX_VALUE));
assertNull(store.getColumnFamily(new NamesQueryFilter("key1", new
QueryPath("Super1"), "SC1".getBytes()), Integer.MAX_VALUE));
assertNull(store.getColumnFamily(new IdentityQueryFilter("key1", new
QueryPath("Super1")), Integer.MAX_VALUE));
