Address review comments; cleanup

community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/schema/SchemaDescriptor.java

@@ -39,7 +39,12 @@ public interface SchemaDescriptor extends SchemaDescriptorSupplier
 {
     int[] ANY_ENTITY_TOKEN = new int[0];
 
-    boolean isAffected( long[] entityIds );
+    /**
+     * Returns true if any of the given entity token ids are part of this schema unit.
+     * @param entityTokenIds entity token ids to check against.
+     * @return true if the supplied ids are relevant to this schema unit.
+     */
+    boolean isAffected( long[] entityTokenIds );
 
     /**
      * This enum signifies how this schema should behave in regards to updates.

community/kernel/src/main/java/org/neo4j/kernel/api/schema/LabelSchemaDescriptor.java

@@ -43,9 +43,9 @@ public class LabelSchemaDescriptor implements org.neo4j.internal.kernel.api.sche
     }
 
     @Override
-    public boolean isAffected( long[] entityIds )
+    public boolean isAffected( long[] entityTokenIds )
     {
-        return ArrayUtils.contains( entityIds, labelId );
+        return ArrayUtils.contains( entityTokenIds, labelId );
     }
 
     @Override

community/kernel/src/main/java/org/neo4j/kernel/api/schema/MultiTokenSchemaDescriptor.java

@@ -51,13 +51,20 @@ public class MultiTokenSchemaDescriptor implements org.neo4j.internal.kernel.api
     }
 
     @Override
-    public boolean isAffected( long[] entityIds )
+    public boolean isAffected( long[] entityTokenIds )
     {
-        if ( entityTokens.length == 0 )
+        if ( Arrays.equals( entityTokens, ANY_ENTITY_TOKEN ) )
         {
             return true;
         }
-        return Arrays.stream( entityTokens ).anyMatch( id -> ArrayUtils.contains( entityIds, id ) );
+        for ( int id : entityTokens )
+        {
+            if ( ArrayUtils.contains( entityTokenIds, id ) )
+            {
+                return true;
+            }
+        }
+        return false;
     }
 
     @Override

community/kernel/src/main/java/org/neo4j/kernel/api/schema/RelationTypeSchemaDescriptor.java

@@ -44,9 +44,9 @@ public class RelationTypeSchemaDescriptor implements org.neo4j.internal.kernel.a
     }
 
     @Override
-    public boolean isAffected( long[] entityIds )
+    public boolean isAffected( long[] entityTokenIds )
     {
-        return ArrayUtils.contains( entityIds, relTypeId );
+        return ArrayUtils.contains( entityTokenIds, relTypeId );
     }
 
     @Override

community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/EntityUpdates.java

@@ -46,8 +46,8 @@
 import static org.neo4j.kernel.impl.api.index.EntityUpdates.PropertyValueType.UnChanged;
 
 /**
- * Subclasses of this represent events related to property changes due to node addition, deletion or update.
- * This is of use in populating indexes that might be relevant to node label and property combinations.
+ * Subclasses of this represent events related to property changes due to entity addition, deletion or update.
+ * This is of use in populating indexes that might be relevant to label/reltype and property combinations.
  */
 public class EntityUpdates implements PropertyLoader.PropertyLoadSink
 {
@@ -175,17 +175,18 @@ public <INDEX_KEY extends SchemaDescriptorSupplier> Iterable<IndexEntryUpdate<IN
     }
 
     /**
-     * Matches the provided schema descriptors to the node updates in this object, and generates an IndexEntryUpdate
+     * Matches the provided schema descriptors to the entity updates in this object, and generates an IndexEntryUpdate
      * for any index that needs to be updated.
      *
-     * In some cases the updates to a node are not enough to determine whether some index should be affected. For
+     * In some cases the updates to an entity are not enough to determine whether some index should be affected. For
      * example if we have and index of label :A and property p1, and :A is added to this node, we cannot say whether
      * this should affect the index unless we know if this node has property p1. This get even more complicated for
      * composite indexes. To solve this problem, a propertyLoader is used to load any additional properties needed to
      * make these calls.
      *
      * @param indexKeys The index keys to generate entry updates for
      * @param propertyLoader The property loader used to fetch needed additional properties
+     * @param type EntityType of the indexes
      * @return IndexEntryUpdates for all relevant index keys
      */
     public <INDEX_KEY extends SchemaDescriptorSupplier> Iterable<IndexEntryUpdate<INDEX_KEY>> forIndexKeys(
@@ -211,6 +212,7 @@ public <INDEX_KEY extends SchemaDescriptorSupplier> Iterable<IndexEntryUpdate<IN
         return gatherUpdatesForPotentials( potentiallyRelevant );
     }
 
+    @SuppressWarnings( "ConstantConditions" )
     private <INDEX_KEY extends SchemaDescriptorSupplier> Iterable<IndexEntryUpdate<INDEX_KEY>> gatherUpdatesForPotentials(
             Iterable<INDEX_KEY> potentiallyRelevant )
     {
@@ -231,7 +233,7 @@ else if ( !relevantBefore && relevantAfter )
                 indexUpdates.add( IndexEntryUpdate.add( entityId, indexKey, valuesAfter( propertyIds )
                 ) );
             }
-            else if ( relevantBefore )
+            else if ( relevantBefore && relevantAfter )
             {
                 if ( valuesChanged( propertyIds, schema.propertySchemaType() ) )
                 {
@@ -359,10 +361,14 @@ private Value[] valuesAfter( int[] propertyIds )
         return values;
     }
 
+    /**
+     * This method should only be called in a context where you know that your entity is relevant both before and after
+     */
     private boolean valuesChanged( int[] propertyIds, SchemaDescriptor.PropertySchemaType propertySchemaType )
     {
         if ( propertySchemaType == COMPLETE_ALL_TOKENS )
         {
+            // In the case of indexes were all entries must have all indexed tokens, one of the properties must have changed for us to generate a change.
             for ( int propertyId : propertyIds )
             {
                 if ( knownProperties.get( propertyId ).type == Changed )
@@ -374,9 +380,11 @@ private boolean valuesChanged( int[] propertyIds, SchemaDescriptor.PropertySchem
         }
         else
         {
+            // In the case of indexes were we index incomplete index entries, we need to update as long as _anything_ happened to one of the indexed properties.
             for ( int propertyId : propertyIds )
             {
-                if ( knownProperties.get( propertyId ).type != UnChanged )
+                PropertyValueType type = knownProperties.get( propertyId ).type;
+                if ( type != UnChanged && type != NoValue )
                 {
                     return true;
                 }

community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexMap.java

@@ -33,6 +33,7 @@
 import org.eclipse.collections.impl.map.mutable.primitive.LongObjectHashMap;
 import org.eclipse.collections.impl.map.mutable.primitive.ObjectLongHashMap;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -46,6 +47,8 @@
 import org.neo4j.kernel.api.schema.index.StoreIndexDescriptor;
 import org.neo4j.storageengine.api.EntityType;
 
+import static org.neo4j.internal.kernel.api.schema.SchemaDescriptor.ANY_ENTITY_TOKEN;
+
 /**
  * Bundles various mappings to IndexProxy. Used by IndexingService via IndexMapReference.
  *
@@ -59,7 +62,7 @@ public final class IndexMap implements Cloneable
     private final MutableObjectLongMap<SchemaDescriptor> indexIdsByDescriptor;
     private final MutableIntObjectMap<Set<SchemaDescriptor>> descriptorsByLabel;
     private final MutableIntObjectMap<Set<SchemaDescriptor>> descriptorsByReltype;
-    private final MutableIntObjectMap<Set<SchemaDescriptor>> descriptorsByProperty;
+    private final MutableIntObjectMap<Set<SchemaDescriptor>> nodeDescriptorsByProperty;
     private final MutableIntObjectMap<Set<SchemaDescriptor>> relationshipDescriptorsByProperty;
     private final Set<SchemaDescriptor> descriptorsForAllLabels;
     private final Set<SchemaDescriptor> descriptorsForAllReltypes;
@@ -84,7 +87,7 @@ private IndexMap(
         this.indexIdsByDescriptor = indexIdsByDescriptor;
         this.descriptorsByLabel = new IntObjectHashMap<>();
         this.descriptorsByReltype = new IntObjectHashMap<>();
-        this.descriptorsByProperty = new IntObjectHashMap<>();
+        this.nodeDescriptorsByProperty = new IntObjectHashMap<>();
         this.relationshipDescriptorsByProperty = new IntObjectHashMap<>();
         this.descriptorsForAllLabels = new HashSet<>();
         this.descriptorsForAllReltypes = new HashSet<>();
@@ -131,7 +134,7 @@ public IndexProxy removeIndexProxy( long indexId )
         indexesByDescriptor.remove( schema );
         if ( schema.entityType() == EntityType.NODE )
         {
-            if ( schema.getEntityTokenIds() == SchemaDescriptor.ANY_ENTITY_TOKEN )
+            if ( Arrays.equals(schema.getEntityTokenIds(), ANY_ENTITY_TOKEN ) )
             {
                 descriptorsForAllLabels.remove( schema );
             }
@@ -141,12 +144,12 @@ public IndexProxy removeIndexProxy( long indexId )
             }
             for ( int propertyId : schema.getPropertyIds() )
             {
-                removeFromLookup( propertyId, schema, descriptorsByProperty );
+                removeFromLookup( propertyId, schema, nodeDescriptorsByProperty );
             }
         }
         else if ( schema.entityType() == EntityType.RELATIONSHIP )
         {
-            if ( schema.getEntityTokenIds().length == 0 )
+            if ( Arrays.equals(schema.getEntityTokenIds(), ANY_ENTITY_TOKEN ) )
             {
                 descriptorsForAllReltypes.remove( schema );
             }
@@ -181,38 +184,37 @@ public Iterable<IndexProxy> getAllIndexProxies()
      * @param changedEntityTokens set of labels that have changed
      * @param unchangedEntityTokens set of labels that are unchanged
      * @param properties set of properties
-     * @param entityType
+     * @param entityType type of indexes to get
      * @return set of SchemaDescriptors describing the potentially affected indexes
      */
-    public Set<SchemaDescriptor> getRelatedIndexes( long[] changedEntityTokens, long[] unchangedEntityTokens, IntSet properties,
-            EntityType entityType )
+    public Set<SchemaDescriptor> getRelatedIndexes( long[] changedEntityTokens, long[] unchangedEntityTokens, IntSet properties, EntityType entityType )
     {
-        Function<long[],Set<SchemaDescriptor>> byEnitiyIds;
-        BiFunction<long[],IntSet,Set<SchemaDescriptor>> byProperties;
+        Function<long[],Set<SchemaDescriptor>> indexesByEntityIds;
+        BiFunction<long[],IntSet,Set<SchemaDescriptor>> indexesByProperties;
         if ( entityType == EntityType.NODE )
         {
-            byEnitiyIds = labels -> extractIndexesByEntityTokens( labels, descriptorsByLabel, descriptorsForAllLabels );
-            byProperties = ( unchangedLabels, changedProperties ) -> getDescriptorsByProperties( unchangedLabels, changedProperties, descriptorsByLabel,
-                    descriptorsForAllLabels, descriptorsByProperty );
+            indexesByEntityIds = labels -> extractIndexesByEntityTokens( labels, descriptorsByLabel, descriptorsForAllLabels );
+            indexesByProperties = ( unchangedLabels, changedProperties ) -> getDescriptorsByProperties( unchangedLabels, changedProperties,
+                    descriptorsByLabel, descriptorsForAllLabels, nodeDescriptorsByProperty );
         }
         else
         {
-            byEnitiyIds = reltypes -> extractIndexesByEntityTokens( reltypes, descriptorsByReltype, descriptorsForAllReltypes );
-            byProperties = ( unchangedLabels, changedProperties ) -> getDescriptorsByProperties( unchangedLabels, changedProperties, descriptorsByReltype,
-                    descriptorsForAllReltypes, relationshipDescriptorsByProperty );
+            indexesByEntityIds = reltypes -> extractIndexesByEntityTokens( reltypes, descriptorsByReltype, descriptorsForAllReltypes );
+            indexesByProperties = ( unchangedLabels, changedProperties ) -> getDescriptorsByProperties( unchangedLabels, changedProperties,
+                    descriptorsByReltype, descriptorsForAllReltypes, relationshipDescriptorsByProperty );
         }
 
         if ( properties.isEmpty() )
         {
-            Set<SchemaDescriptor> descriptors = byEnitiyIds.apply( changedEntityTokens );
+            Set<SchemaDescriptor> descriptors = indexesByEntityIds.apply( changedEntityTokens );
             return descriptors == null ? Collections.emptySet() : descriptors;
         }
         if ( changedEntityTokens.length == 0 )
         {
-            return byProperties.apply( unchangedEntityTokens, properties );
+            return indexesByProperties.apply( unchangedEntityTokens, properties );
         }
-        Set<SchemaDescriptor> descriptors = byEnitiyIds.apply( changedEntityTokens );
-        descriptors.addAll( byProperties.apply( unchangedEntityTokens, properties ) );
+        Set<SchemaDescriptor> descriptors = indexesByEntityIds.apply( changedEntityTokens );
+        descriptors.addAll( indexesByProperties.apply( unchangedEntityTokens, properties ) );
 
         return descriptors;
     }
@@ -251,7 +253,7 @@ private void addDescriptorToLookups( SchemaDescriptor schema )
     {
         if ( schema.entityType() == EntityType.NODE )
         {
-            if ( schema.getEntityTokenIds().length == 0 )
+            if ( Arrays.equals(schema.getEntityTokenIds(), ANY_ENTITY_TOKEN ) )
             {
                 descriptorsForAllLabels.add( schema );
             }
@@ -261,12 +263,12 @@ private void addDescriptorToLookups( SchemaDescriptor schema )
             }
             for ( int propertyId : schema.getPropertyIds() )
             {
-                addToLookup( propertyId, schema, descriptorsByProperty );
+                addToLookup( propertyId, schema, nodeDescriptorsByProperty );
             }
         }
         else if ( schema.entityType() == EntityType.RELATIONSHIP )
         {
-            if ( schema.getEntityTokenIds().length == 0 )
+            if ( Arrays.equals(schema.getEntityTokenIds(), ANY_ENTITY_TOKEN ) )
             {
                 descriptorsForAllReltypes.add( schema );
             }
@@ -329,33 +331,36 @@ private static MutableObjectLongMap<SchemaDescriptor> indexIdsByDescriptor( Long
      * the lookup using the unchanged labels or the changed properties given the smallest final
      * set of indexes, and chooses the best way.
      *
-     * @param unchangedLabels set of labels that are unchanged
+     * @param unchangedEntityTokens set of labels that are unchanged
      * @param properties set of properties that have changed
-     * @param descriptorsByEntityToken
-     * @param descriptorsForAllEntityTokens
-     * @param descriptorsByProperty
+     * @param descriptorsByEntityToken map from entity token id to descriptors
+     * @param descriptorsForAllEntityTokens set of descriptors for all entity tokens
+     * @param descriptorsByProperty map from property ids to descriptors
      * @return set of SchemaDescriptors describing the potentially affected indexes
      */
-    private Set<SchemaDescriptor> getDescriptorsByProperties( long[] unchangedLabels, IntSet properties,
+    private Set<SchemaDescriptor> getDescriptorsByProperties( long[] unchangedEntityTokens, IntSet properties,
             IntObjectMap<Set<SchemaDescriptor>> descriptorsByEntityToken, Set<SchemaDescriptor> descriptorsForAllEntityTokens,
             IntObjectMap<Set<SchemaDescriptor>> descriptorsByProperty )
     {
-        int nIndexesForLabels = countIndexesByEntityTokens( unchangedLabels, descriptorsByEntityToken, descriptorsForAllEntityTokens );
+        int nIndexesForEntityTokens = countIndexesByEntityTokens( unchangedEntityTokens, descriptorsByEntityToken, descriptorsForAllEntityTokens );
         int nIndexesForProperties = countIndexesByProperties( properties, descriptorsByProperty );
 
-        if ( nIndexesForLabels == 0 || nIndexesForProperties == 0 )
+        // Our lowest bound is zero applicable indexes, i.e. no indexes are relevant.
+        if ( nIndexesForEntityTokens == 0 || nIndexesForProperties == 0 )
         {
             return Collections.emptySet();
         }
-        if ( unchangedLabels.length == 0 )
+        // Even if we don't have any token ids, we still need to return the anytoken indexes for the given properties.
+        if ( unchangedEntityTokens.length == 0 )
         {
             Set<SchemaDescriptor> descriptors = extractIndexesByProperties( properties, descriptorsByProperty );
             descriptors.retainAll( descriptorsForAllEntityTokens );
             return descriptors;
         }
-        if ( nIndexesForLabels < nIndexesForProperties )
+        // Grab indexes in the fashion that results in the smallest set of indexes.
+        if ( nIndexesForEntityTokens < nIndexesForProperties )
         {
-            return extractIndexesByEntityTokens( unchangedLabels, descriptorsByEntityToken, descriptorsForAllEntityTokens );
+            return extractIndexesByEntityTokens( unchangedEntityTokens, descriptorsByEntityToken, descriptorsForAllEntityTokens );
         }
         else
         {

community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java

@@ -292,14 +292,13 @@ public void start()
             while ( iterator.hasNext() )
             {
                 StoreIndexDescriptor descriptor = iterator.next();
-                StoreIndexDescriptor indexDescriptor = descriptor;
-                if ( indexDescriptor.schema().entityType() == EntityType.NODE )
+                if ( descriptor.schema().entityType() == EntityType.NODE )
                 {
-                    rebuildingNodeDescriptors.put( indexDescriptor.getId(), descriptor );
+                    rebuildingNodeDescriptors.put( descriptor.getId(), descriptor );
                 }
                 else
                 {
-                    rebuildingRelationshipDescriptors.put( indexDescriptor.getId(), descriptor );
+                    rebuildingRelationshipDescriptors.put( descriptor.getId(), descriptor );
                 }
             }
 
@@ -377,12 +376,11 @@ private void performRecoveredIndexDropActions()
     }
 
     private void populate( MutableLongObjectMap<StoreIndexDescriptor> rebuildingDescriptors, IndexMap indexMap, IndexPopulationJob populationJob )
-
     {
         rebuildingDescriptors.forEachKeyValue( ( indexId, descriptor ) ->
         {
-            IndexProxy proxy = indexProxyCreator.createPopulatingIndexProxy( descriptor, false,
-                    // never pass through a tentative online state during recovery
+            IndexProxy proxy = indexProxyCreator.createPopulatingIndexProxy( descriptor,
+         false,// never pass through a tentative online state during recovery
                     monitor, populationJob );
             proxy.start();
             indexMap.putIndexProxy( proxy );
@@ -528,7 +526,7 @@ public Iterable<IndexEntryUpdate<SchemaDescriptor>> convertToIndexUpdates( Entit
                                                 entityUpdates.entityTokensChanged(),
                                                 entityUpdates.entityTokensUnchanged(),
                                                 entityUpdates.propertiesChanged(),
-                                                type);
+                                                type );
 
         return entityUpdates.forIndexKeys( relatedIndexes, storeView, type );
     }

community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/storeview/NeoStoreIndexStoreView.java

@@ -198,11 +198,10 @@ public void loadProperties( long entityId, EntityType type, MutableIntSet proper
             for ( PropertyBlock block : propertyRecord )
             {
                 int currentPropertyId = block.getKeyIndexId();
-                if ( propertyIds.contains( currentPropertyId ) )
+                if ( propertyIds.remove( currentPropertyId ) )
                 {
                     Value currentValue = block.getType().value( block, propertyStore );
                     sink.onProperty( currentPropertyId, currentValue );
-                    propertyIds.remove( currentPropertyId );
                 }
             }
         }

community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/storeview/PropertyAwareEntityStoreScan.java

@@ -51,7 +51,7 @@
 public abstract class PropertyAwareEntityStoreScan<RECORD extends PrimitiveRecord, FAILURE extends Exception> implements StoreScan<FAILURE>
 {
     private final RecordStore<RECORD> store;
-    private boolean continueScanning;
+    private volatile boolean continueScanning;
     private long count;
     private long totalCount;
     private final IntPredicate propertyKeyIdFilter;