diff --git a/CHANGES.txt b/CHANGES.txt index c09abf651d..febc9d5da0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.1 + * Make legacy index rebuilds safe on Gossip -> TCM upgrades (CASSANDRA-20887) * Minor improvements and hardening for IndexHints (CASSANDRA-20888) * Stop repair scheduler if two major versions are detected (CASSANDRA-20048) * Optimize audit logic for batch operations especially when audit is not enabled for DML (CASSANDRA-20885) diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index 5b39dd42da..0d6cd2eb9b 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -477,6 +477,19 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean, Memtable.Owner Directories directories, boolean loadSSTables, boolean registerBookeeping) + { + this(keyspace, columnFamilyName, sstableIdGenerator, initMetadata, directories, loadSSTables, registerBookeeping, true); + } + + @VisibleForTesting + public ColumnFamilyStore(Keyspace keyspace, + String columnFamilyName, + Supplier sstableIdGenerator, + TableMetadata initMetadata, + Directories directories, + boolean loadSSTables, + boolean registerBookeeping, + boolean addIndexes) { assert directories != null; assert initMetadata != null : "null metadata for " + keyspace + ':' + columnFamilyName; @@ -528,9 +541,11 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean, Memtable.Owner // create the private ColumnFamilyStores for the secondary column indexes indexManager = new SecondaryIndexManager(this); - for (IndexMetadata info : initMetadata.indexes) + + if (addIndexes) { - indexManager.addIndex(info, true); + for (IndexMetadata info : initMetadata.indexes) + indexManager.addIndex(info, true); } // See CASSANDRA-16228. We need to ensure that metrics are exposed after the CFS is initialized, @@ -744,18 +759,29 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean, Memtable.Owner } - public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace, TableMetadata metadata, boolean loadSSTables) + public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace, TableMetadata metadata, boolean loadSSTables, boolean addIndexes) { - return createColumnFamilyStore(keyspace, metadata.name, metadata, loadSSTables); + return createColumnFamilyStore(keyspace, metadata.name, metadata, loadSSTables, addIndexes); } public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace, String columnFamily, TableMetadata metadata, - boolean loadSSTables) + boolean loadSSTables, + boolean addIndexes) { Directories directories = new Directories(metadata); - return createColumnFamilyStore(keyspace, columnFamily, metadata, directories, loadSSTables, true); + return createColumnFamilyStore(keyspace, columnFamily, metadata, directories, loadSSTables, true, addIndexes); + } + + public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace, + String columnFamily, + TableMetadata metadata, + Directories directories, + boolean loadSSTables, + boolean registerBookkeeping) + { + return createColumnFamilyStore(keyspace, columnFamily, metadata, directories, loadSSTables, registerBookkeeping, true); } /** This is only directly used by offline tools */ @@ -764,11 +790,12 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean, Memtable.Owner TableMetadata metadata, Directories directories, boolean loadSSTables, - boolean registerBookkeeping) + boolean registerBookkeeping, + boolean addIndexes) { return new ColumnFamilyStore(keyspace, columnFamily, directories.getUIDGenerator(SSTableIdFactory.instance.defaultBuilder()), - metadata, directories, loadSSTables, registerBookkeeping); + metadata, directories, loadSSTables, registerBookkeeping, addIndexes); } /** diff --git a/src/java/org/apache/cassandra/db/Keyspace.java b/src/java/org/apache/cassandra/db/Keyspace.java index f31662660b..cbf6039e42 100644 --- a/src/java/org/apache/cassandra/db/Keyspace.java +++ b/src/java/org/apache/cassandra/db/Keyspace.java @@ -255,10 +255,10 @@ public class Keyspace private Keyspace(String keyspaceName, SchemaProvider schema, boolean loadSSTables) { - this(schema, schema.getKeyspaceMetadata(keyspaceName), loadSSTables); + this(schema, schema.getKeyspaceMetadata(keyspaceName), loadSSTables, true); } - public Keyspace(SchemaProvider schema, KeyspaceMetadata metadata, boolean loadSSTables) + public Keyspace(SchemaProvider schema, KeyspaceMetadata metadata, boolean loadSSTables, boolean addIndexes) { this.schema = schema; this.name = metadata.name; @@ -275,7 +275,7 @@ public class Keyspace for (TableMetadata cfm : metadata.tablesAndViews()) { logger.trace("Initializing {}.{}", getName(), cfm.name); - initCf(cfm, loadSSTables); + initCf(cfm, loadSSTables, addIndexes); } this.viewManager.reload(metadata); @@ -367,7 +367,7 @@ public class Keyspace /** * adds a cf to internal structures, ends up creating disk files). */ - public void initCf(TableMetadata metadata, boolean loadSSTables) + public void initCf(TableMetadata metadata, boolean loadSSTables, boolean addIndexes) { ColumnFamilyStore cfs = columnFamilyStores.get(metadata.id); @@ -377,7 +377,7 @@ public class Keyspace // We don't worry about races here; startup is safe, and adding multiple idential CFs // simultaneously is a "don't do that" scenario. ColumnFamilyStore oldCfs = columnFamilyStores.putIfAbsent(metadata.id, - ColumnFamilyStore.createColumnFamilyStore(this, metadata, loadSSTables)); + ColumnFamilyStore.createColumnFamilyStore(this, metadata, loadSSTables, addIndexes)); // CFS mbean instantiation will error out before we hit this, but in case that changes... if (oldCfs != null) throw new IllegalStateException("added multiple mappings for cf id " + metadata.id); diff --git a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java index b07e446bf2..53100ad499 100644 --- a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java +++ b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java @@ -235,7 +235,8 @@ public abstract class CassandraIndex implements Index indexCfs = ColumnFamilyStore.createColumnFamilyStore(baseCfs.keyspace, tm.name, tm, - baseCfs.getTracker().loadsstables); + baseCfs.getTracker().loadsstables, + true); indexedColumn = target.left; } diff --git a/src/java/org/apache/cassandra/schema/DistributedSchema.java b/src/java/org/apache/cassandra/schema/DistributedSchema.java index 0eceb9b169..77fec54eda 100644 --- a/src/java/org/apache/cassandra/schema/DistributedSchema.java +++ b/src/java/org/apache/cassandra/schema/DistributedSchema.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import org.apache.cassandra.auth.AuthKeyspace; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.cql3.functions.UserFunction; +import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.db.Mutation; import org.apache.cassandra.db.marshal.UserType; @@ -226,7 +227,7 @@ public class DistributedSchema implements MetadataValue schemaChangeNotifier.notifyPreChanges(new SchemaTransformation.SchemaTransformationResult(prev, this, ksDiff)); ksDiff.dropped.forEach(metadata -> dropKeyspace(metadata, true)); - ksDiff.created.forEach(metadata -> keyspaceInstances.put(metadata.name, new Keyspace(Schema.instance, metadata, loadSSTables))); + ksDiff.created.forEach(metadata -> keyspaceInstances.put(metadata.name, new Keyspace(Schema.instance, metadata, loadSSTables, DatabaseDescriptor.isClientOrToolInitialized()))); ksDiff.altered.forEach(delta -> { boolean initialized = Keyspace.isInitialized(); @@ -281,6 +282,22 @@ public class DistributedSchema implements MetadataValue } }); ksDiff.created.forEach(schemaChangeNotifier::notifyKeyspaceCreated); + + ksDiff.created.forEach(ks -> { + if (ks.tables.size() == 0) + return; + + boolean initialized = Keyspace.isInitialized(); + Keyspace keyspace = initialized ? keyspaceInstances.get(ks.name) : null; + + if (keyspace != null) + { + for (ColumnFamilyStore cfs : keyspace.getColumnFamilyStores()) + for (IndexMetadata info : cfs.metadata().indexes) + cfs.indexManager.addIndex(info, true); + } + }); + ksDiff.altered.forEach(delta -> { boolean initialized = Keyspace.isInitialized(); Keyspace keyspace = initialized ? keyspaceInstances.get(delta.before.name) : null; @@ -296,6 +313,13 @@ public class DistributedSchema implements MetadataValue // add tables and views delta.tables.created.forEach(t -> SchemaDiagnostics.tableCreated(Schema.instance, t)); + + delta.tables.created.forEach(t -> { + ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(t.name); + for (IndexMetadata info : cfs.metadata().indexes) + cfs.indexManager.addIndex(info, true); + }); + delta.views.created.forEach(v -> SchemaDiagnostics.tableCreated(Schema.instance, v.metadata)); // update tables and views @@ -369,13 +393,13 @@ public class DistributedSchema implements MetadataValue private void createTable(Keyspace keyspace, TableMetadata table, boolean loadSSTables) { SchemaDiagnostics.tableCreating(Schema.instance, table); - keyspace.initCf(table, loadSSTables); + keyspace.initCf(table, loadSSTables, DatabaseDescriptor.isClientOrToolInitialized()); } private void createView(Keyspace keyspace, ViewMetadata view) { SchemaDiagnostics.tableCreating(Schema.instance, view.metadata); - keyspace.initCf(view.metadata, true); + keyspace.initCf(view.metadata, true, DatabaseDescriptor.isClientOrToolInitialized()); } private void alterTable(Keyspace keyspace, TableMetadata updated) diff --git a/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadata2iUpgradeTest.java b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadata2iUpgradeTest.java new file mode 100644 index 0000000000..1259104284 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadata2iUpgradeTest.java @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.distributed.upgrade; + +import org.junit.Test; + +import org.apache.cassandra.distributed.api.ConsistencyLevel; +import org.apache.cassandra.distributed.api.Feature; +import org.apache.cassandra.distributed.test.TestBaseImpl; + +import static org.apache.cassandra.distributed.upgrade.UpgradeTestBase.v50; + +public class ClusterMetadata2iUpgradeTest extends TestBaseImpl +{ + @Test + public void upgradeIndexIsNotBuiltTest() throws Throwable + { + new UpgradeTestBase.TestCase() + .nodes(1) + .nodesToUpgrade(1) + .withConfig((cfg) -> cfg.with(Feature.GOSSIP)) + .upgradesToCurrentFrom(v50) + .setup((cluster) -> { + cluster.schemaChange("CREATE TABLE " + KEYSPACE + ".tbl (pk int, ck int, v int, PRIMARY KEY (pk, ck))"); + cluster.schemaChange(withKeyspace("create index iii on %s.tbl (ck)")); + cluster.schemaChange(withKeyspace("create index iii2 on %s.tbl (v)")); + for (int i = 0; i < 1000; i++) + cluster.coordinator(1).execute(withKeyspace("insert into %s.tbl (pk, ck, v) values (?, ?, ?)"), ConsistencyLevel.ALL, i, i, i); + cluster.forEach(i -> i.flush(KEYSPACE)); + cluster.forEach(i -> i.executeInternal("truncate system.\"IndexInfo\"")); + }) + .runAfterClusterUpgrade((cluster) -> { + }).run(); + } +} diff --git a/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java b/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java index de9ac3a54d..2a06a715bb 100644 --- a/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java +++ b/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java @@ -183,7 +183,8 @@ public class CustomCassandraIndex implements Index indexCfs = ColumnFamilyStore.createColumnFamilyStore(baseCfs.keyspace, cfm.name, cfm.ref.get(), - baseCfs.getTracker().loadsstables); + baseCfs.getTracker().loadsstables, + false); indexedColumn = target.left; }