Skip to content

Commit 55db318

Browse files
Marcus Erikssonmaedhrozkrummas
committed
Make legacy index rebuilds safe on Gossip -> TCM upgrades
patch by Caleb Rackliffe; reviewed by Marcus Eriksson for CASSANDRA-20887 Co-authored-by: Caleb Rackliffe <[email protected]> Co-authored-by: Marcus Eriksson <[email protected]>
1 parent ed00bf9 commit 55db318

File tree

7 files changed

+123
-18
lines changed

7 files changed

+123
-18
lines changed

‎CHANGES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
5.1
2+
* Make legacy index rebuilds safe on Gossip -> TCM upgrades (CASSANDRA-20887)
23
* Minor improvements and hardening for IndexHints (CASSANDRA-20888)
34
* Stop repair scheduler if two major versions are detected (CASSANDRA-20048)
45
* Optimize audit logic for batch operations especially when audit is not enabled for DML (CASSANDRA-20885)

‎src/java/org/apache/cassandra/db/ColumnFamilyStore.java‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,19 @@ public ColumnFamilyStore(Keyspace keyspace,
477477
Directories directories,
478478
boolean loadSSTables,
479479
boolean registerBookeeping)
480+
{
481+
this(keyspace, columnFamilyName, sstableIdGenerator, initMetadata, directories, loadSSTables, registerBookeeping, true);
482+
}
483+
484+
@VisibleForTesting
485+
public ColumnFamilyStore(Keyspace keyspace,
486+
String columnFamilyName,
487+
Supplier<? extends SSTableId> sstableIdGenerator,
488+
TableMetadata initMetadata,
489+
Directories directories,
490+
boolean loadSSTables,
491+
boolean registerBookeeping,
492+
boolean addIndexes)
480493
{
481494
assert directories != null;
482495
assert initMetadata != null : "null metadata for " + keyspace + ':' + columnFamilyName;
@@ -528,9 +541,11 @@ public ColumnFamilyStore(Keyspace keyspace,
528541

529542
// create the private ColumnFamilyStores for the secondary column indexes
530543
indexManager = new SecondaryIndexManager(this);
531-
for (IndexMetadata info : initMetadata.indexes)
544+
545+
if (addIndexes)
532546
{
533-
indexManager.addIndex(info, true);
547+
for (IndexMetadata info : initMetadata.indexes)
548+
indexManager.addIndex(info, true);
534549
}
535550

536551
// See CASSANDRA-16228. We need to ensure that metrics are exposed after the CFS is initialized,
@@ -744,18 +759,29 @@ void unregisterMBean() throws MalformedObjectNameException
744759
}
745760

746761

747-
public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace, TableMetadata metadata, boolean loadSSTables)
762+
public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace, TableMetadata metadata, boolean loadSSTables, boolean addIndexes)
748763
{
749-
return createColumnFamilyStore(keyspace, metadata.name, metadata, loadSSTables);
764+
return createColumnFamilyStore(keyspace, metadata.name, metadata, loadSSTables, addIndexes);
750765
}
751766

752767
public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace,
753768
String columnFamily,
754769
TableMetadata metadata,
755-
boolean loadSSTables)
770+
boolean loadSSTables,
771+
boolean addIndexes)
756772
{
757773
Directories directories = new Directories(metadata);
758-
return createColumnFamilyStore(keyspace, columnFamily, metadata, directories, loadSSTables, true);
774+
return createColumnFamilyStore(keyspace, columnFamily, metadata, directories, loadSSTables, true, addIndexes);
775+
}
776+
777+
public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace,
778+
String columnFamily,
779+
TableMetadata metadata,
780+
Directories directories,
781+
boolean loadSSTables,
782+
boolean registerBookkeeping)
783+
{
784+
return createColumnFamilyStore(keyspace, columnFamily, metadata, directories, loadSSTables, registerBookkeeping, true);
759785
}
760786

761787
/** This is only directly used by offline tools */
@@ -764,11 +790,12 @@ public static synchronized ColumnFamilyStore createColumnFamilyStore(Keyspace ke
764790
TableMetadata metadata,
765791
Directories directories,
766792
boolean loadSSTables,
767-
boolean registerBookkeeping)
793+
boolean registerBookkeeping,
794+
boolean addIndexes)
768795
{
769796
return new ColumnFamilyStore(keyspace, columnFamily,
770797
directories.getUIDGenerator(SSTableIdFactory.instance.defaultBuilder()),
771-
metadata, directories, loadSSTables, registerBookkeeping);
798+
metadata, directories, loadSSTables, registerBookkeeping, addIndexes);
772799
}
773800

