cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [1/3] cassandra git commit: Remove Index.indexes() method from 2ndary index API
Date Thu, 03 Dec 2015 14:02:35 GMT
Repository: cassandra
Updated Branches:
  refs/heads/trunk 0cb117cd3 -> 4f5845eb5


Remove Index.indexes() method from 2ndary index API

patch by slebresne; reviewed by beobal for CASSANDRA-10690


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

Branch: refs/heads/trunk
Commit: fffa6d8e668dbc10b6e79e4aa1bec54c35978212
Parents: 9784be5
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Tue Nov 17 17:48:00 2015 +0100
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Thu Dec 3 15:00:07 2015 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 NEWS.txt                                        |  3 +
 src/java/org/apache/cassandra/index/Index.java  | 20 +++---
 .../cassandra/index/SecondaryIndexManager.java  | 65 +++++++++-----------
 .../index/internal/CassandraIndex.java          | 26 +++++---
 .../org/apache/cassandra/index/StubIndex.java   |  6 +-
 .../index/internal/CustomCassandraIndex.java    | 10 ++-
 7 files changed, 62 insertions(+), 69 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/fffa6d8e/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 4dd1b97..507a709 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.1
+ * Remove unclear Indexer.indexes() method (CASSANDRA-10690)
  * Fix NPE on stream read error (CASSANDRA-10771)
  * Normalize cqlsh DESC output (CASSANDRA-10431)
  * Rejects partition range deletions when columns are specified (CASSANDRA-10739)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/fffa6d8e/NEWS.txt
----------------------------------------------------------------------
diff --git a/NEWS.txt b/NEWS.txt
index 9cebf58..b3c304a 100644
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -20,6 +20,9 @@ Upgrading
 ---------
    - The return value of SelectStatement::getLimit as been changed from DataLimits
      to int.
+   - Custom index implementation should be aware that the method Indexer::indexes()
+     has been removed as its contract was misleading and all custom implementation
+     should have almost surely returned true inconditionally for that method.
 
 
 3.0

http://git-wip-us.apache.org/repos/asf/cassandra/blob/fffa6d8e/src/java/org/apache/cassandra/index/Index.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/index/Index.java b/src/java/org/apache/cassandra/index/Index.java
index b6c12a9..084d0e3 100644
--- a/src/java/org/apache/cassandra/index/Index.java
+++ b/src/java/org/apache/cassandra/index/Index.java
@@ -191,13 +191,6 @@ public interface Index
      */
 
     /**
-     * Called to determine whether this index should process a particular partition update.
-     * @param columns
-     * @return
-     */
-    public boolean indexes(PartitionColumns columns);
-
-    /**
      * Called to determine whether this index targets a specific column.
      * Used during schema operations such as when dropping or renaming a column, to check
if
      * the index will be affected by the change. Typically, if an index answers that it does
@@ -275,19 +268,22 @@ public interface Index
      */
 
     /**
-     * Factory method for write time event handlers.
-     * Callers should check the indexes method first and only get a new
-     * handler when the index claims an interest in the specific update
-     * otherwise work may be done unnecessarily
+     * Creates an new {@code Indexer} object for updates to a given partition.
      *
      * @param key key of the partition being modified
+     * @param columns the regular and static columns the created indexer will have to deal
with.
+     * This can be empty as an update might only contain partition, range and row deletions,
but
+     * the indexer is guaranteed to not get any cells for a column that is not part of {@code
columns}.
      * @param nowInSec current time of the update operation
      * @param opGroup operation group spanning the update operation
      * @param transactionType indicates what kind of update is being performed on the base
data
      *                        i.e. a write time insert/update/delete or the result of compaction
-     * @return
+     * @return the newly created indexer or {@code null} if the index is not interested by
the update
+     * (this could be because the index doesn't care about that particular partition, doesn't
care about
+     * that type of transaction, ...).
      */
     public Indexer indexerFor(DecoratedKey key,
+                              PartitionColumns columns,
                               int nowInSec,
                               OpOrder.Group opGroup,
                               IndexTransaction.Type transactionType);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/fffa6d8e/src/java/org/apache/cassandra/index/SecondaryIndexManager.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java
index ba2c680..16cb9c4 100644
--- a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java
+++ b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java
@@ -524,9 +524,11 @@ public class SecondaryIndexManager implements IndexRegistry
             DecoratedKey key = partition.partitionKey();
             Set<Index.Indexer> indexers = indexes.stream()
                                                  .map(index -> index.indexerFor(key,
+                                                                                partition.columns(),
                                                                                 nowInSec,
                                                                                 opGroup,
                                                                                 IndexTransaction.Type.UPDATE))
+                                                 .filter(Objects::nonNull)
                                                  .collect(Collectors.toSet());
 
             indexers.forEach(Index.Indexer::begin);
