Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 502AC1899E for ; Wed, 28 Oct 2015 09:45:30 +0000 (UTC) Received: (qmail 52309 invoked by uid 500); 28 Oct 2015 09:45:03 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 52176 invoked by uid 500); 28 Oct 2015 09:45:03 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 52152 invoked by uid 99); 28 Oct 2015 09:45:03 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 28 Oct 2015 09:45:03 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id DC178E04BE; Wed, 28 Oct 2015 09:45:02 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: samt@apache.org To: commits@cassandra.apache.org Date: Wed, 28 Oct 2015 09:45:03 -0000 Message-Id: <3655ca31f2ac4ef7aead4a9c3bb1d9a1@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [2/3] cassandra git commit: Secondary indexes which aren't registered are not initialized 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 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/5bc3c4b6 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/5bc3c4b6 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/5bc3c4b6 Branch: refs/heads/trunk Commit: 5bc3c4b6d5fe10f08395bfb2f860d90b0a0e2ed8 Parents: 27ca491 Author: Sam Tunnicliffe Authored: Mon Oct 26 18:37:18 2015 +0000 Committer: Sam Tunnicliffe Committed: Wed Oct 28 09:38:43 2015 +0000 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/db/ReadCommand.java | 2 +- src/java/org/apache/cassandra/index/Index.java | 6 ----- .../cassandra/index/SecondaryIndexManager.java | 23 +++++++++++--------- .../index/internal/CassandraIndex.java | 19 ++++++---------- .../apache/cassandra/db/RangeTombstoneTest.java | 2 +- .../org/apache/cassandra/index/StubIndex.java | 5 ----- .../index/internal/CustomCassandraIndex.java | 20 ++++++----------- 8 files changed, 30 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/5bc3c4b6/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 9b81b6e..dd7949e 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) http://git-wip-us.apache.org/repos/asf/cassandra/blob/5bc3c4b6/src/java/org/apache/cassandra/db/ReadCommand.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/ReadCommand.java b/src/java/org/apache/cassandra/db/ReadCommand.java index 4d9b65b..2eb1d1d 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) http://git-wip-us.apache.org/repos/asf/cassandra/blob/5bc3c4b6/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 3ceec13..0f4ecbd 100644 --- a/src/java/org/apache/cassandra/index/Index.java +++ b/src/java/org/apache/cassandra/index/Index.java @@ -144,12 +144,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 * Index implementations. http://git-wip-us.apache.org/repos/asf/cassandra/blob/5bc3c4b6/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 87b47d9..7d866df 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; } http://git-wip-us.apache.org/repos/asf/cassandra/blob/5bc3c4b6/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 93f5d61..7b9d18f 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) http://git-wip-us.apache.org/repos/asf/cassandra/blob/5bc3c4b6/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java b/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java index 95f38c1..a6ee48f 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(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/5bc3c4b6/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 05c860a..834ff87 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); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/5bc3c4b6/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 cbcf069..2dc9535 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)