From 83cce240bf61efe41c2074b8da4958794f15b123 Mon Sep 17 00:00:00 2001 From: Jonathan Ellis Date: Thu, 3 Dec 2009 19:17:03 +0000 Subject: [PATCH] Fix not returning live subcolumns of deleted supercolumns patch by jbellis; reviewed by junrao for CASSANDRA-583 git-svn-id: https://svn.apache.org/repos/asf/incubator/cassandra/trunk@886886 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 3 +++ src/java/org/apache/cassandra/db/Column.java | 2 +- .../org/apache/cassandra/db/ColumnFamilyStore.java | 2 +- src/java/org/apache/cassandra/db/IColumn.java | 2 +- src/java/org/apache/cassandra/db/SuperColumn.java | 7 +++---- .../apache/cassandra/db/filter/NamesQueryFilter.java | 2 +- .../org/apache/cassandra/db/filter/QueryFilter.java | 10 ++++++++++ .../apache/cassandra/db/filter/SliceQueryFilter.java | 11 ++--------- test/system/test_server.py | 8 +++++++- .../apache/cassandra/db/RemoveSuperColumnTest.java | 2 +- 10 files changed, 30 insertions(+), 19 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 7c3d9199ce..0219636733 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -7,6 +7,9 @@ * fix cleanup of local "system" keyspace (CASSANDRA-576) * improve computation of cluster load balance (CASSANDRA-554) * added count and column/row delete to cassandra-cli (CASSANDRA-594) + * fix returning live subcolumns of deleted supercolumns (CASSANDRA-583) + * respect JAVA_HOME in bin/ scripts (several tickets) + * add StorageService.initClient for fat clients on the JVM (CASSANDRA-535) 0.5.0 beta diff --git a/src/java/org/apache/cassandra/db/Column.java b/src/java/org/apache/cassandra/db/Column.java index ea67ca4cef..a6a7ca471b 100644 --- a/src/java/org/apache/cassandra/db/Column.java +++ b/src/java/org/apache/cassandra/db/Column.java @@ -132,7 +132,7 @@ public final class Column implements IColumn return timestamp; } - public long mostRecentChangeAt() + public long mostRecentLiveChangeAt() { return timestamp; } diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index ff172b1475..ee37d8171e 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -1117,7 +1117,7 @@ public final class ColumnFamilyStore implements ColumnFamilyStoreMBean if (filter.path.superColumnName != null) { QueryFilter nameFilter = new NamesQueryFilter(filter.key, new QueryPath(columnFamily_), filter.path.superColumnName); - ColumnFamily cf = getColumnFamilyInternal(nameFilter, getDefaultGCBefore()); + ColumnFamily cf = getColumnFamilyInternal(nameFilter, gcBefore); if (cf == null || cf.getColumnCount() == 0) return cf; diff --git a/src/java/org/apache/cassandra/db/IColumn.java b/src/java/org/apache/cassandra/db/IColumn.java index a920857697..46d0bd1569 100644 --- a/src/java/org/apache/cassandra/db/IColumn.java +++ b/src/java/org/apache/cassandra/db/IColumn.java @@ -30,7 +30,7 @@ public interface IColumn public boolean isMarkedForDelete(); public long getMarkedForDeleteAt(); - public long mostRecentChangeAt(); + public long mostRecentLiveChangeAt(); public byte[] name(); public int size(); public int serializedSize(); diff --git a/src/java/org/apache/cassandra/db/SuperColumn.java b/src/java/org/apache/cassandra/db/SuperColumn.java index 60f6108673..0531d19084 100644 --- a/src/java/org/apache/cassandra/db/SuperColumn.java +++ b/src/java/org/apache/cassandra/db/SuperColumn.java @@ -20,7 +20,6 @@ package org.apache.cassandra.db; import java.io.*; import java.util.Collection; -import java.util.Arrays; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.ConcurrentSkipListMap; import java.security.MessageDigest; @@ -158,14 +157,14 @@ public final class SuperColumn implements IColumn, IColumnContainer throw new IllegalArgumentException("Timestamp was requested for a column that does not exist."); } - public long mostRecentChangeAt() + public long mostRecentLiveChangeAt() { long max = Long.MIN_VALUE; for (IColumn column : columns_.values()) { - if (column.mostRecentChangeAt() > max) + if (!column.isMarkedForDelete() && column.timestamp() > max) { - max = column.mostRecentChangeAt(); + max = column.timestamp(); } } return max; diff --git a/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java b/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java index a77a3d20c2..6523354ae6 100644 --- a/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java +++ b/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java @@ -86,7 +86,7 @@ public class NamesQueryFilter extends QueryFilter while (reducedColumns.hasNext()) { IColumn column = reducedColumns.next(); - if (!column.isMarkedForDelete() || column.getLocalDeletionTime() > gcBefore) + if (QueryFilter.isRelevant(column, container, gcBefore)) container.addColumn(column); } } diff --git a/src/java/org/apache/cassandra/db/filter/QueryFilter.java b/src/java/org/apache/cassandra/db/filter/QueryFilter.java index 251da22ef0..144efe1bb8 100644 --- a/src/java/org/apache/cassandra/db/filter/QueryFilter.java +++ b/src/java/org/apache/cassandra/db/filter/QueryFilter.java @@ -111,4 +111,14 @@ public abstract class QueryFilter { return path.columnFamilyName; } + + public static boolean isRelevant(IColumn column, IColumnContainer container, int gcBefore) + { + // 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(); + return (!column.isMarkedForDelete() || column.getLocalDeletionTime() > gcBefore || maxChange > column.getMarkedForDeleteAt()) // (1) + && (!container.isMarkedForDelete() || maxChange > container.getMarkedForDeleteAt()); // (2) + } } diff --git a/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java b/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java index 5cac316594..37f66522d3 100644 --- a/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java +++ b/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java @@ -35,7 +35,6 @@ import org.apache.commons.collections.IteratorUtils; import org.apache.cassandra.io.SSTableReader; import org.apache.cassandra.db.*; import org.apache.cassandra.db.marshal.AbstractType; -import org.apache.cassandra.config.DatabaseDescriptor; public class SliceQueryFilter extends QueryFilter { @@ -124,20 +123,14 @@ public class SliceQueryFilter extends QueryFilter // only count live columns towards the `count` criteria if (!column.isMarkedForDelete() && (!container.isMarkedForDelete() - || column.mostRecentChangeAt() > container.getMarkedForDeleteAt())) + || column.mostRecentLiveChangeAt() > container.getMarkedForDeleteAt())) { liveColumns++; } // but we need to add all non-gc-able columns to the result for read repair: - // the column itself must be not gc-able, (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) - if ((!column.isMarkedForDelete() || column.getLocalDeletionTime() > gcBefore) // (1) - && (!container.isMarkedForDelete() || column.mostRecentChangeAt() > container.getMarkedForDeleteAt())) // (2) - { + if (QueryFilter.isRelevant(column, container, gcBefore)) container.addColumn(column); - } } } } diff --git a/test/system/test_server.py b/test/system/test_server.py index 5d19d2757e..d67a8021de 100644 --- a/test/system/test_server.py +++ b/test/system/test_server.py @@ -452,7 +452,7 @@ class TestMutations(CassandraTester): # Test resurrection. First, re-insert the value w/ older timestamp, # and make sure it stays removed: - client.insert('Keyspace1', 'key1', ColumnPath('Super1', 'sc2', _i64(5)), 'value5', 0, ConsistencyLevel.ONE) + client.insert('Keyspace1', 'key1', ColumnPath('Super1', 'sc2', _i64(5)), 'value5', 1, ConsistencyLevel.ONE) super_columns = [result.super_column for result in _big_slice('Keyspace1', 'key1', ColumnParent('Super1'))] assert super_columns == super_columns_expected, super_columns @@ -465,6 +465,12 @@ class TestMutations(CassandraTester): SuperColumn(name='sc2', columns=[Column(_i64(5), 'value5', 6)])] assert super_columns == super_columns_expected, super_columns + # check slicing at the subcolumn level too + p = SlicePredicate(slice_range=SliceRange('', '', False, 1000)) + columns = [result.column + for result in client.get_slice('Keyspace1', 'key1', ColumnParent('Super1', 'sc2'), p, ConsistencyLevel.ONE)] + assert columns == [Column(_i64(5), 'value5', 6)], columns + def test_empty_range(self): assert client.get_key_range('Keyspace1', 'Standard1', '', '', 1000, ConsistencyLevel.ONE) == [] diff --git a/test/unit/org/apache/cassandra/db/RemoveSuperColumnTest.java b/test/unit/org/apache/cassandra/db/RemoveSuperColumnTest.java index e462a82834..31815c2e57 100644 --- a/test/unit/org/apache/cassandra/db/RemoveSuperColumnTest.java +++ b/test/unit/org/apache/cassandra/db/RemoveSuperColumnTest.java @@ -153,7 +153,7 @@ public class RemoveSuperColumnTest private void validateRemoveWithNewData() throws IOException { ColumnFamilyStore store = Table.open("Keyspace1").getColumnFamilyStore("Super2"); - ColumnFamily resolved = store.getColumnFamily(new NamesQueryFilter("key1", new QueryPath("Super2", "SC1".getBytes()), getBytes(2))); + ColumnFamily resolved = store.getColumnFamily(new NamesQueryFilter("key1", new QueryPath("Super2", "SC1".getBytes()), getBytes(2)), Integer.MAX_VALUE); Collection subColumns = resolved.getSortedColumns().iterator().next().getSubColumns(); assert subColumns.size() == 1; assert subColumns.iterator().next().timestamp() == 2;