Repository: cassandra Updated Branches: refs/heads/cassandra-2.1 35b88c560 -> ec38458d6 refs/heads/cassandra-2.1.0 75364296c -> 0bc4663aa refs/heads/trunk 047c04611 -> e58f379ca
Consider expiry when reconciling otherwise equal cells patch by Benedict Elliott Smith; reviewed by Aleksey Yeschenko and Sylvain Lebresne for CASSANDRA-7403 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/0bc4663a Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/0bc4663a Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/0bc4663a Branch: refs/heads/cassandra-2.1 Commit: 0bc4663aad3257f359058465dccbb36141fc75c6 Parents: 7536429 Author: Benedict Elliott Smith <[email protected]> Authored: Mon Jul 7 21:10:57 2014 +0100 Committer: Benedict Elliott Smith <[email protected]> Committed: Mon Jul 7 21:12:49 2014 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/db/AbstractCell.java | 16 ++--- .../apache/cassandra/db/BufferExpiringCell.java | 22 ++++++ .../apache/cassandra/db/NativeExpiringCell.java | 22 ++++++ test/unit/org/apache/cassandra/db/CellTest.java | 76 ++++++++++++++++++++ 5 files changed, 127 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/0bc4663a/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index ff2f586..641326e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.1.0-rc3 + * Consider expiry when reconciling otherwise equal cells (CASSANDRA-7403) * Introduce CQL support for stress tool (CASSANDRA-6146) * Fix ClassCastException processing expired messages (CASSANDRA-7496) * Fix prepared marker for collections inside UDT (CASSANDRA-7472) http://git-wip-us.apache.org/repos/asf/cassandra/blob/0bc4663a/src/java/org/apache/cassandra/db/AbstractCell.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/AbstractCell.java b/src/java/org/apache/cassandra/db/AbstractCell.java index 9dad6db..82f1989 100644 --- a/src/java/org/apache/cassandra/db/AbstractCell.java +++ b/src/java/org/apache/cassandra/db/AbstractCell.java @@ -120,16 +120,12 @@ public abstract class AbstractCell implements Cell public Cell reconcile(Cell cell) { - // tombstones take precedence. (if both are tombstones, then it doesn't matter which one we use.) - if (!isLive()) - return timestamp() < cell.timestamp() ? cell : this; - if (!cell.isLive()) - return timestamp() > cell.timestamp() ? this : cell; - // break ties by comparing values. - if (timestamp() == cell.timestamp()) - return value().compareTo(cell.value()) < 0 ? cell : this; - // neither is tombstoned and timestamps are different - return timestamp() < cell.timestamp() ? cell : this; + long ts1 = timestamp(), ts2 = cell.timestamp(); + if (ts1 != ts2) + return ts1 < ts2 ? cell : this; + if (isLive() != cell.isLive()) + return isLive() ? cell : this; + return value().compareTo(cell.value()) < 0 ? cell : this; } @Override http://git-wip-us.apache.org/repos/asf/cassandra/blob/0bc4663a/src/java/org/apache/cassandra/db/BufferExpiringCell.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/BufferExpiringCell.java b/src/java/org/apache/cassandra/db/BufferExpiringCell.java index 38d84f4..a2b4f19 100644 --- a/src/java/org/apache/cassandra/db/BufferExpiringCell.java +++ b/src/java/org/apache/cassandra/db/BufferExpiringCell.java @@ -142,6 +142,28 @@ public class BufferExpiringCell extends BufferCell implements ExpiringCell throw new MarshalException("The local expiration time should not be negative"); } + public Cell reconcile(Cell cell) + { + long ts1 = timestamp(), ts2 = cell.timestamp(); + if (ts1 != ts2) + return ts1 < ts2 ? cell : this; + // we should prefer tombstones + if (cell instanceof DeletedCell) + return cell; + // however if we're both ExpiringCells, we should prefer the one with the longest ttl + // (really in preference _always_ to the value comparison) + int c = value().compareTo(cell.value()); + if (c != 0) + return c < 0 ? cell : this; + if (cell instanceof ExpiringCell) + { + int let1 = localExpirationTime, let2 = cell.getLocalDeletionTime(); + if (let1 < let2) + return cell; + } + return this; + } + @Override public boolean equals(Cell cell) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/0bc4663a/src/java/org/apache/cassandra/db/NativeExpiringCell.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/NativeExpiringCell.java b/src/java/org/apache/cassandra/db/NativeExpiringCell.java index f265511..5648375 100644 --- a/src/java/org/apache/cassandra/db/NativeExpiringCell.java +++ b/src/java/org/apache/cassandra/db/NativeExpiringCell.java @@ -128,6 +128,28 @@ public class NativeExpiringCell extends NativeCell implements ExpiringCell FBUtilities.updateWithInt(digest, getTimeToLive()); } + public Cell reconcile(Cell cell) + { + long ts1 = timestamp(), ts2 = cell.timestamp(); + if (ts1 != ts2) + return ts1 < ts2 ? cell : this; + // we should prefer tombstones + if (cell instanceof DeletedCell) + return cell; + // however if we're both ExpiringCells, we should prefer the one with the longest ttl + // (really in preference _always_ to the value comparison) + int c = value().compareTo(cell.value()); + if (c != 0) + return c < 0 ? cell : this; + if (cell instanceof ExpiringCell) + { + int let1 = getLocalDeletionTime(), let2 = cell.getLocalDeletionTime(); + if (let1 < let2) + return cell; + } + return this; + } + public boolean equals(Cell cell) { return cell instanceof ExpiringCell && equals((ExpiringCell) cell); http://git-wip-us.apache.org/repos/asf/cassandra/blob/0bc4663a/test/unit/org/apache/cassandra/db/CellTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/CellTest.java b/test/unit/org/apache/cassandra/db/CellTest.java new file mode 100644 index 0000000..668bebc --- /dev/null +++ b/test/unit/org/apache/cassandra/db/CellTest.java @@ -0,0 +1,76 @@ +package org.apache.cassandra.db; + +import org.junit.Test; + +import junit.framework.Assert; +import org.apache.cassandra.Util; +import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.concurrent.OpOrder; +import org.apache.cassandra.utils.memory.NativeAllocator; +import org.apache.cassandra.utils.memory.NativePool; + +public class CellTest +{ + + private static final OpOrder order = new OpOrder(); + private static NativeAllocator allocator = new NativePool(Integer.MAX_VALUE, Integer.MAX_VALUE, 1f, null).newAllocator(); + + @Test + public void testExpiringCellReconile() + { + // equal + Assert.assertEquals(0, testExpiring("a", "a", 1, 1, null, null, null, null)); + + // newer timestamp + Assert.assertEquals(-1, testExpiring("a", "a", 2, 1, null, null, 1L, null)); + Assert.assertEquals(-1, testExpiring("a", "a", 2, 1, null, "b", 1L, 2)); + + // newer TTL + Assert.assertEquals(-1, testExpiring("a", "a", 1, 2, null, null, null, 1)); + Assert.assertEquals(1, testExpiring("a", "a", 1, 2, null, "b", null, 1)); + + // newer value + Assert.assertEquals(-1, testExpiring("a", "b", 2, 1, null, "a", null, null)); + Assert.assertEquals(-1, testExpiring("a", "b", 2, 1, null, "a", null, 2)); + } + + private int testExpiring(String n1, String v1, long t1, int et1, String n2, String v2, Long t2, Integer et2) + { + if (n2 == null) + n2 = n1; + if (v2 == null) + v2 = v1; + if (t2 == null) + t2 = t1; + if (et2 == null) + et2 = et1; + int result = testExpiring(n1, v1, t1, et1, false, n2, v2, t2, et2, false); + Assert.assertEquals(result, testExpiring(n1, v1, t1, et1, false, n2, v2, t2, et2, true)); + Assert.assertEquals(result, testExpiring(n1, v1, t1, et1, true, n2, v2, t2, et2, false)); + Assert.assertEquals(result, testExpiring(n1, v1, t1, et1, true, n2, v2, t2, et2, true)); + return result; + } + + private int testExpiring(String n1, String v1, long t1, int et1, boolean native1, String n2, String v2, long t2, int et2, boolean native2) + { + Cell c1 = expiring(n1, v1, t1, et1, native1); + Cell c2 = expiring(n2, v2, t2, et2, native2); + return reconcile(c1, c2); + } + + int reconcile(Cell c1, Cell c2) + { + if (c1.reconcile(c2) == c1) + return c2.reconcile(c1) == c1 ? -1 : 0; + return c2.reconcile(c1) == c2 ? 1 : 0; + } + + private Cell expiring(String name, String value, long timestamp, int expirationTime, boolean nativeCell) + { + ExpiringCell cell = new BufferExpiringCell(Util.cellname(name), ByteBufferUtil.bytes(value), timestamp, 1, expirationTime); + if (nativeCell) + cell = new NativeExpiringCell(allocator, order.getCurrent(), cell); + return cell; + } + +}
