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 2016/07/20 12:30:52 UTC

[2/6] cassandra git commit: NullPointerExpception when reading/compacting table

NullPointerExpception when reading/compacting table

patch by Sylvain Lebresne; reviewed by Carl Yeksigian for CASSANDRA-11988


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/979e5590
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/979e5590
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/979e5590

Branch: refs/heads/cassandra-3.9
Commit: 979e5590e0c2e7c33e5ffb6e56c67d855fc17813
Parents: dce4537
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Tue Jul 19 15:07:37 2016 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Jul 20 14:27:18 2016 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../db/partitions/AbstractBTreePartition.java   |  2 +-
 .../cassandra/db/partitions/PurgeFunction.java  | 15 ++++++++-----
 .../cassandra/db/rows/BaseRowIterator.java      |  2 +-
 src/java/org/apache/cassandra/db/rows/Row.java  | 13 +++++++++---
 .../apache/cassandra/db/transform/BaseRows.java |  2 +-
 .../apache/cassandra/db/transform/Filter.java   | 12 +++++++----
 .../validation/entities/StaticColumnsTest.java  | 22 ++++++++++++++++++++
 8 files changed, 54 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/979e5590/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 739c095..6ea39cd 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.9
+ * NullPointerExpception when reading/compacting table (CASSANDRA-11988)
  * Fix problem with undeleteable rows on upgrade to new sstable format (CASSANDRA-12144)
  * Fix paging logic for deleted partitions with static columns (CASSANDRA-12107)
  * Wait until the message is being send to decide which serializer must be used (CASSANDRA-11393)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/979e5590/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java b/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java
index b7fa691..1fa3324 100644
--- a/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java
+++ b/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java
@@ -60,7 +60,7 @@ public abstract class AbstractBTreePartition implements Partition, Iterable<Row>
             this.columns = columns;
             this.tree = tree;
             this.deletionInfo = deletionInfo;
-            this.staticRow = staticRow;
+            this.staticRow = staticRow == null ? Rows.EMPTY_STATIC_ROW : staticRow;
             this.stats = stats;
         }
     }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/979e5590/src/java/org/apache/cassandra/db/partitions/PurgeFunction.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/partitions/PurgeFunction.java b/src/java/org/apache/cassandra/db/partitions/PurgeFunction.java
index 492bab1..d3255d3 100644
--- a/src/java/org/apache/cassandra/db/partitions/PurgeFunction.java
+++ b/src/java/org/apache/cassandra/db/partitions/PurgeFunction.java
@@ -55,7 +55,8 @@ public abstract class PurgeFunction extends Transformation<UnfilteredRowIterator
     {
     }
 
-    public UnfilteredRowIterator applyToPartition(UnfilteredRowIterator partition)
+    @Override
+    protected UnfilteredRowIterator applyToPartition(UnfilteredRowIterator partition)
     {
         onNewPartition(partition.partitionKey());
 
@@ -71,24 +72,28 @@ public abstract class PurgeFunction extends Transformation<UnfilteredRowIterator
         return purged;
     }
 
-    public DeletionTime applyToDeletion(DeletionTime deletionTime)
+    @Override
+    protected DeletionTime applyToDeletion(DeletionTime deletionTime)
     {
         return purger.shouldPurge(deletionTime) ? DeletionTime.LIVE : deletionTime;
     }
 
-    public Row applyToStatic(Row row)
+    @Override
+    protected Row applyToStatic(Row row)
     {
         updateProgress();
         return row.purge(purger, nowInSec);
     }
 
-    public Row applyToRow(Row row)
+    @Override
+    protected Row applyToRow(Row row)
     {
         updateProgress();
         return row.purge(purger, nowInSec);
     }
 
