Repository: cassandra Updated Branches: refs/heads/cassandra-2.1 ec38458d6 -> 04944f0ca
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 Commit: 8733de64409ad8fdca9ddbd3b5dd7476e3e33d77 Parents: 0bc4663 Author: Sylvain Lebresne <[email protected]> Authored: Tue Jul 8 14:00:35 2014 +0200 Committer: Sylvain Lebresne <[email protected]> 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(); - } } }
