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 <[email protected]> Authored: Tue Nov 17 17:48:00 2015 +0100 Committer: Sylvain Lebresne <[email protected]> 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()
