SuperColumn may ignore relevant tombstones patch by slebresne; reviewed by jbellis for CASSANDRA-3875
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/ad80cf4c Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/ad80cf4c Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/ad80cf4c Branch: refs/heads/cassandra-1.0 Commit: ad80cf4c20977f68cea047d9d03b5e927668db12 Parents: 359391f Author: Sylvain Lebresne <sylv...@datastax.com> Authored: Wed Feb 8 15:08:46 2012 +0100 Committer: Sylvain Lebresne <sylv...@datastax.com> Committed: Wed Feb 8 15:08:46 2012 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + src/java/org/apache/cassandra/db/Column.java | 5 ++ .../org/apache/cassandra/db/ColumnFamilyStore.java | 2 +- src/java/org/apache/cassandra/db/IColumn.java | 1 + src/java/org/apache/cassandra/db/SuperColumn.java | 7 +++- .../apache/cassandra/db/filter/QueryFilter.java | 2 +- .../apache/cassandra/db/RemoveSubColumnTest.java | 34 +++++++++++++++ 7 files changed, 49 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/ad80cf4c/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 513d0ab..dc17889 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -7,6 +7,7 @@ * block on flush before compacting hints (may prevent OOM) (CASSANDRA-3733) * (Pig) fix CassandraStorage to use correct comparator in Super ColumnFamily case (CASSANDRA-3251) + * Fix relevant tomstone ignored with super columns (CASSANDRA-3875) 0.8.9 * avoid logging (harmless) exception when GC takes < 1ms (CASSANDRA-3656) http://git-wip-us.apache.org/repos/asf/cassandra/blob/ad80cf4c/src/java/org/apache/cassandra/db/Column.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/Column.java b/src/java/org/apache/cassandra/db/Column.java index d621591..6c6337f 100644 --- a/src/java/org/apache/cassandra/db/Column.java +++ b/src/java/org/apache/cassandra/db/Column.java @@ -113,6 +113,11 @@ public class Column implements IColumn return timestamp; } + public long mostRecentNonGCableChangeAt(int gcbefore) + { + return timestamp; + } + public int size() { /* http://git-wip-us.apache.org/repos/asf/cassandra/blob/ad80cf4c/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 81b4e78..a0b74a2 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -1292,7 +1292,7 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean return cached; } - private ColumnFamily getColumnFamily(QueryFilter filter, int gcBefore) + ColumnFamily getColumnFamily(QueryFilter filter, int gcBefore) { assert columnFamily.equals(filter.getColumnFamilyName()) : filter.getColumnFamilyName(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/ad80cf4c/src/java/org/apache/cassandra/db/IColumn.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/IColumn.java b/src/java/org/apache/cassandra/db/IColumn.java index ad5a442..1ddfa2c 100644 --- a/src/java/org/apache/cassandra/db/IColumn.java +++ b/src/java/org/apache/cassandra/db/IColumn.java @@ -34,6 +34,7 @@ public interface IColumn public boolean isMarkedForDelete(); public long getMarkedForDeleteAt(); public long mostRecentLiveChangeAt(); + public long mostRecentNonGCableChangeAt(int gcbefore); public ByteBuffer name(); public int size(); public int serializedSize(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/ad80cf4c/src/java/org/apache/cassandra/db/SuperColumn.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/SuperColumn.java b/src/java/org/apache/cassandra/db/SuperColumn.java index d41ef9b..4cd9ebe 100644 --- a/src/java/org/apache/cassandra/db/SuperColumn.java +++ b/src/java/org/apache/cassandra/db/SuperColumn.java @@ -159,10 +159,15 @@ public class SuperColumn implements IColumn, IColumnContainer public long mostRecentLiveChangeAt() { + return mostRecentNonGCableChangeAt(Integer.MAX_VALUE); + } + + public long mostRecentNonGCableChangeAt(int gcbefore) + { long max = Long.MIN_VALUE; for (IColumn column : columns_.values()) { - if (!column.isMarkedForDelete() && column.timestamp() > max) + if ((!column.isMarkedForDelete() || column.getLocalDeletionTime() > gcbefore) && column.timestamp() > max) { max = column.timestamp(); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/ad80cf4c/src/java/org/apache/cassandra/db/filter/QueryFilter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/filter/QueryFilter.java b/src/java/org/apache/cassandra/db/filter/QueryFilter.java index d4cccda..2e911b6 100644 --- a/src/java/org/apache/cassandra/db/filter/QueryFilter.java +++ b/src/java/org/apache/cassandra/db/filter/QueryFilter.java @@ -152,7 +152,7 @@ public class QueryFilter // the column itself must be not gc-able (it is live, or a still relevant tombstone, or has live subcolumns), (1) // and if its container is deleted, the column must be changed more recently than the container tombstone (2) // (since otherwise, the only thing repair cares about is the container tombstone) - long maxChange = column.mostRecentLiveChangeAt(); + long maxChange = column.mostRecentNonGCableChangeAt(gcBefore); return (!column.isMarkedForDelete() || column.getLocalDeletionTime() > gcBefore || maxChange > column.getMarkedForDeleteAt()) // (1) && (!container.isMarkedForDelete() || maxChange > container.getMarkedForDeleteAt()); // (2) } http://git-wip-us.apache.org/repos/asf/cassandra/blob/ad80cf4c/test/unit/org/apache/cassandra/db/RemoveSubColumnTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/RemoveSubColumnTest.java b/test/unit/org/apache/cassandra/db/RemoveSubColumnTest.java index c3f43f3..d1e54ba 100644 --- a/test/unit/org/apache/cassandra/db/RemoveSubColumnTest.java +++ b/test/unit/org/apache/cassandra/db/RemoveSubColumnTest.java @@ -58,4 +58,38 @@ public class RemoveSubColumnTest extends CleanupHelper assert retrieved.getColumn(ByteBufferUtil.bytes("SC1")).getSubColumn(getBytes(1)).isMarkedForDelete(); assertNull(Util.cloneAndRemoveDeleted(retrieved, Integer.MAX_VALUE)); } + + @Test + public void testRemoveSubColumnAndContainer() throws IOException, ExecutionException, InterruptedException + { + Table table = Table.open("Keyspace1"); + ColumnFamilyStore store = table.getColumnFamilyStore("Super1"); + RowMutation rm; + DecoratedKey dk = Util.dk("key2"); + + // add data + rm = new RowMutation("Keyspace1", dk.key); + Util.addMutation(rm, "Super1", "SC1", 1, "asdf", 0); + rm.apply(); + store.forceBlockingFlush(); + + // remove the SC + rm = new RowMutation("Keyspace1", dk.key); + rm.delete(new QueryPath("Super1", ByteBufferUtil.bytes("SC1"), null), 1); + rm.apply(); + + // Mark current time and make sure the next insert happens at least + // one second after the previous one (since gc resolution is the second) + int gcbefore = (int)(System.currentTimeMillis() / 1000); + Thread.currentThread().sleep(1000); + + // remove the column itself + rm = new RowMutation("Keyspace1", dk.key); + rm.delete(new QueryPath("Super1", ByteBufferUtil.bytes("SC1"), getBytes(1)), 2); + rm.apply(); + + ColumnFamily retrieved = store.getColumnFamily(QueryFilter.getIdentityFilter(dk, new QueryPath("Super1", ByteBufferUtil.bytes("SC1"))), gcbefore); + assert retrieved.getColumn(ByteBufferUtil.bytes("SC1")).getSubColumn(getBytes(1)).isMarkedForDelete(); + assertNull(Util.cloneAndRemoveDeleted(retrieved, Integer.MAX_VALUE)); + } }