@@ -666,10 +668,8 @@ public class SecondaryIndexManager implements IndexRegistry
      */
     public void validate(PartitionUpdate update) throws InvalidRequestException
     {
-        indexes.values()
-               .stream()
-               .filter(i -> i.indexes(update.columns()))
-               .forEach(i -> i.validate(update));
+        for (Index index : indexes.values())
+            index.validate(update);
     }
 
     /**
@@ -720,15 +720,13 @@ public class SecondaryIndexManager implements IndexRegistry
         if (!hasIndexes())
             return UpdateTransaction.NO_OP;
 
-        // todo : optimize lookup, we can probably cache quite a bit of stuff, rather than
doing
-        // a linear scan every time. Holding off that though until CASSANDRA-7771 to figure
out
-        // exactly how indexes are to be identified & associated with a given partition
update
         Index.Indexer[] indexers = indexes.values().stream()
-                                          .filter(i -> i.indexes(update.columns()))
                                           .map(i -> i.indexerFor(update.partitionKey(),
+                                                                 update.columns(),
                                                                  nowInSec,
                                                                  opGroup,
                                                                  IndexTransaction.Type.UPDATE))
+                                          .filter(Objects::nonNull)
                                           .toArray(Index.Indexer[]::new);
 
         return indexers.length == 0 ? UpdateTransaction.NO_OP : new WriteTimeTransaction(indexers);
@@ -743,14 +741,7 @@ public class SecondaryIndexManager implements IndexRegistry
                                                           int nowInSec)
     {
         // the check for whether there are any registered indexes is already done in CompactionIterator
-
-        Index[] interestedIndexes = indexes.values().stream()
-                                           .filter(i -> i.indexes(partitionColumns))
-                                           .toArray(Index[]::new);
-
-        return interestedIndexes.length == 0
-               ? CompactionTransaction.NO_OP
-               : new IndexGCTransaction(key, versions, nowInSec, interestedIndexes);
+        return new IndexGCTransaction(key, partitionColumns, versions, nowInSec, listIndexes());
     }
 
     /**
@@ -760,17 +751,10 @@ public class SecondaryIndexManager implements IndexRegistry
                                                     PartitionColumns partitionColumns,
                                                     int nowInSec)
     {
-        //
         if (!hasIndexes())
             return CleanupTransaction.NO_OP;
 
-        Index[] interestedIndexes = indexes.values().stream()
-                                           .filter(i -> i.indexes(partitionColumns))
-                                           .toArray(Index[]::new);
-
-        return interestedIndexes.length == 0
-               ? CleanupTransaction.NO_OP
-               : new CleanupGCTransaction(key, nowInSec, interestedIndexes);
+        return new CleanupGCTransaction(key, partitionColumns, nowInSec, listIndexes());
     }
 
     /**
@@ -807,7 +791,8 @@ public class SecondaryIndexManager implements IndexRegistry
 
         public void onInserted(Row row)
         {
-            Arrays.stream(indexers).forEach(h -> h.insertRow(row));
+            for (Index.Indexer indexer : indexers)
+                indexer.insertRow(row);
         }
 
         public void onUpdated(Row existing, Row updated)
@@ -882,21 +867,21 @@ public class SecondaryIndexManager implements IndexRegistry
     private static final class IndexGCTransaction implements CompactionTransaction
     {
         private final DecoratedKey key;
+        private final PartitionColumns columns;
         private final int versions;
         private final int nowInSec;
-        private final Index[] indexes;
+        private final Collection<Index> indexes;
 
         private Row[] rows;
 
         private IndexGCTransaction(DecoratedKey key,
+                                   PartitionColumns columns,
                                    int versions,
                                    int nowInSec,
-                                   Index...indexes)
+                                   Collection<Index> indexes)
         {
-            // don't allow null indexers, if we don't have any, use a noop transaction
-            for (Index index : indexes) assert index != null;
-
             this.key = key;
+            this.columns = columns;
             this.versions = versions;
             this.indexes = indexes;
             this.nowInSec = nowInSec;
@@ -957,7 +942,10 @@ public class SecondaryIndexManager implements IndexRegistry
             {
                 for (Index index : indexes)
                 {
-                    Index.Indexer indexer = index.indexerFor(key, nowInSec, opGroup, Type.COMPACTION);
+                    Index.Indexer indexer = index.indexerFor(key, columns, nowInSec, opGroup,
Type.COMPACTION);
+                    if (indexer == null)
+                        continue;
+
                     indexer.begin();
                     for (Row row : rows)
                         if (row != null)
@@ -977,20 +965,20 @@ public class SecondaryIndexManager implements IndexRegistry
     private static final class CleanupGCTransaction implements CleanupTransaction
     {
         private final DecoratedKey key;
+        private final PartitionColumns columns;
         private final int nowInSec;
-        private final Index[] indexes;
+        private final Collection<Index> indexes;
 
         private Row row;
         private DeletionTime partitionDelete;
 
         private CleanupGCTransaction(DecoratedKey key,
+                                     PartitionColumns columns,
                                      int nowInSec,
-                                     Index...indexes)
+                                     Collection<Index> indexes)
         {
-            // don't allow null indexers, if we don't have any, use a noop transaction
-            for (Index index : indexes) assert index != null;
-
             this.key = key;
+            this.columns = columns;
             this.indexes = indexes;
             this.nowInSec = nowInSec;
         }
@@ -1018,7 +1006,10 @@ public class SecondaryIndexManager implements IndexRegistry
             {
                 for (Index index : indexes)
                 {
-                    Index.Indexer indexer = index.indexerFor(key, nowInSec, opGroup, Type.CLEANUP);
+                    Index.Indexer indexer = index.indexerFor(key, columns, nowInSec, opGroup,
Type.CLEANUP);
+                    if (indexer == null)
+                        continue;
+
                     indexer.begin();
 
                     if (partitionDelete != null)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/fffa6d8e/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
index 717126b..6223d8a 100644
--- a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
+++ b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
@@ -217,12 +217,6 @@ public abstract class CassandraIndex implements Index
         return true;
     }
 
-    public boolean indexes(PartitionColumns columns)
-    {
-        // if we have indexes on the partition key or clustering columns, return true
-        return isPrimaryKeyIndex() || columns.contains(indexedColumn);
-    }
-
     public boolean dependsOn(ColumnDefinition column)
     {
         return indexedColumn.name.equals(column.name);
@@ -304,19 +298,34 @@ public abstract class CassandraIndex implements Index
                 validateClusterings(update);
                 break;
             case REGULAR:
-                validateRows(update);
+                if (update.columns().regulars.contains(indexedColumn))
+                    validateRows(update);
                 break;
             case STATIC:
-                validateRows(Collections.singleton(update.staticRow()));
+                if (update.columns().statics.contains(indexedColumn))
+                    validateRows(Collections.singleton(update.staticRow()));
                 break;
         }
     }
 
     public Indexer indexerFor(final DecoratedKey key,
+                              final PartitionColumns columns,
                               final int nowInSec,
                               final OpOrder.Group opGroup,
                               final IndexTransaction.Type transactionType)
     {
+        /**
+         * Indexes on regular and static columns (the non primary-key ones) only care about
updates with live
+         * data for the column they index. In particular, they don't care about having just
row or range deletions
+         * as they don't know how to update the index table unless they know exactly the
value that is deleted.
+         *
+         * Note that in practice this means that those indexes are only purged of stale entries
on compaction,
+         * when we resolve both the deletion and the prior data it deletes. Of course, such
stale entries are also
+         * filtered on read.
+         */
+        if (!isPrimaryKeyIndex() && !columns.contains(indexedColumn))
+            return null;
+
         return new Indexer()
         {
             public void begin()
@@ -359,7 +368,6 @@ public abstract class CassandraIndex implements Index
                     removeCell(row.clustering(), row.getCell(indexedColumn));
             }
 
