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 A022718CE2 for ; Wed, 16 Sep 2015 16:33:04 +0000 (UTC) Received: (qmail 57909 invoked by uid 500); 16 Sep 2015 16:32:48 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 57882 invoked by uid 500); 16 Sep 2015 16:32:48 -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 57870 invoked by uid 99); 16 Sep 2015 16:32:48 -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, 16 Sep 2015 16:32:48 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 60E6BDFFC2; Wed, 16 Sep 2015 16:32:48 +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, 16 Sep 2015 16:32:47 -0000 Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: [1/5] cassandra git commit: Remove target_columns from index metadata Repository: cassandra Updated Branches: refs/heads/cassandra-3.0 1d94dc2b5 -> fde97c3b3 refs/heads/trunk 5ba0adf50 -> f5cf57167 http://git-wip-us.apache.org/repos/asf/cassandra/blob/fde97c3b/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 0003f8f..2a43e33 100644 --- a/test/unit/org/apache/cassandra/db/SecondaryIndexTest.java +++ b/test/unit/org/apache/cassandra/db/SecondaryIndexTest.java @@ -29,12 +29,11 @@ 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; import org.apache.cassandra.cql3.Operator; +import org.apache.cassandra.cql3.statements.IndexTarget; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.partitions.*; import org.apache.cassandra.db.rows.Row; @@ -44,6 +43,8 @@ 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.apache.cassandra.Util.throwAssert; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -63,8 +64,8 @@ public class SecondaryIndexTest SchemaLoader.prepareServer(); SchemaLoader.createKeyspace(KEYSPACE1, KeyspaceParams.simple(1), - SchemaLoader.compositeIndexCFMD(KEYSPACE1, WITH_COMPOSITE_INDEX, true) .gcGraceSeconds(0), - SchemaLoader.compositeIndexCFMD(KEYSPACE1, COMPOSITE_INDEX_TO_BE_ADDED, false) .gcGraceSeconds(0), + SchemaLoader.compositeIndexCFMD(KEYSPACE1, WITH_COMPOSITE_INDEX, true).gcGraceSeconds(0), + SchemaLoader.compositeIndexCFMD(KEYSPACE1, COMPOSITE_INDEX_TO_BE_ADDED, false).gcGraceSeconds(0), SchemaLoader.keysIndexCFMD(KEYSPACE1, WITH_KEYS_INDEX, true).gcGraceSeconds(0)); } @@ -435,10 +436,12 @@ public class SecondaryIndexTest // create a row and update the birthdate value, test that the index query fetches the new version new RowUpdateBuilder(cfs.metadata, 0, "k1").clustering("c").add("birthdate", 1L).build().applyUnsafe(); + String indexName = "birthdate_index"; ColumnDefinition old = cfs.metadata.getColumnDefinition(ByteBufferUtil.bytes("birthdate")); - IndexMetadata indexDef = IndexMetadata.singleColumnIndex(old, - "birthdate_index", - IndexMetadata.IndexType.COMPOSITES, + IndexMetadata indexDef = IndexMetadata.singleTargetIndex(cfs.metadata, + new IndexTarget(old.name, IndexTarget.Type.VALUES), + indexName, + IndexMetadata.Kind.COMPOSITES, Collections.EMPTY_MAP); cfs.metadata.indexes(cfs.metadata.getIndexes().with(indexDef)); Future future = cfs.indexManager.addIndex(indexDef); @@ -446,15 +449,11 @@ public class SecondaryIndexTest // we had a bug (CASSANDRA-2244) where index would get created but not flushed -- check for that // the way we find the index cfs is a bit convoluted at the moment - ColumnDefinition cDef = cfs.metadata.getColumnDefinition(ByteBufferUtil.bytes("birthdate")); - String indexName = getIndexNameForColumn(cfs, cDef); - assertNotNull(indexName); boolean flushed = false; - for (ColumnFamilyStore indexCfs : cfs.indexManager.getAllIndexColumnFamilyStores()) - { - if (SecondaryIndexManager.getIndexName(indexCfs).equals(indexName)) - flushed = indexCfs.getLiveSSTables().size() > 0; - } + ColumnFamilyStore indexCfs = cfs.indexManager.getIndex(indexDef) + .getBackingTable() + .orElseThrow(throwAssert("Index not found")); + flushed = !indexCfs.getLiveSSTables().isEmpty(); assertTrue(flushed); assertIndexedOne(cfs, ByteBufferUtil.bytes("birthdate"), 1L); @@ -469,18 +468,6 @@ public class SecondaryIndexTest assertIndexedOne(cfs, ByteBufferUtil.bytes("birthdate"), 1L); } - private String getIndexNameForColumn(ColumnFamilyStore cfs, ColumnDefinition column) - { - // this is mega-ugly because there is a mismatch between the name of an index - // stored in schema metadata & the name used to refer to that index in other - // places (such as system.IndexInfo). Hopefully this is temporary and - // Index.getIndexName() can be made equalivalent to Index.getIndexMetadata().name - // (ideally even removing the former completely) - Collection indexes = cfs.metadata.getIndexes().get(column); - assertEquals(1, indexes.size()); - return cfs.indexManager.getIndex(indexes.iterator().next()).getIndexName(); - } - @Test public void testKeysSearcherSimple() throws Exception { http://git-wip-us.apache.org/repos/asf/cassandra/blob/fde97c3b/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 0ea03dc..544d482 100644 --- a/test/unit/org/apache/cassandra/index/StubIndex.java +++ b/test/unit/org/apache/cassandra/index/StubIndex.java @@ -23,7 +23,6 @@ import java.util.concurrent.Callable; import java.util.function.BiFunction; import org.apache.cassandra.config.ColumnDefinition; -import org.apache.cassandra.cql3.ColumnIdentifier; import org.apache.cassandra.cql3.Operator; import org.apache.cassandra.db.*; import org.apache.cassandra.db.filter.RowFilter; @@ -70,6 +69,11 @@ public class StubIndex implements Index return false; } + public boolean dependsOn(ColumnDefinition column) + { + return false; + } + public boolean supportsExpression(ColumnDefinition column, Operator operator) { return operator == Operator.EQ; http://git-wip-us.apache.org/repos/asf/cassandra/blob/fde97c3b/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 0a62d9b..8695018 100644 --- a/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java +++ b/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java @@ -20,27 +20,27 @@ import org.apache.cassandra.db.compaction.CompactionManager; import org.apache.cassandra.db.filter.RowFilter; import org.apache.cassandra.db.lifecycle.SSTableSet; import org.apache.cassandra.db.lifecycle.View; -import org.apache.cassandra.db.marshal.AbstractType; -import org.apache.cassandra.db.marshal.CollectionType; import org.apache.cassandra.db.partitions.PartitionIterator; import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.rows.*; -import org.apache.cassandra.dht.LocalPartitioner; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.index.Index; import org.apache.cassandra.index.IndexRegistry; import org.apache.cassandra.index.SecondaryIndexBuilder; -import org.apache.cassandra.index.internal.composites.CompositesSearcher; -import org.apache.cassandra.index.internal.keys.KeysSearcher; import org.apache.cassandra.index.transactions.IndexTransaction; import org.apache.cassandra.index.transactions.UpdateTransaction; import org.apache.cassandra.io.sstable.ReducingKeyIterator; import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.schema.IndexMetadata; import org.apache.cassandra.utils.FBUtilities; +import org.apache.cassandra.utils.Pair; import org.apache.cassandra.utils.concurrent.OpOrder; import org.apache.cassandra.utils.concurrent.Refs; +import static org.apache.cassandra.index.internal.CassandraIndex.getFunctions; +import static org.apache.cassandra.index.internal.CassandraIndex.indexCfsMetadata; +import static org.apache.cassandra.index.internal.CassandraIndex.parseTarget; + /** * Clone of KeysIndex used in CassandraIndexTest#testCustomIndexWithCFS to verify * behaviour of flushing CFS backed CUSTOM indexes @@ -145,14 +145,14 @@ public class CustomCassandraIndex implements Index private void setMetadata(IndexMetadata indexDef) { metadata = indexDef; - functions = getFunctions(baseCfs.metadata, indexDef); + Pair target = parseTarget(baseCfs.metadata, indexDef); + functions = getFunctions(indexDef, target); CFMetaData cfm = indexCfsMetadata(baseCfs.metadata, indexDef); indexCfs = ColumnFamilyStore.createColumnFamilyStore(baseCfs.keyspace, cfm.cfName, cfm, baseCfs.getTracker().loadsstables); - assert indexDef.columns.size() == 1 : "Build in indexes on multiple target columns are not supported"; - indexedColumn = indexDef.indexedColumn(baseCfs.metadata); + indexedColumn = target.left; } public Callable getTruncateTask(final long truncatedAt) @@ -174,6 +174,11 @@ public class CustomCassandraIndex implements Index return isPrimaryKeyIndex() || columns.contains(indexedColumn); } + public boolean dependsOn(ColumnDefinition column) + { + return column.equals(indexedColumn); + } + public boolean supportsExpression(ColumnDefinition column, Operator operator) { return indexedColumn.name.equals(column.name) @@ -668,76 +673,4 @@ public class CustomCassandraIndex implements Index .map(SSTableReader::toString) .collect(Collectors.joining(", ")); } - - /** - * Construct the CFMetadata for an index table, the clustering columns in the index table - * vary dependent on the kind of the indexed value. - * @param baseCfsMetadata - * @param indexMetadata - * @return - */ - public static final CFMetaData indexCfsMetadata(CFMetaData baseCfsMetadata, IndexMetadata indexMetadata) - { - CassandraIndexFunctions utils = getFunctions(baseCfsMetadata, indexMetadata); - ColumnDefinition indexedColumn = indexMetadata.indexedColumn(baseCfsMetadata); - AbstractType indexedValueType = utils.getIndexedValueType(indexedColumn); - CFMetaData.Builder builder = CFMetaData.Builder.create(baseCfsMetadata.ksName, - baseCfsMetadata.indexColumnFamilyName(indexMetadata)) - .withId(baseCfsMetadata.cfId) - .withPartitioner(new LocalPartitioner(indexedValueType)) - .addPartitionKey(indexedColumn.name, indexedColumn.type); - - builder.addClusteringColumn("partition_key", baseCfsMetadata.partitioner.partitionOrdering()); - builder = utils.addIndexClusteringColumns(builder, baseCfsMetadata, indexedColumn); - return builder.build().reloadIndexMetadataProperties(baseCfsMetadata); - } - - /** - * Factory method for new CassandraIndex instances - * @param baseCfs - * @param indexMetadata - * @return - */ - public static final CassandraIndex newIndex(ColumnFamilyStore baseCfs, IndexMetadata indexMetadata) - { - return getFunctions(baseCfs.metadata, indexMetadata).newIndexInstance(baseCfs, indexMetadata); - } - - private static CassandraIndexFunctions getFunctions(CFMetaData baseCfMetadata, IndexMetadata indexDef) - { - if (indexDef.isKeys()) - return CassandraIndexFunctions.KEYS_INDEX_FUNCTIONS; - - ColumnDefinition indexedColumn = indexDef.indexedColumn(baseCfMetadata); - if (indexedColumn.type.isCollection() && indexedColumn.type.isMultiCell()) - { - switch (((CollectionType)indexedColumn.type).kind) - { - case LIST: - return CassandraIndexFunctions.COLLECTION_VALUE_INDEX_FUNCTIONS; - case SET: - return CassandraIndexFunctions.COLLECTION_KEY_INDEX_FUNCTIONS; - case MAP: - if (indexDef.options.containsKey(IndexTarget.INDEX_KEYS_OPTION_NAME)) - return CassandraIndexFunctions.COLLECTION_KEY_INDEX_FUNCTIONS; - else if (indexDef.options.containsKey(IndexTarget.INDEX_ENTRIES_OPTION_NAME)) - return CassandraIndexFunctions.COLLECTION_ENTRY_INDEX_FUNCTIONS; - else - return CassandraIndexFunctions.COLLECTION_VALUE_INDEX_FUNCTIONS; - } - } - - switch (indexedColumn.kind) - { - case CLUSTERING: - return CassandraIndexFunctions.CLUSTERING_COLUMN_INDEX_FUNCTIONS; - case REGULAR: - return CassandraIndexFunctions.REGULAR_COLUMN_INDEX_FUNCTIONS; - case PARTITION_KEY: - return CassandraIndexFunctions.PARTITION_KEY_INDEX_FUNCTIONS; - //case COMPACT_VALUE: - // return new CompositesIndexOnCompactValue(); - } - throw new AssertionError(); - } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/fde97c3b/test/unit/org/apache/cassandra/schema/DefsTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/schema/DefsTest.java b/test/unit/org/apache/cassandra/schema/DefsTest.java index ee73b2b..f348c30 100644 --- a/test/unit/org/apache/cassandra/schema/DefsTest.java +++ b/test/unit/org/apache/cassandra/schema/DefsTest.java @@ -44,14 +44,13 @@ import org.apache.cassandra.db.lifecycle.LifecycleTransaction; import org.apache.cassandra.db.marshal.BytesType; import org.apache.cassandra.db.marshal.UTF8Type; import org.apache.cassandra.exceptions.ConfigurationException; -import org.apache.cassandra.index.Index; import org.apache.cassandra.io.sstable.Component; import org.apache.cassandra.io.sstable.Descriptor; import org.apache.cassandra.locator.OldNetworkTopologyStrategy; import org.apache.cassandra.service.MigrationManager; -import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.FBUtilities; +import static org.apache.cassandra.Util.throwAssert; import static org.apache.cassandra.cql3.CQLTester.assertRows; import static org.apache.cassandra.cql3.CQLTester.row; import static org.junit.Assert.assertEquals; @@ -501,6 +500,7 @@ public class DefsTest // persist keyspace definition in the system keyspace SchemaKeyspace.makeCreateKeyspaceMutation(Schema.instance.getKSMetaData(KEYSPACE6), FBUtilities.timestampMicros()).applyUnsafe(); ColumnFamilyStore cfs = Keyspace.open(KEYSPACE6).getColumnFamilyStore(TABLE1i); + String indexName = "birthdate_key_index"; // insert some data. save the sstable descriptor so we can make sure it's marked for delete after the drop QueryProcessor.executeInternal(String.format( @@ -510,37 +510,17 @@ public class DefsTest "key0", "col0", 1L, 1L); cfs.forceBlockingFlush(); - ColumnDefinition indexedColumn = cfs.metadata.getColumnDefinition(ByteBufferUtil.bytes("birthdate")); - IndexMetadata index = cfs.metadata.getIndexes() - .get(indexedColumn) - .iterator() - .next(); - ColumnFamilyStore indexCfs = cfs.indexManager.listIndexes() - .stream() - .filter(i -> i.getIndexMetadata().equals(index)) - .map(Index::getBackingTable) - .findFirst() - .orElseThrow(() -> new AssertionError("Index not found")) - .orElseThrow(() -> new AssertionError("Index has no backing table")); + ColumnFamilyStore indexCfs = cfs.indexManager.getIndexByName(indexName) + .getBackingTable() + .orElseThrow(throwAssert("Cannot access index cfs")); Descriptor desc = indexCfs.getLiveSSTables().iterator().next().descriptor; // drop the index CFMetaData meta = cfs.metadata.copy(); - // We currently have a mismatch between IndexMetadata.name (which is simply the name - // of the index) and what gets returned from SecondaryIndex#getIndexName() (usually, this - // defaults to .. - // IndexMetadata takes its lead from the prior implementation of ColumnDefinition.name - // which did not include the table name. - // This mismatch causes some other, long standing inconsistencies: - // nodetool rebuild_index - must be qualified, i.e. include the redundant table name - // without it, the rebuild silently fails - // system.IndexInfo (which is also exposed over JMX as CF.BuildIndexes) uses the form . - // cqlsh> describe index [.] - here must not be qualified by the table name. - // - // This should get resolved as part of #9459 by better separating the index name from the - // name of it's underlying CFS (if it as one), as the comment in CFMetaData#indexColumnFamilyName promises - // Then we will be able to just use the value of SI#getIndexName() when removing an index from CFMetaData - IndexMetadata existing = meta.getIndexes().iterator().next(); + IndexMetadata existing = cfs.metadata.getIndexes() + .get(indexName) + .orElseThrow(throwAssert("Index not found")); + meta.indexes(meta.getIndexes().without(existing.name)); MigrationManager.announceColumnFamilyUpdate(meta, false); http://git-wip-us.apache.org/repos/asf/cassandra/blob/fde97c3b/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java b/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java index d841e91..7124e40 100644 --- a/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java +++ b/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java @@ -34,6 +34,7 @@ import org.apache.cassandra.cql3.ColumnIdentifier; import org.apache.cassandra.cql3.functions.*; import org.apache.cassandra.db.*; import org.apache.cassandra.db.marshal.*; +import org.apache.cassandra.index.internal.CassandraIndex; import org.apache.cassandra.thrift.ThriftConversion; import static java.lang.String.format; @@ -490,7 +491,7 @@ public class LegacySchemaMigratorTest { IndexMetadata i = index.get(); adder.add("index_name", i.name); - adder.add("index_type", i.indexType.toString()); + adder.add("index_type", i.kind.toString()); adder.add("index_options", json(i.options)); } else @@ -504,11 +505,14 @@ public class LegacySchemaMigratorTest } private static Optional findIndexForColumn(Indexes indexes, - CFMetaData table, - ColumnDefinition column) + CFMetaData table, + ColumnDefinition column) { + // makes the assumptions that the string option denoting the + // index targets can be parsed by CassandraIndex.parseTarget + // which should be true for any pre-3.0 index for (IndexMetadata index : indexes) - if (index.indexedColumn(table).equals(column)) + if (CassandraIndex.parseTarget(table, index).left.equals(column)) return Optional.of(index); return Optional.empty();