You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jb...@apache.org on 2012/01/25 22:58:41 UTC
[2/2] git commit: Clean up isMarkedForDelete to avoid relying on the
current time where possible patch by slebresne;
reviewed by jbellis for CASSANDRA-3716
Clean up isMarkedForDelete to avoid relying on the current time where possible
patch by slebresne; reviewed by jbellis for CASSANDRA-3716
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/e0aec20f
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/e0aec20f
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/e0aec20f
Branch: refs/heads/trunk
Commit: e0aec20f451515d23c84d1ce61666b43dd928f11
Parents: 45e1d51
Author: Jonathan Ellis <jb...@apache.org>
Authored: Wed Jan 25 15:46:48 2012 -0600
Committer: Jonathan Ellis <jb...@apache.org>
Committed: Wed Jan 25 15:58:26 2012 -0600
----------------------------------------------------------------------
.../cassandra/db/AbstractColumnContainer.java | 2 +-
src/java/org/apache/cassandra/db/Column.java | 6 ++--
.../org/apache/cassandra/db/ColumnFamilyStore.java | 7 +----
.../org/apache/cassandra/db/DeletedColumn.java | 6 -----
.../org/apache/cassandra/db/ExpiringColumn.java | 18 ---------------
src/java/org/apache/cassandra/db/IColumn.java | 5 ++++
.../apache/cassandra/db/filter/QueryFilter.java | 2 +-
7 files changed, 12 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e0aec20f/src/java/org/apache/cassandra/db/AbstractColumnContainer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/AbstractColumnContainer.java b/src/java/org/apache/cassandra/db/AbstractColumnContainer.java
index 3baba86..a87985e 100644
--- a/src/java/org/apache/cassandra/db/AbstractColumnContainer.java
+++ b/src/java/org/apache/cassandra/db/AbstractColumnContainer.java
@@ -197,7 +197,7 @@ public abstract class AbstractColumnContainer implements IColumnContainer, IIter
public boolean hasExpiredTombstones(int gcBefore)
{
- if (isMarkedForDelete() && getLocalDeletionTime() < gcBefore)
+ if (getLocalDeletionTime() < gcBefore)
return true;
for (IColumn column : columns)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e0aec20f/src/java/org/apache/cassandra/db/Column.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/Column.java b/src/java/org/apache/cassandra/db/Column.java
index 367cc13..c5735dd 100644
--- a/src/java/org/apache/cassandra/db/Column.java
+++ b/src/java/org/apache/cassandra/db/Column.java
@@ -106,7 +106,7 @@ public class Column implements IColumn
public boolean isMarkedForDelete()
{
- return false;
+ return (int) (System.currentTimeMillis() / 1000) >= getLocalDeletionTime();
}
public long getMarkedForDeleteAt()
@@ -185,7 +185,7 @@ public class Column implements IColumn
public int getLocalDeletionTime()
{
- throw new IllegalStateException("column is not marked for delete");
+ return Integer.MAX_VALUE;
}
public IColumn reconcile(IColumn column)
@@ -278,7 +278,7 @@ public class Column implements IColumn
public boolean hasExpiredTombstones(int gcBefore)
{
- return isMarkedForDelete() && getLocalDeletionTime() < gcBefore;
+ return getLocalDeletionTime() < gcBefore;
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e0aec20f/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index 0edae3f..15a0b64 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -750,10 +750,7 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
// remove columns if
// (a) the column itself is tombstoned or
// (b) the CF is tombstoned and the column is not newer than it
- //
- // Note that we need the inequality below for case (a) to be strict for expiring columns
- // to work correctly -- see the comment in ExpiringColumn.isMarkedForDelete().
- if ((c.isMarkedForDelete() && c.getLocalDeletionTime() < gcBefore)
+ if (c.getLocalDeletionTime() < gcBefore
|| c.timestamp() <= cf.getMarkedForDeleteAt())
{
iter.remove();
@@ -779,7 +776,7 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
// (a) the subcolumn itself is tombstoned or
// (b) the supercolumn is tombstoned and the subcolumn is not newer than it
if (subColumn.timestamp() <= minTimestamp
- || (subColumn.isMarkedForDelete() && subColumn.getLocalDeletionTime() < gcBefore))
+ || subColumn.getLocalDeletionTime() < gcBefore)
{
subIter.remove();
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e0aec20f/src/java/org/apache/cassandra/db/DeletedColumn.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/DeletedColumn.java b/src/java/org/apache/cassandra/db/DeletedColumn.java
index eff8c5c..c1986ae 100644
--- a/src/java/org/apache/cassandra/db/DeletedColumn.java
+++ b/src/java/org/apache/cassandra/db/DeletedColumn.java
@@ -39,12 +39,6 @@ public class DeletedColumn extends Column
}
@Override
- public boolean isMarkedForDelete()
- {
- return true;
- }
-
- @Override
public long getMarkedForDeleteAt()
{
return timestamp;
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e0aec20f/src/java/org/apache/cassandra/db/ExpiringColumn.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/ExpiringColumn.java b/src/java/org/apache/cassandra/db/ExpiringColumn.java
index 1309e24..adf1913 100644
--- a/src/java/org/apache/cassandra/db/ExpiringColumn.java
+++ b/src/java/org/apache/cassandra/db/ExpiringColumn.java
@@ -75,24 +75,6 @@ public class ExpiringColumn extends Column
}
@Override
- public boolean isMarkedForDelete()
- {
- /*
- * For compaction, we need to ensure that at all time if
- * localExpirationTime < gcbefore, then isMarkedForDelete() == true
- * (otherwise LCR may expire columns between it's two phases compaction -- see #3579).
- *
- * Since during compaction we know that at all time, gcbefore <= now
- * (the = is important in case where gc_grace=0), it follows that to
- * ensure the propery above we need for isMarkedForDelete to be
- * now > localExpirationTime (*not* now >= localExpiration). For the
- * same reason, compaction should consider a column tomstoned if
- * getLocalDeletionTime() < gcbefore, *not* if getLocalDeletionTime() <= gcbefore.
- */
- return (int) (System.currentTimeMillis() / 1000 ) > localExpirationTime;
- }
-
- @Override
public int size()
{
/*
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e0aec20f/src/java/org/apache/cassandra/db/IColumn.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/IColumn.java b/src/java/org/apache/cassandra/db/IColumn.java
index e75d069..903fa12 100644
--- a/src/java/org/apache/cassandra/db/IColumn.java
+++ b/src/java/org/apache/cassandra/db/IColumn.java
@@ -33,7 +33,12 @@ public interface IColumn
{
public static final int MAX_NAME_LENGTH = FBUtilities.MAX_UNSIGNED_SHORT;
+ /**
+ * @return true if the column has been deleted (is a tombstone). This depends on comparing the server clock
+ * with getLocalDeletionTime, so it can change during a single request if you're not careful.
+ */
public boolean isMarkedForDelete();
+
public long getMarkedForDeleteAt();
public long mostRecentLiveChangeAt();
public ByteBuffer name();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e0aec20f/src/java/org/apache/cassandra/db/filter/QueryFilter.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/filter/QueryFilter.java b/src/java/org/apache/cassandra/db/filter/QueryFilter.java
index 8dc0dd4..e647957 100644
--- a/src/java/org/apache/cassandra/db/filter/QueryFilter.java
+++ b/src/java/org/apache/cassandra/db/filter/QueryFilter.java
@@ -150,7 +150,7 @@ public class QueryFilter
// 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)
+ return (column.getLocalDeletionTime() >= gcBefore || maxChange > column.getMarkedForDeleteAt()) // (1)
&& (!container.isMarkedForDelete() || maxChange > container.getMarkedForDeleteAt()); // (2)
}