From 5bc3c4b6d5fe10f08395bfb2f860d90b0a0e2ed8 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Mon, 26 Oct 2015 18:37:18 +0000 Subject: [PATCH] Secondary indexes which aren't registered are not initialized This commit also removes Index::getIndexName in favour of accessing IndexMetadata.name directly Patch by Sam Tunnicliffe; reviewed by Sergio Bossa for CASSANDRA-10595 --- CHANGES.txt | 1 + .../org/apache/cassandra/db/ReadCommand.java | 2 +- .../org/apache/cassandra/index/Index.java | 6 ----- .../index/SecondaryIndexManager.java | 23 +++++++++++-------- .../index/internal/CassandraIndex.java | 19 ++++++--------- .../cassandra/db/RangeTombstoneTest.java | 2 +- .../org/apache/cassandra/index/StubIndex.java | 5 ---- .../index/internal/CustomCassandraIndex.java | 20 ++++++---------- 8 files changed, 30 insertions(+), 48 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 9b81b6e070..dd7949e540 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0 + * Skip initialization of non-registered 2i instances, remove Index::getIndexName (CASSANDRA-10595) * Fix batches on multiple tables (CASSANDRA-10554) * Ensure compaction options are validated when updating KeyspaceMetadata (CASSANDRA-10569) * Flatten Iterator Transformation Hierarchy (CASSANDRA-9975) diff --git a/src/java/org/apache/cassandra/db/ReadCommand.java b/src/java/org/apache/cassandra/db/ReadCommand.java index 4d9b65bf0b..2eb1d1da71 100644 --- a/src/java/org/apache/cassandra/db/ReadCommand.java +++ b/src/java/org/apache/cassandra/db/ReadCommand.java @@ -337,7 +337,7 @@ public abstract class ReadCommand implements ReadQuery Index.Searcher searcher = index == null ? null : index.searcherFor(this); if (index != null) - Tracing.trace("Executing read on {}.{} using index {}", cfs.metadata.ksName, cfs.metadata.cfName, index.getIndexName()); + Tracing.trace("Executing read on {}.{} using index {}", cfs.metadata.ksName, cfs.metadata.cfName, index.getIndexMetadata().name); UnfilteredPartitionIterator resultIterator = searcher == null ? queryStorage(cfs, orderGroup) diff --git a/src/java/org/apache/cassandra/index/Index.java b/src/java/org/apache/cassandra/index/Index.java index 3ceec13f24..0f4ecbd8ca 100644 --- a/src/java/org/apache/cassandra/index/Index.java +++ b/src/java/org/apache/cassandra/index/Index.java @@ -143,12 +143,6 @@ public interface Index */ public void register(IndexRegistry registry); - /** - * Return an identifier for the index. This should be unique across all indexes on a given base table - * @return the name of the index - */ - public String getIndexName(); - /** * If the index implementation uses a local table to store its index data this method should return a * handle to it. If not, an empty Optional should be returned. Typically, this is useful for the built-in diff --git a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java index 87b47d90c5..7d866df962 100644 --- a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java +++ b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java @@ -159,7 +159,10 @@ public class SecondaryIndexManager implements IndexRegistry { Index index = createInstance(indexDef); index.register(this); - final Callable initialBuildTask = index.getInitializationTask(); + // if the index didn't register itself, we can probably assume that no initialization needs to happen + final Callable initialBuildTask = indexes.containsKey(indexDef.name) + ? index.getInitializationTask() + : null; return initialBuildTask == null ? Futures.immediateFuture(null) : asyncExecutor.submit(initialBuildTask); @@ -223,7 +226,7 @@ public class SecondaryIndexManager implements IndexRegistry public void rebuildIndexesBlocking(Collection sstables, Set indexNames) { Set toRebuild = indexes.values().stream() - .filter(index -> indexNames.contains(index.getIndexName())) + .filter(index -> indexNames.contains(index.getIndexMetadata().name)) .filter(Index::shouldBuildBlocking) .collect(Collectors.toSet()); if (toRebuild.isEmpty()) @@ -232,11 +235,11 @@ public class SecondaryIndexManager implements IndexRegistry return; } - toRebuild.forEach(indexer -> markIndexRemoved(indexer.getIndexName())); + toRebuild.forEach(indexer -> markIndexRemoved(indexer.getIndexMetadata().name)); buildIndexesBlocking(sstables, toRebuild); - toRebuild.forEach(indexer -> markIndexBuilt(indexer.getIndexName())); + toRebuild.forEach(indexer -> markIndexBuilt(indexer.getIndexMetadata().name)); } public void buildAllIndexesBlocking(Collection sstables) @@ -256,7 +259,7 @@ public class SecondaryIndexManager implements IndexRegistry Refs sstables = viewFragment.refs) { buildIndexesBlocking(sstables, Collections.singleton(index)); - markIndexBuilt(index.getIndexName()); + markIndexBuilt(index.getIndexMetadata().name); } } } @@ -338,7 +341,7 @@ public class SecondaryIndexManager implements IndexRegistry return; logger.info("Submitting index build of {} for data in {}", - indexes.stream().map(Index::getIndexName).collect(Collectors.joining(",")), + indexes.stream().map(i -> i.getIndexMetadata().name).collect(Collectors.joining(",")), sstables.stream().map(SSTableReader::toString).collect(Collectors.joining(","))); SecondaryIndexBuilder builder = new SecondaryIndexBuilder(baseCfs, @@ -349,7 +352,7 @@ public class SecondaryIndexManager implements IndexRegistry flushIndexesBlocking(indexes); logger.info("Index build of {} complete", - indexes.stream().map(Index::getIndexName).collect(Collectors.joining(","))); + indexes.stream().map(i -> i.getIndexMetadata().name).collect(Collectors.joining(","))); } private void markIndexBuilt(String indexName) @@ -461,7 +464,7 @@ public class SecondaryIndexManager implements IndexRegistry { Set allIndexNames = new HashSet<>(); indexes.values().stream() - .map(Index::getIndexName) + .map(i -> i.getIndexMetadata().name) .forEach(allIndexNames::add); return SystemKeyspace.getBuiltIndexes(baseCfs.keyspace.getName(), allIndexNames); } @@ -618,9 +621,9 @@ public class SecondaryIndexManager implements IndexRegistry if (Tracing.isTracing()) { Tracing.trace("Index mean cardinalities are {}. Scanning with {}.", - searchableIndexes.stream().map(i -> i.getIndexName() + ':' + i.getEstimatedResultRows()) + searchableIndexes.stream().map(i -> i.getIndexMetadata().name + ':' + i.getEstimatedResultRows()) .collect(Collectors.joining(",")), - selected.getIndexName()); + selected.getIndexMetadata().name); } return selected; } diff --git a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java index 93f5d61c77..7b9d18f1cd 100644 --- a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java +++ b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java @@ -160,11 +160,6 @@ public abstract class CassandraIndex implements Index return metadata; } - public String getIndexName() - { - return metadata.name; - } - public Optional getBackingTable() { return indexCfs == null ? Optional.empty() : Optional.of(indexCfs); @@ -583,7 +578,7 @@ public abstract class CassandraIndex implements Index throw new InvalidRequestException(String.format( "Cannot index value of size %d for index %s on %s.%s(%s) (maximum allowed size=%d)", value.remaining(), - getIndexName(), + metadata.name, baseCfs.metadata.ksName, baseCfs.metadata.cfName, indexedColumn.name.toString(), @@ -634,17 +629,17 @@ public abstract class CassandraIndex implements Index private boolean isBuilt() { - return SystemKeyspace.isIndexBuilt(baseCfs.keyspace.getName(), getIndexName()); + return SystemKeyspace.isIndexBuilt(baseCfs.keyspace.getName(), metadata.name); } private void markBuilt() { - SystemKeyspace.setIndexBuilt(baseCfs.keyspace.getName(), getIndexName()); + SystemKeyspace.setIndexBuilt(baseCfs.keyspace.getName(), metadata.name); } private void markRemoved() { - SystemKeyspace.setIndexRemoved(baseCfs.keyspace.getName(), getIndexName()); + SystemKeyspace.setIndexRemoved(baseCfs.keyspace.getName(), metadata.name); } private boolean isPrimaryKeyIndex() @@ -672,13 +667,13 @@ public abstract class CassandraIndex implements Index logger.info("No SSTable data for {}.{} to build index {} from, marking empty index as built", baseCfs.metadata.ksName, baseCfs.metadata.cfName, - getIndexName()); + metadata.name); markBuilt(); return; } logger.info("Submitting index build of {} for data in {}", - getIndexName(), + metadata.name, getSSTableNames(sstables)); SecondaryIndexBuilder builder = new SecondaryIndexBuilder(baseCfs, @@ -689,7 +684,7 @@ public abstract class CassandraIndex implements Index indexCfs.forceBlockingFlush(); markBuilt(); } - logger.info("Index build of {} complete", getIndexName()); + logger.info("Index build of {} complete", metadata.name); } private static String getSSTableNames(Collection sstables) diff --git a/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java b/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java index 95f38c12a5..a6ee48fd2f 100644 --- a/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java +++ b/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java @@ -480,7 +480,7 @@ public class RangeTombstoneTest StubIndex index = (StubIndex)cfs.indexManager.listIndexes() .stream() - .filter(i -> "test_index".equals(i.getIndexName())) + .filter(i -> "test_index".equals(i.getIndexMetadata().name)) .findFirst() .orElseThrow(() -> new RuntimeException(new AssertionError("Index not found"))); index.reset(); diff --git a/test/unit/org/apache/cassandra/index/StubIndex.java b/test/unit/org/apache/cassandra/index/StubIndex.java index 05c860a150..834ff873b4 100644 --- a/test/unit/org/apache/cassandra/index/StubIndex.java +++ b/test/unit/org/apache/cassandra/index/StubIndex.java @@ -151,11 +151,6 @@ public class StubIndex implements Index return indexMetadata; } - public String getIndexName() - { - return indexMetadata != null ? indexMetadata.name : "default_test_index_name"; - } - public void register(IndexRegistry registry){ registry.registerIndex(this); } diff --git a/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java b/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java index cbcf069f44..2dc95352c9 100644 --- a/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java +++ b/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java @@ -105,12 +105,6 @@ public class CustomCassandraIndex implements Index return metadata; } - public String getIndexName() - { - // should return metadata.name, see CASSANDRA-10127 - return indexCfs.name; - } - public Optional getBackingTable() { return indexCfs == null ? Optional.empty() : Optional.of(indexCfs); @@ -564,7 +558,7 @@ public class CustomCassandraIndex implements Index throw new InvalidRequestException(String.format( "Cannot index value of size %d for index %s on %s.%s(%s) (maximum allowed size=%d)", value.remaining(), - getIndexName(), + metadata.name, baseCfs.metadata.ksName, baseCfs.metadata.cfName, indexedColumn.name.toString(), @@ -615,17 +609,17 @@ public class CustomCassandraIndex implements Index private boolean isBuilt() { - return SystemKeyspace.isIndexBuilt(baseCfs.keyspace.getName(), getIndexName()); + return SystemKeyspace.isIndexBuilt(baseCfs.keyspace.getName(), metadata.name); } private void markBuilt() { - SystemKeyspace.setIndexBuilt(baseCfs.keyspace.getName(), getIndexName()); + SystemKeyspace.setIndexBuilt(baseCfs.keyspace.getName(), metadata.name); } private void markRemoved() { - SystemKeyspace.setIndexRemoved(baseCfs.keyspace.getName(), getIndexName()); + SystemKeyspace.setIndexRemoved(baseCfs.keyspace.getName(), metadata.name); } private boolean isPrimaryKeyIndex() @@ -653,13 +647,13 @@ public class CustomCassandraIndex implements Index logger.info("No SSTable data for {}.{} to build index {} from, marking empty index as built", baseCfs.metadata.ksName, baseCfs.metadata.cfName, - getIndexName()); + metadata.name); markBuilt(); return; } logger.info("Submitting index build of {} for data in {}", - getIndexName(), + metadata.name, getSSTableNames(sstables)); SecondaryIndexBuilder builder = new SecondaryIndexBuilder(baseCfs, @@ -670,7 +664,7 @@ public class CustomCassandraIndex implements Index indexCfs.forceBlockingFlush(); markBuilt(); } - logger.info("Index build of {} complete", getIndexName()); + logger.info("Index build of {} complete", metadata.name); } private static String getSSTableNames(Collection sstables)