Repository: cassandra Updated Branches: refs/heads/trunk 1d032e635 -> 6e19e81db
Avoid always rebuilding secondary indexes at startup patch by Sergio Bossa; reviewed by Andres de la Peña for CASSANDRA-13725 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/6e19e81d Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/6e19e81d Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/6e19e81d Branch: refs/heads/trunk Commit: 6e19e81db8e4c43bf5ef33308de1ae79916bb61c Parents: 1d032e6 Author: AndreÌs de la PenÌa <[email protected]> Authored: Wed Jul 26 16:56:59 2017 +0100 Committer: AndreÌs de la PenÌa <[email protected]> Committed: Wed Jul 26 16:56:59 2017 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../apache/cassandra/db/ColumnFamilyStore.java | 2 +- .../cassandra/index/SecondaryIndexManager.java | 36 ++++++++++++-------- .../apache/cassandra/db/RangeTombstoneTest.java | 4 +-- .../apache/cassandra/db/SecondaryIndexTest.java | 2 +- 5 files changed, 26 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/6e19e81d/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 46bbe3c..b4880fb 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0 + * Avoid always rebuilding secondary indexes at startup (CASSANDRA-13725) * Upgrade JMH from 1.13 to 1.19 * Upgrade SLF4J from 1.7.7 to 1.7.25 * Default for start_native_transport now true if not set in config (CASSANDRA-13656) http://git-wip-us.apache.org/repos/asf/cassandra/blob/6e19e81d/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 98ee7df..73e7db6 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -456,7 +456,7 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean // create the private ColumnFamilyStores for the secondary column indexes indexManager = new SecondaryIndexManager(this); for (IndexMetadata info : metadata.get().indexes) - indexManager.addIndex(info); + indexManager.addIndex(info, true); if (registerBookeeping) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/6e19e81d/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 e253f3b..bc17e19 100644 --- a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java +++ b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java @@ -186,7 +186,7 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum // we call add for every index definition in the collection as // some may not have been created here yet, only added to schema for (IndexMetadata tableIndex : tableIndexes) - addIndex(tableIndex); + addIndex(tableIndex, false); } private Future<?> reloadIndex(IndexMetadata indexDef) @@ -199,13 +199,12 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum } @SuppressWarnings("unchecked") - private synchronized Future<?> createIndex(IndexMetadata indexDef) + private synchronized Future<?> createIndex(IndexMetadata indexDef, boolean isNewCF) { final Index index = createInstance(indexDef); index.register(this); - // now mark as building prior to initializing - markIndexesBuilding(ImmutableSet.of(index), true); + markIndexesBuilding(ImmutableSet.of(index), true, isNewCF); Callable<?> initialBuildTask = null; // if the index didn't register itself, we can probably assume that no initialization needs to happen @@ -255,13 +254,15 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum * Adds and builds a index * * @param indexDef the IndexMetadata describing the index + * @param isNewCF true if the index is added as part of a new table/columnfamily (i.e. loading a CF at startup), + * false for all other cases (i.e. newly added index) */ - public synchronized Future<?> addIndex(IndexMetadata indexDef) + public synchronized Future<?> addIndex(IndexMetadata indexDef, boolean isNewCF) { if (indexes.containsKey(indexDef.name)) return reloadIndex(indexDef); else - return createIndex(indexDef); + return createIndex(indexDef, isNewCF); } /** @@ -422,7 +423,7 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum // Mark all indexes as building: this step must happen first, because if any index can't be marked, the whole // process needs to abort - markIndexesBuilding(indexes, isFullRebuild); + markIndexesBuilding(indexes, isFullRebuild, false); // Build indexes in a try/catch, so that any index not marked as either built or failed will be marked as failed: final Set<Index> builtIndexes = new HashSet<>(); @@ -545,7 +546,9 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum * <p> * Marking an index as "building" practically means: * 1) The index is removed from the "failed" set if this is a full rebuild. - * 2) The index is removed from the system keyspace built indexes. + * 2) The index is removed from the system keyspace built indexes; this only happens if this method is not invoked + * for a new table initialization, as in such case there's no need to remove it (it is either already not present, + * or already present because already built). * <p> * Thread safety is guaranteed by having all methods managing index builds synchronized: being synchronized on * the SecondaryIndexManager instance, it means all invocations for all different indexes will go through the same @@ -554,10 +557,12 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum * {@link #markIndexBuilt(Index, boolean)} or {@link #markIndexFailed(Index)} should be always called after the * rebuilding has finished, so that the index build state can be correctly managed and the index rebuilt. * - * @param indexes the index to be marked as building + * @param indexes the index to be marked as building * @param isFullRebuild {@code true} if this method is invoked as a full index rebuild, {@code false} otherwise + * @param isNewCF {@code true} if this method is invoked when initializing a new table/columnfamily (i.e. loading a CF at startup), + * {@code false} for all other cases (i.e. newly added index) */ - private synchronized void markIndexesBuilding(Set<Index> indexes, boolean isFullRebuild) + private synchronized void markIndexesBuilding(Set<Index> indexes, boolean isFullRebuild, boolean isNewCF) { String keyspaceName = baseCfs.keyspace.getName(); @@ -582,14 +587,14 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum if (isFullRebuild) needsFullRebuild.remove(indexName); - if (counter.getAndIncrement() == 0 && DatabaseDescriptor.isDaemonInitialized()) + if (counter.getAndIncrement() == 0 && DatabaseDescriptor.isDaemonInitialized() && !isNewCF) SystemKeyspace.setIndexRemoved(keyspaceName, indexName); }); } /** * Marks the specified index as built if there are no in progress index builds and the index is not failed. - * {@link #markIndexesBuilding(Set, boolean)} should always be invoked before this method. + * {@link #markIndexesBuilding(Set, boolean, boolean)} should always be invoked before this method. * * @param index the index to be marked as built * @param isFullRebuild {@code true} if this method is invoked as a full index rebuild, {@code false} otherwise @@ -597,14 +602,15 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum private synchronized void markIndexBuilt(Index index, boolean isFullRebuild) { String indexName = index.getIndexMetadata().name; + if (isFullRebuild) + queryableIndexes.add(indexName); + AtomicInteger counter = inProgressBuilds.get(indexName); if (counter != null) { assert counter.get() > 0; if (counter.decrementAndGet() == 0) { - if (isFullRebuild) - queryableIndexes.add(indexName); inProgressBuilds.remove(indexName); if (!needsFullRebuild.contains(indexName) && DatabaseDescriptor.isDaemonInitialized()) SystemKeyspace.setIndexBuilt(baseCfs.keyspace.getName(), indexName); @@ -614,7 +620,7 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum /** * Marks the specified index as failed. - * {@link #markIndexesBuilding(Set, boolean)} should always be invoked before this method. + * {@link #markIndexesBuilding(Set, boolean, boolean)} should always be invoked before this method. * * @param index the index to be marked as built */ http://git-wip-us.apache.org/repos/asf/cassandra/blob/6e19e81d/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 4f9b12f..449a613 100644 --- a/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java +++ b/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java @@ -479,7 +479,7 @@ public class RangeTombstoneTest MigrationManager.announceTableUpdate(updated, true); } - Future<?> rebuild = cfs.indexManager.addIndex(indexDef); + Future<?> rebuild = cfs.indexManager.addIndex(indexDef, false); // If rebuild there is, wait for the rebuild to finish so it doesn't race with the following insertions if (rebuild != null) rebuild.get(); @@ -585,7 +585,7 @@ public class RangeTombstoneTest MigrationManager.announceTableUpdate(updated, true); } - Future<?> rebuild = cfs.indexManager.addIndex(indexDef); + Future<?> rebuild = cfs.indexManager.addIndex(indexDef, false); // If rebuild there is, wait for the rebuild to finish so it doesn't race with the following insertions if (rebuild != null) rebuild.get(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/6e19e81d/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 8341e30..eecb55e 100644 --- a/test/unit/org/apache/cassandra/db/SecondaryIndexTest.java +++ b/test/unit/org/apache/cassandra/db/SecondaryIndexTest.java @@ -503,7 +503,7 @@ public class SecondaryIndexTest assertFalse(cfs.getBuiltIndexes().contains(indexName)); // rebuild & re-query - Future future = cfs.indexManager.addIndex(indexDef); + Future future = cfs.indexManager.addIndex(indexDef, false); future.get(); assertIndexedOne(cfs, ByteBufferUtil.bytes("birthdate"), 1L); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
