Repository: cassandra Updated Branches: refs/heads/trunk 9068a31f9 -> 1c310a698
Make naming for secondary indexes consistent patch by Benjamin Lerer; reviewed by Jake Luciani for CASSANDRA-10127 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/cd5d03d2 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/cd5d03d2 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/cd5d03d2 Branch: refs/heads/trunk Commit: cd5d03d2b78c634109e7e39b1986965f12630699 Parents: 8ebd590 Author: blerer <benjamin.le...@datastax.com> Authored: Thu Sep 3 21:34:45 2015 +0200 Committer: blerer <benjamin.le...@datastax.com> Committed: Thu Sep 3 21:34:45 2015 +0200 ---------------------------------------------------------------------- NEWS.txt | 2 + .../apache/cassandra/db/ColumnFamilyStore.java | 25 ++----- src/java/org/apache/cassandra/db/Keyspace.java | 17 +++-- .../cassandra/index/SecondaryIndexManager.java | 79 +++++++++++++++++++- .../index/internal/CassandraIndex.java | 3 +- .../cassandra/service/StorageService.java | 16 +++- .../cassandra/tools/nodetool/RebuildIndex.java | 2 +- .../org/apache/cassandra/cql3/CQLTester.java | 3 +- .../apache/cassandra/db/SecondaryIndexTest.java | 5 +- .../index/internal/CassandraIndexTest.java | 5 +- 10 files changed, 119 insertions(+), 38 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/NEWS.txt ---------------------------------------------------------------------- diff --git a/NEWS.txt b/NEWS.txt index 33d67da..c7976b9 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -18,6 +18,8 @@ using the provided 'sstableupgrade' tool. New features ------------ + - nodetool rebuild_index accepts the index argument without + the redundant table name - Materialized Views, which allow for server-side denormalization, is now available. Materialized views provide an alternative to secondary indexes for non-primary key queries, and perform much better for indexing high http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/src/java/org/apache/cassandra/db/ColumnFamilyStore.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index efac287..096172d 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -775,7 +775,7 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean Iterable<SSTableReader> sstables = cfs.getSSTables(SSTableSet.CANONICAL); try (Refs<SSTableReader> refs = Refs.ref(sstables)) { - logger.info(String.format("User Requested secondary index re-build for %s/%s indexes", ksName, cfName)); + logger.info("User Requested secondary index re-build for {}/{} indexes: {}", ksName, cfName, Joiner.on(',').join(idxNames)); cfs.indexManager.rebuildIndexesBlocking(refs, indexes); } } @@ -1382,27 +1382,18 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean */ public boolean rebuildOnFailedScrub(Throwable failure) { - if (!isIndex()) - return false; - - boolean validIndex = false; - ColumnFamilyStore parentCfs = null; - String indexName = null; - if (metadata.cfName.contains(Directories.SECONDARY_INDEX_NAME_SEPARATOR)) - { - String[] parts = metadata.cfName.split("\\" + Directories.SECONDARY_INDEX_NAME_SEPARATOR, 2); - parentCfs = keyspace.getColumnFamilyStore(parts[0]); - assert parentCfs.indexManager.getAllIndexColumnFamilyStores().contains(this); - validIndex = true; - indexName = this.name; - } - - if (! validIndex) + if (!isIndex() || !SecondaryIndexManager.isIndexColumnFamilyStore(this)) return false; truncateBlocking(); logger.warn("Rebuilding index for {} because of <{}>", name, failure.getMessage()); + + ColumnFamilyStore parentCfs = SecondaryIndexManager.getParentCfs(this); + assert parentCfs.indexManager.getAllIndexColumnFamilyStores().contains(this); + + String indexName = SecondaryIndexManager.getIndexName(this); + parentCfs.rebuildSecondaryIndex(indexName); return true; } http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/src/java/org/apache/cassandra/db/Keyspace.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/Keyspace.java b/src/java/org/apache/cassandra/db/Keyspace.java index 981209c..4661bae 100644 --- a/src/java/org/apache/cassandra/db/Keyspace.java +++ b/src/java/org/apache/cassandra/db/Keyspace.java @@ -27,12 +27,13 @@ import java.util.concurrent.locks.Lock; import com.google.common.base.Function; import com.google.common.collect.Iterables; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.apache.cassandra.concurrent.Stage; import org.apache.cassandra.concurrent.StageManager; -import org.apache.cassandra.config.*; +import org.apache.cassandra.config.CFMetaData; +import org.apache.cassandra.config.Config; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.config.Schema; import org.apache.cassandra.db.commitlog.CommitLog; import org.apache.cassandra.db.commitlog.ReplayPosition; import org.apache.cassandra.db.compaction.CompactionManager; @@ -42,6 +43,7 @@ import org.apache.cassandra.db.rows.UnfilteredRowIterator; import org.apache.cassandra.db.view.MaterializedViewManager; import org.apache.cassandra.exceptions.WriteTimeoutException; import org.apache.cassandra.index.Index; +import org.apache.cassandra.index.SecondaryIndexManager; import org.apache.cassandra.index.transactions.UpdateTransaction; import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.locator.AbstractReplicationStrategy; @@ -53,6 +55,8 @@ import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.JVMStabilityInspector; import org.apache.cassandra.utils.concurrent.OpOrder; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * It represents a Keyspace. @@ -548,16 +552,15 @@ public class Keyspace // include the specified stores and possibly the stores of any of their indexes for (String cfName : cfNames) { - int separatorPos = cfName.indexOf(Directories.SECONDARY_INDEX_NAME_SEPARATOR); - if (separatorPos > -1) + if (SecondaryIndexManager.isIndexColumnFamily(cfName)) { if (!allowIndexes) { logger.warn("Operation not allowed on secondary Index table ({})", cfName); continue; } - String baseName = cfName.substring(0, separatorPos); - String indexName = cfName.substring(separatorPos + Directories.SECONDARY_INDEX_NAME_SEPARATOR.length()); + String baseName = SecondaryIndexManager.getParentCfsName(cfName); + String indexName = SecondaryIndexManager.getIndexName(cfName); ColumnFamilyStore baseCfs = getColumnFamilyStore(baseName); Index index = baseCfs.indexManager.getIndexByName(indexName); http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/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 c776cb7..6f30305 100644 --- a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java +++ b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java @@ -24,15 +24,19 @@ import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; +import com.google.common.base.Joiner; import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.common.primitives.Longs; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.MoreExecutors; + +import org.apache.commons.lang3.StringUtils; + +import org.apache.cassandra.db.Directories; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import org.apache.cassandra.concurrent.JMXEnabledThreadPoolExecutor; import org.apache.cassandra.concurrent.NamedThreadFactory; import org.apache.cassandra.concurrent.StageManager; @@ -209,7 +213,7 @@ public class SecondaryIndexManager implements IndexRegistry .collect(Collectors.toSet()); if (toRebuild.isEmpty()) { - logger.info("No defined indexes with the supplied names"); + logger.info("No defined indexes with the supplied names: {}", Joiner.on(',').join(indexNames)); return; } @@ -236,6 +240,77 @@ public class SecondaryIndexManager implements IndexRegistry } } + /** + * Checks if the specified {@link ColumnFamilyStore} is a secondary index. + * + * @param cfs the <code>ColumnFamilyStore</code> to check. + * @return <code>true</code> if the specified <code>ColumnFamilyStore</code> is a secondary index, + * <code>false</code> otherwise. + */ + public static boolean isIndexColumnFamilyStore(ColumnFamilyStore cfs) + { + return isIndexColumnFamily(cfs.name); + } + + /** + * Checks if the specified {@link ColumnFamilyStore} is the one secondary index. + * + * @param cfs the name of the <code>ColumnFamilyStore</code> to check. + * @return <code>true</code> if the specified <code>ColumnFamilyStore</code> is a secondary index, + * <code>false</code> otherwise. + */ + public static boolean isIndexColumnFamily(String cfName) + { + return cfName.contains(Directories.SECONDARY_INDEX_NAME_SEPARATOR); + } + + /** + * Returns the parent of the specified {@link ColumnFamilyStore}. + * + * @param cfs the <code>ColumnFamilyStore</code> + * @return the parent of the specified <code>ColumnFamilyStore</code> + */ + public static ColumnFamilyStore getParentCfs(ColumnFamilyStore cfs) + { + String parentCfs = getParentCfsName(cfs.name); + return cfs.keyspace.getColumnFamilyStore(parentCfs); + } + + /** + * Returns the parent name of the specified {@link ColumnFamilyStore}. + * + * @param cfName the <code>ColumnFamilyStore</code> name + * @return the parent name of the specified <code>ColumnFamilyStore</code> + */ + public static String getParentCfsName(String cfName) + { + assert isIndexColumnFamily(cfName); + return StringUtils.substringBefore(cfName, Directories.SECONDARY_INDEX_NAME_SEPARATOR); + } + + /** + * Returns the index name + * + * @param cfName the <code>ColumnFamilyStore</code> name + * @return the index name + */ + public static String getIndexName(ColumnFamilyStore cfs) + { + return getIndexName(cfs.name); + } + + /** + * Returns the index name + * + * @param cfName the <code>ColumnFamilyStore</code> name + * @return the index name + */ + public static String getIndexName(String cfName) + { + assert isIndexColumnFamily(cfName); + return StringUtils.substringAfter(cfName, Directories.SECONDARY_INDEX_NAME_SEPARATOR); + } + private void buildIndexesBlocking(Collection<SSTableReader> sstables, Set<Index> indexes) { if (indexes.isEmpty()) http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/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 b17ab4e..fd19e7a 100644 --- a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java +++ b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java @@ -159,8 +159,7 @@ public abstract class CassandraIndex implements Index public String getIndexName() { - // should return metadata.name, see CASSANDRA-10127 - return indexCfs.name; + return metadata.name; } public Optional<ColumnFamilyStore> getBackingTable() http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/src/java/org/apache/cassandra/service/StorageService.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 13dc29c7..baab821 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -127,10 +127,11 @@ import org.apache.cassandra.utils.WrappedRunnable; import org.apache.cassandra.utils.progress.ProgressEvent; import org.apache.cassandra.utils.progress.ProgressEventType; import org.apache.cassandra.utils.progress.jmx.JMXProgressSupport; + import org.apache.commons.lang3.StringUtils; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.jmx.JMXConfiguratorMBean; import ch.qos.logback.classic.spi.ILoggingEvent; @@ -152,6 +153,12 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.Uninterruptibles; +import static java.util.stream.Collectors.toList; + +import static org.apache.cassandra.index.SecondaryIndexManager.getIndexName; +import static org.apache.cassandra.index.SecondaryIndexManager.isIndexColumnFamily; +import static java.util.Arrays.asList; + /** * This abstraction contains the token/identifier of this node * on the identifier space. This token gets gossiped around. @@ -4285,7 +4292,12 @@ public class StorageService extends NotificationBroadcasterSupport implements IE public void rebuildSecondaryIndex(String ksName, String cfName, String... idxNames) { - ColumnFamilyStore.rebuildSecondaryIndex(ksName, cfName, idxNames); + String[] indices = asList(idxNames).stream() + .map(p -> isIndexColumnFamily(p) ? getIndexName(p) : p) + .collect(toList()) + .toArray(new String[idxNames.length]); + + ColumnFamilyStore.rebuildSecondaryIndex(ksName, cfName, indices); } public void resetLocalSchema() throws IOException http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/src/java/org/apache/cassandra/tools/nodetool/RebuildIndex.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/tools/nodetool/RebuildIndex.java b/src/java/org/apache/cassandra/tools/nodetool/RebuildIndex.java index 9985b2b..5fd7327 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/RebuildIndex.java +++ b/src/java/org/apache/cassandra/tools/nodetool/RebuildIndex.java @@ -31,7 +31,7 @@ import org.apache.cassandra.tools.NodeTool.NodeToolCmd; @Command(name = "rebuild_index", description = "A full rebuild of native secondary indexes for a given table") public class RebuildIndex extends NodeToolCmd { - @Arguments(usage = "<keyspace> <table> <indexName...>", description = "The keyspace and table name followed by a list of index names (IndexNameExample: Standard3.IdxName Standard3.IdxName1)") + @Arguments(usage = "<keyspace> <table> <indexName...>", description = "The keyspace and table name followed by a list of index names") List<String> args = new ArrayList<>(); @Override http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/test/unit/org/apache/cassandra/cql3/CQLTester.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java index e540ec5..349975d 100644 --- a/test/unit/org/apache/cassandra/cql3/CQLTester.java +++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java @@ -547,13 +547,12 @@ public abstract class CQLTester { long start = System.currentTimeMillis(); boolean indexCreated = false; - String indedName = String.format("%s.%s", table, index); while (!indexCreated) { Object[][] results = getRows(execute("select index_name from system.\"IndexInfo\" where table_name = ?", keyspace)); for(int i = 0; i < results.length; i++) { - if (indedName.equals(results[i][0])) + if (index.equals(results[i][0])) { indexCreated = true; break; http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/test/unit/org/apache/cassandra/db/SecondaryIndexTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/SecondaryIndexTest.java b/test/unit/org/apache/cassandra/db/SecondaryIndexTest.java index c017675..0003f8f 100644 --- a/test/unit/org/apache/cassandra/db/SecondaryIndexTest.java +++ b/test/unit/org/apache/cassandra/db/SecondaryIndexTest.java @@ -29,6 +29,8 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.apache.cassandra.index.SecondaryIndexManager; + import org.apache.cassandra.SchemaLoader; import org.apache.cassandra.Util; import org.apache.cassandra.config.ColumnDefinition; @@ -42,7 +44,6 @@ import org.apache.cassandra.schema.IndexMetadata; import org.apache.cassandra.schema.KeyspaceParams; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.FBUtilities; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -451,7 +452,7 @@ public class SecondaryIndexTest boolean flushed = false; for (ColumnFamilyStore indexCfs : cfs.indexManager.getAllIndexColumnFamilyStores()) { - if (indexCfs.name.equals(indexName)) + if (SecondaryIndexManager.getIndexName(indexCfs).equals(indexName)) flushed = indexCfs.getLiveSSTables().size() > 0; } assertTrue(flushed); http://git-wip-us.apache.org/repos/asf/cassandra/blob/cd5d03d2/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java b/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java index e938737..73ce6c0 100644 --- a/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java +++ b/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java @@ -703,15 +703,14 @@ public class CassandraIndexTest extends CQLTester private void waitForIndexBuild() throws Throwable { ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); - String fullIndexName = String.format("%s.%s", currentTable(), indexName); long maxWaitMillis = 10000; long startTime = System.currentTimeMillis(); - while (! cfs.indexManager.getBuiltIndexNames().contains(fullIndexName)) + while (! cfs.indexManager.getBuiltIndexNames().contains(indexName)) { Thread.sleep(100); long wait = System.currentTimeMillis() - startTime; if (wait > maxWaitMillis) - fail(String.format("Timed out waiting for index %s to build (%s)ms", fullIndexName, wait)); + fail(String.format("Timed out waiting for index %s to build (%s)ms", indexName, wait)); } } }