-    public RangeTombstoneMarker applyToMarker(RangeTombstoneMarker marker)
+    @Override
+    protected RangeTombstoneMarker applyToMarker(RangeTombstoneMarker marker)
     {
         updateProgress();
         boolean reversed = isReverseOrder;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/979e5590/src/java/org/apache/cassandra/db/rows/BaseRowIterator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/rows/BaseRowIterator.java b/src/java/org/apache/cassandra/db/rows/BaseRowIterator.java
index fb9e908..ce37297 100644
--- a/src/java/org/apache/cassandra/db/rows/BaseRowIterator.java
+++ b/src/java/org/apache/cassandra/db/rows/BaseRowIterator.java
@@ -53,7 +53,7 @@ public interface BaseRowIterator<U extends Unfiltered> extends CloseableIterator
 
     /**
      * The static part corresponding to this partition (this can be an empty
-     * row).
+     * row but cannot be {@code null}).
      */
     public Row staticRow();
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/979e5590/src/java/org/apache/cassandra/db/rows/Row.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/rows/Row.java b/src/java/org/apache/cassandra/db/rows/Row.java
index 82c07a7..c7c3216 100644
--- a/src/java/org/apache/cassandra/db/rows/Row.java
+++ b/src/java/org/apache/cassandra/db/rows/Row.java
@@ -200,7 +200,8 @@ public interface Row extends Unfiltered, Collection<ColumnData>
      *
      * @param purger the {@code DeletionPurger} to use to decide what can be purged.
      * @param nowInSec the current time to decide what is deleted and what isn't (in the case of expired cells).
-     * @return this row but without any deletion info purged by {@code purger}.
+     * @return this row but without any deletion info purged by {@code purger}. If the purged row is empty, returns
+     * {@code null}.
      */
     public Row purge(DeletionPurger purger, int nowInSec);
 
@@ -210,8 +211,14 @@ public interface Row extends Unfiltered, Collection<ColumnData>
     public Row markCounterLocalToBeCleared();
 
     /**
-     * returns a copy of this row where all live timestamp have been replaced by {@code newTimestamp} and every deletion timestamp
-     * by {@code newTimestamp - 1}. See {@link Commit} for why we need this.
+     * Returns a copy of this row where all live timestamp have been replaced by {@code newTimestamp} and every deletion
+     * timestamp by {@code newTimestamp - 1}.
+     *
+     * @param newTimestamp the timestamp to use for all live data in the returned row.
+     * @param a copy of this row with timestamp updated using {@code newTimestamp}. This can return {@code null} in the
+     * rare where the row only as a shadowable row deletion and the new timestamp supersedes it.
+     *
+     * @see Commit for why we need this.
      */
     public Row updateAllTimestamp(long newTimestamp);
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/979e5590/src/java/org/apache/cassandra/db/transform/BaseRows.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/transform/BaseRows.java b/src/java/org/apache/cassandra/db/transform/BaseRows.java
index b0e642b..7b0bb99 100644
--- a/src/java/org/apache/cassandra/db/transform/BaseRows.java
+++ b/src/java/org/apache/cassandra/db/transform/BaseRows.java
@@ -69,7 +69,7 @@ implements BaseRowIterator<R>
 
     public Row staticRow()
     {
-        return staticRow;
+        return staticRow == null ? Rows.EMPTY_STATIC_ROW : staticRow;
     }
 
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/979e5590/src/java/org/apache/cassandra/db/transform/Filter.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/transform/Filter.java b/src/java/org/apache/cassandra/db/transform/Filter.java
index 138d3c8..48c8b1a 100644
--- a/src/java/org/apache/cassandra/db/transform/Filter.java
+++ b/src/java/org/apache/cassandra/db/transform/Filter.java
@@ -33,7 +33,8 @@ final class Filter extends Transformation
         this.nowInSec = nowInSec;
     }
 
-    public RowIterator applyToPartition(BaseRowIterator iterator)
+    @Override
+    protected RowIterator applyToPartition(BaseRowIterator iterator)
     {
         RowIterator filtered = iterator instanceof UnfilteredRows
                                ? new FilteredRows(this, (UnfilteredRows) iterator)
@@ -45,7 +46,8 @@ final class Filter extends Transformation
         return filtered;
     }
 
-    public Row applyToStatic(Row row)
+    @Override
+    protected Row applyToStatic(Row row)
     {
         if (row.isEmpty())
             return Rows.EMPTY_STATIC_ROW;
@@ -54,12 +56,14 @@ final class Filter extends Transformation
         return row == null ? Rows.EMPTY_STATIC_ROW : row;
     }
 
-    public Row applyToRow(Row row)
+    @Override
+    protected Row applyToRow(Row row)
     {
         return row.purge(DeletionPurger.PURGE_ALL, nowInSec);
     }
 
-    public RangeTombstoneMarker applyToMarker(RangeTombstoneMarker marker)
+    @Override
+    protected RangeTombstoneMarker applyToMarker(RangeTombstoneMarker marker)
     {
         return null;
     }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/979e5590/test/unit/org/apache/cassandra/cql3/validation/entities/StaticColumnsTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/StaticColumnsTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/StaticColumnsTest.java
index cef6f1f..75cbcc7 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/entities/StaticColumnsTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/entities/StaticColumnsTest.java
@@ -268,4 +268,26 @@ public class StaticColumnsTest extends CQLTester
         // We shouldn 't allow static when there is not clustering columns
         assertInvalid("ALTER TABLE %s ADD bar2 text static");
     }
+
+    /**
+     * Ensure that deleting and compacting a static row that should be purged doesn't throw.
+     * This is a test for #11988.
+     */
+    @Test
+    public void testStaticColumnPurging() throws Throwable
+    {
+        createTable("CREATE TABLE %s (pkey text, ckey text, value text, static_value text static, PRIMARY KEY(pkey, ckey)) WITH gc_grace_seconds = 0");
+
+        execute("INSERT INTO %s (pkey, ckey, static_value, value) VALUES (?, ?, ?, ?)", "k1", "c1", "s1", "v1");
+
+        flush();
+
+        execute("DELETE static_value FROM %s WHERE pkey = ?", "k1");
+
+        flush();
+
+        compact();
+
+        assertRows(execute("SELECT * FROM %s"), row("k1", "c1", null, "v1"));
+    }
 }