cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jbel...@apache.org
Subject [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
Date Wed, 25 Jan 2012 21:58:41 GMT
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 <jbellis@apache.org>
Authored: Wed Jan 25 15:46:48 2012 -0600
Committer: Jonathan Ellis <jbellis@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)
     }
 


Mime
View raw message