-
             public void updateRow(Row oldRow, Row newRow)
             {
                 if (isPrimaryKeyIndex())

http://git-wip-us.apache.org/repos/asf/cassandra/blob/fffa6d8e/test/unit/org/apache/cassandra/index/StubIndex.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/index/StubIndex.java b/test/unit/org/apache/cassandra/index/StubIndex.java
index 834ff87..cd0541f 100644
--- a/test/unit/org/apache/cassandra/index/StubIndex.java
+++ b/test/unit/org/apache/cassandra/index/StubIndex.java
@@ -69,11 +69,6 @@ public class StubIndex implements Index
         this.indexMetadata = metadata;
     }
 
-    public boolean indexes(PartitionColumns columns)
-    {
-        return true;
-    }
-
     public boolean shouldBuildBlocking()
     {
         return false;
@@ -100,6 +95,7 @@ public class StubIndex implements Index
     }
 
     public Indexer indexerFor(final DecoratedKey key,
+                              PartitionColumns columns,
                               int nowInSec,
                               OpOrder.Group opGroup,
                               IndexTransaction.Type transactionType)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/fffa6d8e/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java b/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java
index 3bce683..a30cf4e 100644
--- a/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java
+++ b/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java
@@ -162,12 +162,6 @@ public class CustomCassandraIndex implements Index
         return true;
     }
 
-    public boolean indexes(PartitionColumns columns)
-    {
-        // if we have indexes on the partition key or clustering columns, return true
-        return isPrimaryKeyIndex() || columns.contains(indexedColumn);
-    }
-
     public boolean dependsOn(ColumnDefinition column)
     {
         return column.equals(indexedColumn);
@@ -271,10 +265,14 @@ public class CustomCassandraIndex implements Index
     }
 
     public Indexer indexerFor(final DecoratedKey key,
+                              final PartitionColumns columns,
                               final int nowInSec,
                               final OpOrder.Group opGroup,
                               final IndexTransaction.Type transactionType)
     {
+        if (!isPrimaryKeyIndex() && !columns.contains(indexedColumn))
+            return null;
+
         return new Indexer()
         {
             public void begin()


Mime
View raw message