Repository: cassandra
Updated Branches:
  refs/heads/cassandra-2.0 850cd59ca -> 1f13efefd


Improve PerRowSecondaryIndex performance

patch by Sergio Bossa; reviewed by Aleksey Yeschenko for CASSANDRA-6876


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/1f13efef
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/1f13efef
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/1f13efef

Branch: refs/heads/cassandra-2.0
Commit: 1f13efefd354db4bfa5ee9c785e35764dca7274f
Parents: 850cd59
Author: Aleksey Yeschenko <alek...@apache.org>
Authored: Tue Mar 18 14:31:31 2014 +0300
Committer: Aleksey Yeschenko <alek...@apache.org>
Committed: Tue Mar 18 14:31:31 2014 +0300

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 src/java/org/apache/cassandra/db/Keyspace.java  |  2 +-
 .../db/index/SecondaryIndexManager.java         | 82 ++++++++++++--------
 3 files changed, 50 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/1f13efef/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 86d4e6f..862ea3e 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -15,6 +15,7 @@
  * Read message id as string from earlier versions (CASSANDRA-6840)
  * Properly use the Paxos consistency for (non-protocol) batch (CASSANDRA-6837)
  * Add paranoid disk failure option (CASSANDRA-6646)
+ * Improve PerRowSecondaryIndex performance (CASSANDRA-6876)
 Merged from 1.2:
  * add extra SSL cipher suites (CASSANDRA-6613)
  * fix nodetool getsstables for blob PK (CASSANDRA-6803)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1f13efef/src/java/org/apache/cassandra/db/Keyspace.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/Keyspace.java 
b/src/java/org/apache/cassandra/db/Keyspace.java
index 36789e2..f5369f9 100644
--- a/src/java/org/apache/cassandra/db/Keyspace.java
+++ b/src/java/org/apache/cassandra/db/Keyspace.java
@@ -389,7 +389,7 @@ public class Keyspace
         if (logger.isDebugEnabled())
             logger.debug("Indexing row {} ", 
cfs.metadata.getKeyValidator().getString(key.key));
 
-        Collection<SecondaryIndex> indexes = 
cfs.indexManager.getIndexesByNames(idxNames);
+        Set<SecondaryIndex> indexes = 
cfs.indexManager.getIndexesByNames(idxNames);
 
         switchLock.readLock().lock();
         try

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1f13efef/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java 
b/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java
index 7bfed33..5e49966 100644
--- a/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java
+++ b/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java
@@ -60,15 +60,18 @@ public class SecondaryIndexManager
      */
     private final ConcurrentNavigableMap<ByteBuffer, SecondaryIndex> 
indexesByColumn;
 
-
     /**
      * Keeps a single instance of a SecondaryIndex for many columns when the 
index type
      * has isRowLevelIndex() == true
      *
      * This allows updates to happen to an entire row at once
      */
-    private final Map<Class<? extends SecondaryIndex>,SecondaryIndex> 
rowLevelIndexMap;
+    private final ConcurrentMap<Class<? extends SecondaryIndex>, 
SecondaryIndex> rowLevelIndexMap;
 
+    /**
+     * Keeps all secondary index instances, either per-column or per-row
+     */
+    private final Set<SecondaryIndex> allIndexes;
 
     /**
      * The underlying column family containing the source data for these 
indexes
@@ -78,7 +81,8 @@ public class SecondaryIndexManager
     public SecondaryIndexManager(ColumnFamilyStore baseCfs)
     {
         indexesByColumn = new ConcurrentSkipListMap<>();
-        rowLevelIndexMap = new HashMap<>();
+        rowLevelIndexMap = new ConcurrentHashMap<>();
+        allIndexes = Collections.newSetFromMap(new 
ConcurrentHashMap<SecondaryIndex, Boolean>());
 
         this.baseCfs = baseCfs;
     }
@@ -104,16 +108,14 @@ public class SecondaryIndexManager
             if (cdef.getIndexType() != null && 
!indexedColumnNames.contains(cdef.name))
                 addIndexedColumn(cdef);
 
-        Set<SecondaryIndex> reloadedIndexes = Collections.newSetFromMap(new 
IdentityHashMap<SecondaryIndex, Boolean>());
-        for (SecondaryIndex index : indexesByColumn.values())
-            if (reloadedIndexes.add(index))
-                index.reload();
+        for (SecondaryIndex index : allIndexes)
+            index.reload();
     }
 
     public Set<String> allIndexesNames()
     {
-        Set<String> names = new HashSet<>(indexesByColumn.size());
-        for (SecondaryIndex index : indexesByColumn.values())
+        Set<String> names = new HashSet<>(allIndexes.size());
+        for (SecondaryIndex index : allIndexes)
             names.add(index.getIndexName());
         return names;
     }
@@ -144,24 +146,33 @@ public class SecondaryIndexManager
         logger.info("Index build of " + idxNames + " complete");
     }
 
-    public boolean indexes(ByteBuffer name, Collection<SecondaryIndex> indexes)
+    public boolean indexes(ByteBuffer name, Set<SecondaryIndex> indexes)
     {
-        return !indexFor(name, indexes).isEmpty();
+        boolean matching = false;
+        for (SecondaryIndex index : indexes)
+        {
+            if (index.indexes(name))
+            {
+                matching = true;
+                break;
+            }
+        }
+        return matching;
     }
 
-    public List<SecondaryIndex> indexFor(ByteBuffer name, 
Collection<SecondaryIndex> indexes)
+    public Set<SecondaryIndex> indexFor(ByteBuffer name, Set<SecondaryIndex> 
indexes)
     {
-        List<SecondaryIndex> matching = null;
+        Set<SecondaryIndex> matching = null;
         for (SecondaryIndex index : indexes)
         {
             if (index.indexes(name))
             {
                 if (matching == null)
-                    matching = new ArrayList<>();
+                    matching = new HashSet<>();
                 matching.add(index);
             }
         }
-        return matching == null ? Collections.<SecondaryIndex>emptyList() : 
matching;
+        return matching == null ? Collections.<SecondaryIndex>emptySet() : 
matching;
     }
 
     public boolean indexes(Column column)
@@ -171,12 +182,12 @@ public class SecondaryIndexManager
 
     public boolean indexes(ByteBuffer name)
     {
-        return indexes(name, indexesByColumn.values());
+        return indexes(name, allIndexes);
     }
 
-    public List<SecondaryIndex> indexFor(ByteBuffer name)
+    public Set<SecondaryIndex> indexFor(ByteBuffer name)
     {
-        return indexFor(name, indexesByColumn.values());
+        return indexFor(name, allIndexes);
     }
 
     /**
@@ -221,6 +232,9 @@ public class SecondaryIndexManager
                 rowLevelIndexMap.remove(index.getClass());
         }
 
+        // Remove from all indexes set:
+        allIndexes.remove(index);
+
         index.removeIndex(column);
         SystemKeyspace.setIndexRemoved(baseCfs.metadata.ksName, 
index.getNameForSystemKeyspace(column));
     }
@@ -281,6 +295,9 @@ public class SecondaryIndexManager
         // until the index is actually built before using in queries.
         indexesByColumn.put(cdef.name, index);
 
+        // Add to all indexes set:
+        allIndexes.add(index);
+
         // if we're just linking in the index to indexedColumns on an
         // already-built index post-restart, we're done
         if (index.isIndexBuilt(cdef.name))
@@ -304,7 +321,7 @@ public class SecondaryIndexManager
      */
     public void invalidate()
     {
-        for (SecondaryIndex index : indexesByColumn.values())
+        for (SecondaryIndex index : allIndexes)
             index.invalidate();
     }
 
