You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ad...@apache.org on 2017/07/26 15:57:32 UTC
cassandra git commit: Avoid always rebuilding secondary indexes at
startup
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: Andrés de la Peña <a....@gmail.com>
Authored: Wed Jul 26 16:56:59 2017 +0100
Committer: Andrés de la Peña <a....@gmail.com>
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: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org