774801
/**

‎src/java/org/apache/cassandra/db/Keyspace.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,10 @@ public static Keyspace forSchema(String keyspaceName, SchemaProvider schema)
255255

256256
private Keyspace(String keyspaceName, SchemaProvider schema, boolean loadSSTables)
257257
{
258-
this(schema, schema.getKeyspaceMetadata(keyspaceName), loadSSTables);
258+
this(schema, schema.getKeyspaceMetadata(keyspaceName), loadSSTables, true);
259259
}
260260

261-
public Keyspace(SchemaProvider schema, KeyspaceMetadata metadata, boolean loadSSTables)
261+
public Keyspace(SchemaProvider schema, KeyspaceMetadata metadata, boolean loadSSTables, boolean addIndexes)
262262
{
263263
this.schema = schema;
264264
this.name = metadata.name;
@@ -275,7 +275,7 @@ public Keyspace(SchemaProvider schema, KeyspaceMetadata metadata, boolean loadSS
275275
for (TableMetadata cfm : metadata.tablesAndViews())
276276
{
277277
logger.trace("Initializing {}.{}", getName(), cfm.name);
278-
initCf(cfm, loadSSTables);
278+
initCf(cfm, loadSSTables, addIndexes);
279279
}
280280

281281
this.viewManager.reload(metadata);
@@ -367,7 +367,7 @@ public KeyspaceWriteHandler getWriteHandler()
367367
/**
368368
* adds a cf to internal structures, ends up creating disk files).
369369
*/
370-
public void initCf(TableMetadata metadata, boolean loadSSTables)
370+
public void initCf(TableMetadata metadata, boolean loadSSTables, boolean addIndexes)
371371
{
372372
ColumnFamilyStore cfs = columnFamilyStores.get(metadata.id);
373373

@@ -377,7 +377,7 @@ public void initCf(TableMetadata metadata, boolean loadSSTables)
377377
// We don't worry about races here; startup is safe, and adding multiple idential CFs
378378
// simultaneously is a "don't do that" scenario.
379379
ColumnFamilyStore oldCfs = columnFamilyStores.putIfAbsent(metadata.id,
380-
ColumnFamilyStore.createColumnFamilyStore(this, metadata, loadSSTables));
380+
ColumnFamilyStore.createColumnFamilyStore(this, metadata, loadSSTables, addIndexes));
381381
// CFS mbean instantiation will error out before we hit this, but in case that changes...
382382
if (oldCfs != null)
383383
throw new IllegalStateException("added multiple mappings for cf id " + metadata.id);

‎src/java/org/apache/cassandra/index/internal/CassandraIndex.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,8 @@ private void setMetadata(IndexMetadata indexDef)
235235
indexCfs = ColumnFamilyStore.createColumnFamilyStore(baseCfs.keyspace,
236236
tm.name,
237237
tm,
238-
baseCfs.getTracker().loadsstables);
238+
baseCfs.getTracker().loadsstables,
239+
true);
239240
indexedColumn = target.left;
240241
}
241242

