You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2014/07/08 14:00:45 UTC
git commit: Followup for CASSANDRA-7403
Repository: cassandra
Updated Branches:
refs/heads/cassandra-2.1.0 0bc4663aa -> 8733de644
Followup 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/8733de64
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/8733de64
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/8733de64
Branch: refs/heads/cassandra-2.1.0
Commit: 8733de64409ad8fdca9ddbd3b5dd7476e3e33d77
Parents: 0bc4663
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Tue Jul 8 14:00:35 2014 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Tue Jul 8 14:00:35 2014 +0200
----------------------------------------------------------------------
.../apache/cassandra/db/BufferExpiringCell.java | 4 +--
.../apache/cassandra/db/NativeExpiringCell.java | 4 +--
.../AbstractSimplePerColumnSecondaryIndex.java | 5 ++--
.../db/index/SecondaryIndexManager.java | 31 ++++++++++----------
4 files changed, 23 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8733de64/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 a2b4f19..347604a 100644
--- a/src/java/org/apache/cassandra/db/BufferExpiringCell.java
+++ b/src/java/org/apache/cassandra/db/BufferExpiringCell.java
@@ -142,6 +142,7 @@ public class BufferExpiringCell extends BufferCell implements ExpiringCell
throw new MarshalException("The local expiration time should not be negative");
}
+ @Override
public Cell reconcile(Cell cell)
{
long ts1 = timestamp(), ts2 = cell.timestamp();
@@ -150,11 +151,10 @@ public class BufferExpiringCell extends BufferCell implements ExpiringCell
// 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 we have same timestamp and value, prefer the longest ttl
if (cell instanceof ExpiringCell)
{
int let1 = localExpirationTime, let2 = cell.getLocalDeletionTime();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8733de64/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 5648375..d97e080 100644
--- a/src/java/org/apache/cassandra/db/NativeExpiringCell.java
+++ b/src/java/org/apache/cassandra/db/NativeExpiringCell.java
@@ -128,6 +128,7 @@ public class NativeExpiringCell extends NativeCell implements ExpiringCell
FBUtilities.updateWithInt(digest, getTimeToLive());
}
+ @Override
public Cell reconcile(Cell cell)
{
long ts1 = timestamp(), ts2 = cell.timestamp();
@@ -136,11 +137,10 @@ public class NativeExpiringCell extends NativeCell implements ExpiringCell
// 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 we have same timestamp and value, prefer the longest ttl
if (cell instanceof ExpiringCell)
{
int let1 = getLocalDeletionTime(), let2 = cell.getLocalDeletionTime();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8733de64/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java b/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java
index b64d84f..a2011b6 100644
--- a/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java
+++ b/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java
@@ -121,11 +121,12 @@ public abstract class AbstractSimplePerColumnSecondaryIndex extends PerColumnSec
}
public void update(ByteBuffer rowKey, Cell oldCol, Cell col, OpOrder.Group opGroup)
- {
+ {
// insert the new value before removing the old one, so we never have a period
// where the row is invisible to both queries (the opposite seems preferable); see CASSANDRA-5540
insert(rowKey, col, opGroup);
- delete(rowKey, oldCol, opGroup);
+ if (SecondaryIndexManager.shouldCleanupOldValue(oldCol, col))
+ delete(rowKey, oldCol, opGroup);
}
public void removeIndex(ByteBuffer columnName)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8733de64/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java b/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java
index f78dc86..d32306a 100644
--- a/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java
+++ b/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java
@@ -620,6 +620,22 @@ public class SecondaryIndexManager
return true;
}
+ static boolean shouldCleanupOldValue(Cell oldCell, Cell newCell)
+ {
+ // If any one of name/value/timestamp are different, then we
+ // should delete from the index. If not, then we can infer that
+ // at least one of the cells is an ExpiringColumn and that the
+ // difference is in the expiry time. In this case, we don't want to
+ // delete the old value from the index as the tombstone we insert
+ // will just hide the inserted value.
+ // Completely identical cells (including expiring columns with
+ // identical ttl & localExpirationTime) will not get this far due
+ // to the oldCell.equals(newColumn) in StandardUpdater.update
+ return !oldCell.name().equals(newCell.name())
+ || !oldCell.value().equals(newCell.value())
+ || oldCell.timestamp() != newCell.timestamp();
+ }
+
public static interface Updater
{
/** called when constructing the index against pre-existing data */
@@ -744,20 +760,5 @@ public class SecondaryIndexManager
((PerRowSecondaryIndex) index).index(key.getKey(), cf);
}
- private boolean shouldCleanupOldValue(Cell oldCell, Cell newCell)
- {
- // If any one of name/value/timestamp are different, then we
- // should delete from the index. If not, then we can infer that
- // at least one of the cells is an ExpiringColumn and that the
- // difference is in the expiry time. In this case, we don't want to
- // delete the old value from the index as the tombstone we insert
- // will just hide the inserted value.
- // Completely identical cells (including expiring columns with
- // identical ttl & localExpirationTime) will not get this far due
- // to the oldCell.equals(newColumn) in StandardUpdater.update
- return !oldCell.name().equals(newCell.name())
- || !oldCell.value().equals(newCell.value())
- || oldCell.timestamp() != newCell.timestamp();
- }
}
}