You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by be...@apache.org on 2014/07/07 22:13:20 UTC
[3/6] git commit: Consider expiry when reconciling otherwise equal
cells
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/trunk
Commit: 0bc4663aad3257f359058465dccbb36141fc75c6
Parents: 7536429
Author: Benedict Elliott Smith <be...@apache.org>
Authored: Mon Jul 7 21:10:57 2014 +0100
Committer: Benedict Elliott Smith <be...@apache.org>
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;
+ }
+
+}