@@ -313,7 +330,7 @@ public class SecondaryIndexManager
      */
     public void flushIndexesBlocking()
     {
-        for (SecondaryIndex index : indexesByColumn.values())
+        for (SecondaryIndex index : allIndexes)
             index.forceBlockingFlush();
     }
 
@@ -338,11 +355,11 @@ public class SecondaryIndexManager
     /**
      * @return all CFS from indexes which use a backing CFS internally (KEYS)
      */
-    public Collection<ColumnFamilyStore> getIndexesBackedByCfs()
+    public Set<ColumnFamilyStore> getIndexesBackedByCfs()
     {
-        ArrayList<ColumnFamilyStore> cfsList = new ArrayList<>();
+        Set<ColumnFamilyStore> cfsList = new HashSet<>();
 
-        for (SecondaryIndex index: indexesByColumn.values())
+        for (SecondaryIndex index: allIndexes)
         {
             ColumnFamilyStore cfs = index.getIndexCfs();
             if (cfs != null)
@@ -355,11 +372,11 @@ public class SecondaryIndexManager
     /**
      * @return all indexes which do *not* use a backing CFS internally
      */
-    public Collection<SecondaryIndex> getIndexesNotBackedByCfs()
+    public Set<SecondaryIndex> getIndexesNotBackedByCfs()
     {
         // we use identity map because per row indexes use same instance 
across many columns
         Set<SecondaryIndex> indexes = Collections.newSetFromMap(new 
IdentityHashMap<SecondaryIndex, Boolean>());
-        for (SecondaryIndex index: indexesByColumn.values())
+        for (SecondaryIndex index: allIndexes)
             if (index.getIndexCfs() == null)
                 indexes.add(index);
         return indexes;
@@ -368,12 +385,9 @@ public class SecondaryIndexManager
     /**
      * @return all of the secondary indexes without distinction to the 
(non-)backed by secondary ColumnFamilyStore.
      */
-    public Collection<SecondaryIndex> getIndexes()
+    public Set<SecondaryIndex> getIndexes()
     {
-        // we use identity map because per row indexes use same instance 
across many columns
-        Set<SecondaryIndex> indexes = Collections.newSetFromMap(new 
IdentityHashMap<SecondaryIndex, Boolean>());
-        indexes.addAll(indexesByColumn.values());
-        return indexes;
+        return allIndexes;
     }
 
     /**
@@ -406,7 +420,7 @@ public class SecondaryIndexManager
         // Update entire row only once per row level index
         Set<Class<? extends SecondaryIndex>> appliedRowLevelIndexes = null;
 
-        for (SecondaryIndex index : indexesByColumn.values())
+        for (SecondaryIndex index : allIndexes)
         {
             if (index instanceof PerRowSecondaryIndex)
             {
@@ -538,10 +552,10 @@ public class SecondaryIndexManager
         return indexSearchers.get(0).search(filter);
     }
 
-    public Collection<SecondaryIndex> getIndexesByNames(Set<String> idxNames)
+    public Set<SecondaryIndex> getIndexesByNames(Set<String> idxNames)
     {
-        List<SecondaryIndex> result = new ArrayList<>();
-        for (SecondaryIndex index : indexesByColumn.values())
+        Set<SecondaryIndex> result = new HashSet<>();
+        for (SecondaryIndex index : allIndexes)
             if (idxNames.contains(index.getIndexName()))
                 result.add(index);
         return result;

Reply via email to