‎src/java/org/apache/cassandra/schema/DistributedSchema.java‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.cassandra.auth.AuthKeyspace;
2424
import org.apache.cassandra.config.DatabaseDescriptor;
2525
import org.apache.cassandra.cql3.functions.UserFunction;
26+
import org.apache.cassandra.db.ColumnFamilyStore;
2627
import org.apache.cassandra.db.Keyspace;
2728
import org.apache.cassandra.db.Mutation;
2829
import org.apache.cassandra.db.marshal.UserType;
@@ -226,7 +227,7 @@ public void initializeKeyspaceInstances(DistributedSchema prev, boolean loadSSTa
226227
schemaChangeNotifier.notifyPreChanges(new SchemaTransformation.SchemaTransformationResult(prev, this, ksDiff));
227228

228229
ksDiff.dropped.forEach(metadata -> dropKeyspace(metadata, true));
229-
ksDiff.created.forEach(metadata -> keyspaceInstances.put(metadata.name, new Keyspace(Schema.instance, metadata, loadSSTables)));
230+
ksDiff.created.forEach(metadata -> keyspaceInstances.put(metadata.name, new Keyspace(Schema.instance, metadata, loadSSTables, DatabaseDescriptor.isClientOrToolInitialized())));
230231
ksDiff.altered.forEach(delta -> {
231232
boolean initialized = Keyspace.isInitialized();
232233

@@ -281,6 +282,22 @@ public void notifyPostCommit(DistributedSchema prevSchema, boolean loadSSTables)
281282
}
282283
});
283284
ksDiff.created.forEach(schemaChangeNotifier::notifyKeyspaceCreated);
285+
286+
ksDiff.created.forEach(ks -> {
287+
if (ks.tables.size() == 0)
288+
return;
289+
290+
boolean initialized = Keyspace.isInitialized();
291+
Keyspace keyspace = initialized ? keyspaceInstances.get(ks.name) : null;
292+
293+
if (keyspace != null)
294+
{
295+
for (ColumnFamilyStore cfs : keyspace.getColumnFamilyStores())
296+
for (IndexMetadata info : cfs.metadata().indexes)
297+
cfs.indexManager.addIndex(info, true);
298+
}
299+
});
300+
284301
ksDiff.altered.forEach(delta -> {
285302
boolean initialized = Keyspace.isInitialized();
286303
Keyspace keyspace = initialized ? keyspaceInstances.get(delta.before.name) : null;
@@ -296,6 +313,13 @@ public void notifyPostCommit(DistributedSchema prevSchema, boolean loadSSTables)
296313

297314
// add tables and views
298315
delta.tables.created.forEach(t -> SchemaDiagnostics.tableCreated(Schema.instance, t));
316+
317+
delta.tables.created.forEach(t -> {
318+
ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(t.name);
319+
for (IndexMetadata info : cfs.metadata().indexes)
320+
cfs.indexManager.addIndex(info, true);
321+
});
322+
299323
delta.views.created.forEach(v -> SchemaDiagnostics.tableCreated(Schema.instance, v.metadata));
300324

301325
// update tables and views
@@ -369,13 +393,13 @@ private void dropTable(Keyspace keyspace, TableMetadata metadata, boolean dropDa
369393
private void createTable(Keyspace keyspace, TableMetadata table, boolean loadSSTables)
370394
{
371395
SchemaDiagnostics.tableCreating(Schema.instance, table);
372-
keyspace.initCf(table, loadSSTables);
396+
keyspace.initCf(table, loadSSTables, DatabaseDescriptor.isClientOrToolInitialized());
373397
}
374398

375399
private void createView(Keyspace keyspace, ViewMetadata view)
376400
{
377401
SchemaDiagnostics.tableCreating(Schema.instance, view.metadata);
378-
keyspace.initCf(view.metadata, true);
402+
keyspace.initCf(view.metadata, true, DatabaseDescriptor.isClientOrToolInitialized());
379403
}
380404

381405
private void alterTable(Keyspace keyspace, TableMetadata updated)

‎test/distributed/org/apache/cassandra/distributed/upgrade/ClusterMetadata2iUpgradeTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package org.apache.cassandra.distributed.upgrade;
20+
21+
import org.junit.Test;
22+
23+
import org.apache.cassandra.distributed.api.ConsistencyLevel;
24+
import org.apache.cassandra.distributed.api.Feature;
25+
import org.apache.cassandra.distributed.test.TestBaseImpl;
26+
27+
import static org.apache.cassandra.distributed.upgrade.UpgradeTestBase.v50;
28+
29+
public class ClusterMetadata2iUpgradeTest extends TestBaseImpl
30+
{
31+
@Test
32+
public void upgradeIndexIsNotBuiltTest() throws Throwable
33+
{
34+
new UpgradeTestBase.TestCase()
35+
.nodes(1)
36+
.nodesToUpgrade(1)
37+
.withConfig((cfg) -> cfg.with(Feature.GOSSIP))
38+
.upgradesToCurrentFrom(v50)
39+
.setup((cluster) -> {
40+
cluster.schemaChange("CREATE TABLE " + KEYSPACE + ".tbl (pk int, ck int, v int, PRIMARY KEY (pk, ck))");
41+
cluster.schemaChange(withKeyspace("create index iii on %s.tbl (ck)"));
42+
cluster.schemaChange(withKeyspace("create index iii2 on %s.tbl (v)"));
43+
for (int i = 0; i < 1000; i++)
44+
cluster.coordinator(1).execute(withKeyspace("insert into %s.tbl (pk, ck, v) values (?, ?, ?)"), ConsistencyLevel.ALL, i, i, i);
45+
cluster.forEach(i -> i.flush(KEYSPACE));
46+
cluster.forEach(i -> i.executeInternal("truncate system.\"IndexInfo\""));
47+
})
48+
.runAfterClusterUpgrade((cluster) -> {
49+
}).run();
50+
}
51+
}

‎test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ private void setMetadata(IndexMetadata indexDef)
183183
indexCfs = ColumnFamilyStore.createColumnFamilyStore(baseCfs.keyspace,
184184
cfm.name,
185185
cfm.ref.get(),
186-
baseCfs.getTracker().loadsstables);
186+
baseCfs.getTracker().loadsstables,
187+
false);
187188
indexedColumn = target.left;
188189
}
189190

0 commit comments

Comments
 (0)