cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject git commit: Fix potential NPE in 2ndary indexes
Date Tue, 01 Oct 2013 07:01:57 GMT
Updated Branches:
  refs/heads/cassandra-2.0 65b1e36c7 -> 58eebc9a9


Fix potential NPE in 2ndary indexes

patch by slebresne; reviewed by jbellis for CASSANDRA-6098


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

Branch: refs/heads/cassandra-2.0
Commit: 58eebc9a977ef7dfafe6ea568cece963191598ce
Parents: 65b1e36
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Tue Oct 1 08:59:43 2013 +0200
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Tue Oct 1 08:59:43 2013 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../CompositesIndexOnClusteringKey.java         |  3 +-
 .../CompositesIndexOnPartitionKey.java          |  4 +-
 .../db/index/composites/CompositesSearcher.java |  2 +-
 .../unit/org/apache/cassandra/SchemaLoader.java |  3 +-
 .../cassandra/db/ColumnFamilyStoreTest.java     | 48 ++++++++++++++++++++
 6 files changed, 54 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/58eebc9a/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 465c718..0fa788a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -10,6 +10,7 @@
    leveled manifest (CASSANDRA-6093)
  * make sequential nodetool repair the default (CASSANDRA-5950)
  * Add more hooks for compaction strategy implementations (CASSANDRA-6111)
+ * Fix potential NPE on composite 2ndary indexes (CASSANDRA-6098)
 Merged from 1.2:
  * lock access to TM.endpointToHostIdMap (CASSANDRA-6103)
  * Allow estimated memtable size to exceed slab allocator size (CASSANDRA-6078)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/58eebc9a/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnClusteringKey.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnClusteringKey.java
b/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnClusteringKey.java
index bdb544c..954f380 100644
--- a/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnClusteringKey.java
+++ b/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnClusteringKey.java
@@ -110,7 +110,6 @@ public class CompositesIndexOnClusteringKey extends CompositesIndex
 
     public boolean isStale(IndexedEntry entry, ColumnFamily data, long now)
     {
-        return data == null || data.hasOnlyTombstones(now);
+        return data.hasOnlyTombstones(now);
     }
 }
-

http://git-wip-us.apache.org/repos/asf/cassandra/blob/58eebc9a/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnPartitionKey.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnPartitionKey.java
b/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnPartitionKey.java
index 7a00de8..4e2c580 100644
--- a/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnPartitionKey.java
+++ b/src/java/org/apache/cassandra/db/index/composites/CompositesIndexOnPartitionKey.java
@@ -97,8 +97,6 @@ public class CompositesIndexOnPartitionKey extends CompositesIndex
 
     public boolean isStale(IndexedEntry entry, ColumnFamily data, long now)
     {
-        return data == null || data.hasOnlyTombstones(now);
+        return data.hasOnlyTombstones(now);
     }
 }
-
-

http://git-wip-us.apache.org/repos/asf/cassandra/blob/58eebc9a/src/java/org/apache/cassandra/db/index/composites/CompositesSearcher.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/index/composites/CompositesSearcher.java b/src/java/org/apache/cassandra/db/index/composites/CompositesSearcher.java
index 0c2cfbc..0242fd2 100644
--- a/src/java/org/apache/cassandra/db/index/composites/CompositesSearcher.java
+++ b/src/java/org/apache/cassandra/db/index/composites/CompositesSearcher.java
@@ -240,7 +240,7 @@ public class CompositesSearcher extends SecondaryIndexSearcher
                                                                            Integer.MAX_VALUE,
                                                                            baseCfs.metadata.clusteringKeyColumns().size());
                         ColumnFamily newData = baseCfs.getColumnFamily(new QueryFilter(dk,
baseCfs.name, dataFilter, filter.timestamp));
-                        if (index.isStale(entry, newData, filter.timestamp))
+                        if (newData == null || index.isStale(entry, newData, filter.timestamp))
                         {
                             index.delete(entry);
                             continue;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/58eebc9a/test/unit/org/apache/cassandra/SchemaLoader.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/SchemaLoader.java b/test/unit/org/apache/cassandra/SchemaLoader.java
index f933b6f..55cd329 100644
--- a/test/unit/org/apache/cassandra/SchemaLoader.java
+++ b/test/unit/org/apache/cassandra/SchemaLoader.java
@@ -222,7 +222,8 @@ public class SchemaLoader
                                            superCFMD(ks2, "Super3", bytes),
                                            superCFMD(ks2, "Super4", TimeUUIDType.instance),
                                            indexCFMD(ks2, "Indexed1", true),
-                                           compositeIndexCFMD(ks2, "Indexed2", true, withOldCfIds)));
+                                           compositeIndexCFMD(ks2, "Indexed2", true, withOldCfIds),
+                                           compositeIndexCFMD(ks2, "Indexed3", true, withOldCfIds).gcGraceSeconds(0)));
 
         // Keyspace 3
         schema.add(KSMetaData.testMetadata(ks3,

http://git-wip-us.apache.org/repos/asf/cassandra/blob/58eebc9a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
index 0a78b2a..f017453 100644
--- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
+++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
@@ -554,6 +554,54 @@ public class ColumnFamilyStoreTest extends SchemaLoader
         assertEquals(0, rows.size());
     }
 
+    // See CASSANDRA-6098
+    @Test
+    public void testDeleteCompositeIndex() throws Exception
+    {
+        String keySpace = "Keyspace2";
+        String cfName = "Indexed3"; // has gcGrace 0
+
+        Keyspace keyspace = Keyspace.open(keySpace);
+        ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(cfName);
+        cfs.truncateBlocking();
+
+        ByteBuffer rowKey = ByteBufferUtil.bytes("k1");
+        ByteBuffer clusterKey = ByteBufferUtil.bytes("ck1");
+        ByteBuffer colName = ByteBufferUtil.bytes("col1");
+        CompositeType baseComparator = (CompositeType)cfs.getComparator();
+        CompositeType.Builder builder = baseComparator.builder();
+        builder.add(clusterKey);
+        builder.add(colName);
+        ByteBuffer compositeName = builder.build();
+
+        ByteBuffer val1 = ByteBufferUtil.bytes("v2");
+
+        // Insert indexed value.
+        RowMutation rm;
+        rm = new RowMutation(keySpace, rowKey);
+        rm.add(cfName, compositeName, val1, 0);
+        rm.apply();
+
+        // Now delete the value and flush too.
+        rm = new RowMutation(keySpace, rowKey);
+        rm.delete(cfName, 1);
+        rm.apply();
+
+        // We want the data to be gcable, but even if gcGrace == 0, we still need to wait
1 second
+        // since we won't gc on a tie.
+        try { Thread.sleep(1000); } catch (Exception e) {}
+
+        // Read the index and we check we do get no value (and no NPE)
+        // Note: the index will return the entry because it hasn't been deleted (we
+        // haven't read yet nor compacted) but the data read itself will return null
+        IndexExpression expr = new IndexExpression(colName, IndexOperator.EQ, val1);
+        List<IndexExpression> clause = Arrays.asList(expr);
+        IDiskAtomFilter filter = new IdentityQueryFilter();
+        Range<RowPosition> range = Util.range("", "");
+        List<Row> rows = keyspace.getColumnFamilyStore(cfName).search(range, clause,
filter, 100);
+        assertEquals(0, rows.size());
+    }
+
     // See CASSANDRA-2628
     @Test
     public void testIndexScanWithLimitOne() throws IOException


Mime
View raw message