From 3f3b57b8268412539bef21a13ae249c52c4a4a47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mattias=20Finn=C3=A9?= Date: Mon, 25 Mar 2019 12:03:58 +0100 Subject: [PATCH] Uses efficient schema matching on constraints checking IndexMap caches constraints in addition to indexes. This allows for more efficient schema matching based on the change, instead of going through all constraints and filter. The reading was further cleaned up by making a property key list read in one place and pass in array instead of doing that read arbitrarily in multiple places. This makes the code clearer in what it does and prevents duplicate work. --- ...ltiIndexPopulationConcurrentUpdatesIT.java | 5 +- .../java/org/neo4j/kernel/RecoveryIT.java | 2 +- .../api/KernelTransactionImplementation.java | 2 +- .../neo4j/kernel/impl/api/index/IndexMap.java | 152 ++++++++++++------ .../impl/api/index/IndexMapReference.java | 17 ++ .../impl/api/index/IndexingService.java | 70 +++++++- .../api/index/IndexingServiceFactory.java | 6 +- .../impl/api/index/LabelPropertyMultiSet.java | 97 +++++++---- .../kernel/impl/newapi/AllStoreHolder.java | 13 -- .../impl/newapi/IndexTxStateUpdater.java | 71 ++------ .../kernel/impl/newapi/NodeSchemaMatcher.java | 64 ++------ .../neo4j/kernel/impl/newapi/Operations.java | 123 +++++++++----- .../recordstorage/RecordStorageEngine.java | 2 +- .../command/IndexBatchTransactionApplier.java | 31 +++- .../kernel/impl/api/index/IndexMapTest.java | 15 +- .../impl/api/index/IndexingServiceTest.java | 5 +- .../api/index/LabelPropertyMultiSetTest.java | 10 +- .../impl/newapi/IndexTxStateUpdaterTest.java | 65 ++++---- .../impl/newapi/NodeSchemaMatcherTest.java | 21 +-- .../impl/newapi/OperationsLockTest.java | 14 +- 20 files changed, 469 insertions(+), 316 deletions(-) diff --git a/community/community-it/index-it/src/test/java/schema/MultiIndexPopulationConcurrentUpdatesIT.java b/community/community-it/index-it/src/test/java/schema/MultiIndexPopulationConcurrentUpdatesIT.java index 6a1a359afab58..150bda7e9ef78 100644 --- a/community/community-it/index-it/src/test/java/schema/MultiIndexPopulationConcurrentUpdatesIT.java +++ b/community/community-it/index-it/src/test/java/schema/MultiIndexPopulationConcurrentUpdatesIT.java @@ -85,6 +85,7 @@ import org.neo4j.storageengine.api.StorageReader; import org.neo4j.storageengine.api.schema.IndexDescriptorFactory; import org.neo4j.storageengine.api.schema.IndexReader; +import org.neo4j.storageengine.api.schema.SchemaRule; import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; import org.neo4j.test.rule.EmbeddedDatabaseRule; import org.neo4j.values.storable.Values; @@ -386,9 +387,9 @@ private StoreIndexDescriptor[] createIndexRules( Map labelNameId .toArray( StoreIndexDescriptor[]::new ); } - private List getIndexRules( NeoStores neoStores ) + private List getIndexRules( NeoStores neoStores ) { - return Iterators.asList( new SchemaStorage( neoStores.getSchemaStore() ).indexesGetAll() ); + return Iterators.asList( new SchemaStorage( neoStores.getSchemaStore() ).loadAllSchemaRules() ); } private Map getLabelIdsByName( String... names ) diff --git a/community/community-it/kernel-it/src/test/java/org/neo4j/kernel/RecoveryIT.java b/community/community-it/kernel-it/src/test/java/org/neo4j/kernel/RecoveryIT.java index b3832b44785ef..f91d6aebca5a3 100644 --- a/community/community-it/kernel-it/src/test/java/org/neo4j/kernel/RecoveryIT.java +++ b/community/community-it/kernel-it/src/test/java/org/neo4j/kernel/RecoveryIT.java @@ -72,7 +72,6 @@ import org.neo4j.kernel.api.index.IndexPopulator; import org.neo4j.kernel.api.index.IndexProvider; import org.neo4j.kernel.api.index.IndexUpdater; -import org.neo4j.storageengine.api.NodePropertyAccessor; import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.extension.KernelExtensionFactory; import org.neo4j.kernel.impl.api.index.IndexUpdateMode; @@ -104,6 +103,7 @@ import org.neo4j.logging.NullLog; import org.neo4j.logging.NullLogProvider; import org.neo4j.scheduler.ThreadPoolJobScheduler; +import org.neo4j.storageengine.api.NodePropertyAccessor; import org.neo4j.storageengine.api.StorageEngine; import org.neo4j.storageengine.api.schema.IndexReader; import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java index 3c7ceef712577..f28a39759ec95 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java @@ -229,7 +229,7 @@ public KernelTransactionImplementation( Config config, StatementOperationParts s this.operations = new Operations( allStoreHolder, - new IndexTxStateUpdater( storageReader, allStoreHolder, indexingService ), storageReader, + new IndexTxStateUpdater( allStoreHolder, indexingService ), storageReader, this, new KernelToken( storageReader, this, tokenHolders ), cursors, diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexMap.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexMap.java index 82b4503561ccb..97f6c3ef362db 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexMap.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexMap.java @@ -36,6 +36,9 @@ import java.util.Set; import org.neo4j.internal.kernel.api.schema.SchemaDescriptor; +import org.neo4j.internal.kernel.api.schema.SchemaDescriptorSupplier; +import org.neo4j.kernel.api.schema.constraints.IndexBackedConstraintDescriptor; +import org.neo4j.kernel.impl.store.record.ConstraintRule; import org.neo4j.storageengine.api.EntityType; import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; @@ -48,35 +51,41 @@ public final class IndexMap implements Cloneable { private final MutableLongObjectMap indexesById; + private final MutableLongObjectMap uniquenessConstraintsById; private final Map indexesByDescriptor; private final MutableObjectLongMap indexIdsByDescriptor; - private final LabelPropertyMultiSet descriptorsByLabelThenProperty; - private final LabelPropertyMultiSet descriptorsByReltypeThenProperty; + private final LabelPropertyMultiSet descriptorsByLabelThenProperty; + private final LabelPropertyMultiSet descriptorsByReltypeThenProperty; + private final LabelPropertyMultiSet constraintsByLabelThenProperty; + private final LabelPropertyMultiSet constraintsByRelTypeThenProperty; public IndexMap() { - this( new LongObjectHashMap<>(), new HashMap<>(), new ObjectLongHashMap<>() ); - } - - IndexMap( MutableLongObjectMap indexesById ) - { - this( indexesById, indexesByDescriptor( indexesById ), indexIdsByDescriptor( indexesById ) ); + this( new LongObjectHashMap<>(), new HashMap<>(), new ObjectLongHashMap<>(), new LongObjectHashMap<>() ); } private IndexMap( MutableLongObjectMap indexesById, Map indexesByDescriptor, - MutableObjectLongMap indexIdsByDescriptor ) + MutableObjectLongMap indexIdsByDescriptor, + MutableLongObjectMap uniquenessConstraintsById ) { this.indexesById = indexesById; this.indexesByDescriptor = indexesByDescriptor; this.indexIdsByDescriptor = indexIdsByDescriptor; - this.descriptorsByLabelThenProperty = new LabelPropertyMultiSet(); - this.descriptorsByReltypeThenProperty = new LabelPropertyMultiSet(); + this.uniquenessConstraintsById = uniquenessConstraintsById; + this.descriptorsByLabelThenProperty = new LabelPropertyMultiSet<>(); + this.descriptorsByReltypeThenProperty = new LabelPropertyMultiSet<>(); for ( SchemaDescriptor schema : indexesByDescriptor.keySet() ) { addDescriptorToLookups( schema ); } + this.constraintsByLabelThenProperty = new LabelPropertyMultiSet<>(); + this.constraintsByRelTypeThenProperty = new LabelPropertyMultiSet<>(); + for ( IndexBackedConstraintDescriptor constraint : uniquenessConstraintsById.values() ) + { + addConstraintToLookups( constraint ); + } } public IndexProxy getIndexProxy( long indexId ) @@ -114,14 +123,7 @@ IndexProxy removeIndexProxy( long indexId ) SchemaDescriptor schema = removedProxy.getDescriptor().schema(); indexesByDescriptor.remove( schema ); - if ( schema.entityType() == EntityType.NODE ) - { - descriptorsByLabelThenProperty.remove( schema ); - } - else if ( schema.entityType() == EntityType.RELATIONSHIP ) - { - descriptorsByReltypeThenProperty.remove( schema ); - } + selectIndexesByEntityType( schema.entityType() ).remove( schema ); return removedProxy; } @@ -136,9 +138,61 @@ Iterable getAllIndexProxies() return indexesById.values(); } + void putUniquenessConstraint( ConstraintRule rule ) + { + IndexBackedConstraintDescriptor constraintDescriptor = (IndexBackedConstraintDescriptor) rule.getConstraintDescriptor(); + uniquenessConstraintsById.put( rule.getId(), constraintDescriptor ); + constraintsByLabelThenProperty.add( constraintDescriptor ); + } + + void removeUniquenessConstraint( long constraintId ) + { + IndexBackedConstraintDescriptor constraint = uniquenessConstraintsById.remove( constraintId ); + if ( constraint != null ) + { + selectConstraintsByEntityType( constraint.schema().entityType() ).remove( constraint ); + } + } + + private LabelPropertyMultiSet selectConstraintsByEntityType( EntityType entityType ) + { + switch ( entityType ) + { + case NODE: + return constraintsByLabelThenProperty; + case RELATIONSHIP: + return constraintsByRelTypeThenProperty; + default: + throw new IllegalArgumentException( "Unknown entity type " + entityType ); + } + } + + private LabelPropertyMultiSet selectIndexesByEntityType( EntityType entityType ) + { + switch ( entityType ) + { + case NODE: + return descriptorsByLabelThenProperty; + case RELATIONSHIP: + return descriptorsByReltypeThenProperty; + default: + throw new IllegalArgumentException( "Unknown entity type " + entityType ); + } + } + + boolean hasRelatedSchema( long[] labels, int propertyKey, EntityType entityType ) + { + return selectIndexesByEntityType( entityType ).has( labels, propertyKey ) || selectConstraintsByEntityType( entityType ).has( labels, propertyKey ); + } + + boolean hasRelatedSchema( int label, EntityType entityType ) + { + return selectIndexesByEntityType( entityType ).has( label ) || selectConstraintsByEntityType( entityType ).has( label ); + } + /** - * Get all indexes that would be affected by changes in the input labels and/or properties. The returned - * indexes are guaranteed to contain all affected indexes, but might also contain unaffected indexes as + * Get all descriptors that would be affected by changes in the input labels and/or properties. The returned + * descriptors are guaranteed to contain all affected indexes, but might also contain unaffected indexes as * we cannot provide matching without checking unaffected properties for composite indexes. * * @param changedEntityTokens set of labels that have changed @@ -150,17 +204,26 @@ Iterable getAllIndexProxies() public Set getRelatedIndexes( long[] changedEntityTokens, long[] unchangedEntityTokens, int[] sortedProperties, boolean propertyListIsComplete, EntityType entityType ) { - switch ( entityType ) - { - case NODE: - return getRelatedDescriptors( changedEntityTokens, unchangedEntityTokens, sortedProperties, propertyListIsComplete, - descriptorsByLabelThenProperty ); - case RELATIONSHIP: - return getRelatedDescriptors( changedEntityTokens, unchangedEntityTokens, sortedProperties, propertyListIsComplete, - descriptorsByReltypeThenProperty ); - default: - throw new IllegalArgumentException( "The given EntityType cannot be indexed: " + entityType ); - } + return getRelatedDescriptors( selectIndexesByEntityType( entityType ), changedEntityTokens, unchangedEntityTokens, sortedProperties, + propertyListIsComplete ); + } + + /** + * Get all uniqueness constraints that would be affected by changes in the input labels and/or properties. The returned + * set is guaranteed to contain all affected constraints, but might also contain unaffected constraints as + * we cannot provide matching without checking unaffected properties for composite indexes. + * + * @param changedEntityTokens set of labels that have changed + * @param unchangedEntityTokens set of labels that are unchanged + * @param sortedProperties sorted list of properties + * @param entityType type of indexes to get + * @return set of SchemaDescriptors describing the potentially affected indexes + */ + public Set getRelatedConstraints( long[] changedEntityTokens, long[] unchangedEntityTokens, int[] sortedProperties, + boolean propertyListIsComplete, EntityType entityType ) + { + return getRelatedDescriptors( selectConstraintsByEntityType( entityType ), changedEntityTokens, unchangedEntityTokens, sortedProperties, + propertyListIsComplete ); } /** @@ -170,15 +233,15 @@ public Set getRelatedIndexes( long[] changedEntityTokens, long * @param propertyListIsComplete whether or not the property list is complete. For CREATE/DELETE the list is complete, but may not be for UPDATEs. * @return set of SchemaDescriptors describing the potentially affected indexes */ - Set getRelatedDescriptors( long[] changedLabels, long[] unchangedLabels, int[] sortedProperties, boolean propertyListIsComplete, - LabelPropertyMultiSet set ) + private Set getRelatedDescriptors( LabelPropertyMultiSet set, long[] changedLabels, long[] unchangedLabels, + int[] sortedProperties, boolean propertyListIsComplete ) { - if ( indexesById.isEmpty() ) + if ( set.isEmpty() ) { return Collections.emptySet(); } - Set descriptors = new HashSet<>(); + Set descriptors = new HashSet<>(); if ( propertyListIsComplete ) { set.matchingDescriptorsForCompleteListOfProperties( descriptors, changedLabels, sortedProperties ); @@ -214,7 +277,8 @@ else if ( changedLabels.length == 0 ) @Override public IndexMap clone() { - return new IndexMap( LongObjectHashMap.newMap( indexesById ), cloneMap( indexesByDescriptor ), new ObjectLongHashMap<>( indexIdsByDescriptor ) ); + return new IndexMap( LongObjectHashMap.newMap( indexesById ), cloneMap( indexesByDescriptor ), new ObjectLongHashMap<>( indexIdsByDescriptor ), + LongObjectHashMap.newMap( uniquenessConstraintsById ) ); } public Iterator descriptors() @@ -243,14 +307,12 @@ private Map cloneMap( Map map ) private void addDescriptorToLookups( SchemaDescriptor schema ) { - if ( schema.entityType() == EntityType.NODE ) - { - descriptorsByLabelThenProperty.add( schema ); - } - else if ( schema.entityType() == EntityType.RELATIONSHIP ) - { - descriptorsByReltypeThenProperty.add( schema ); - } + selectIndexesByEntityType( schema.entityType() ).add( schema ); + } + + private void addConstraintToLookups( IndexBackedConstraintDescriptor constraint ) + { + selectConstraintsByEntityType( constraint.schema().entityType() ).add( constraint ); } private static Map indexesByDescriptor( LongObjectMap indexesById ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexMapReference.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexMapReference.java index 61074e2eeee22..822fb572c316a 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexMapReference.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexMapReference.java @@ -25,6 +25,7 @@ import org.neo4j.function.ThrowingFunction; import org.neo4j.internal.kernel.api.exceptions.schema.IndexNotFoundKernelException; import org.neo4j.internal.kernel.api.schema.SchemaDescriptor; +import org.neo4j.kernel.api.schema.constraints.IndexBackedConstraintDescriptor; import org.neo4j.storageengine.api.EntityType; import org.neo4j.values.storable.Value; @@ -109,6 +110,22 @@ public Collection getRelatedIndexes( long[] changedEntityToken return indexMap.getRelatedIndexes( changedEntityTokens, unchangedEntityTokens, sortedProperties, propertyListIsComplete, entityType ); } + public Collection getRelatedConstraints( long[] changedLabels, long[] unchangedLabels, int[] sortedProperties, + boolean propertyListIsComplete, EntityType entityType ) + { + return indexMap.getRelatedConstraints( changedLabels, unchangedLabels, sortedProperties, propertyListIsComplete, entityType ); + } + + public boolean hasRelatedSchema( long[] labels, int propertyKey, EntityType entityType ) + { + return indexMap.hasRelatedSchema( labels, propertyKey, entityType ); + } + + public boolean hasRelatedSchema( int label, EntityType entityType ) + { + return indexMap.hasRelatedSchema( label, entityType ); + } + public IndexUpdaterMap createIndexUpdaterMap( IndexUpdateMode mode ) { return new IndexUpdaterMap( indexMap, mode ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java index 655be5892c23b..7fbb663127a57 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java @@ -65,10 +65,12 @@ import org.neo4j.kernel.api.index.IndexPopulator; import org.neo4j.kernel.api.index.IndexProvider; import org.neo4j.kernel.api.index.IndexUpdater; +import org.neo4j.kernel.api.schema.constraints.IndexBackedConstraintDescriptor; import org.neo4j.kernel.impl.api.SchemaState; import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingController; import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingMode; import org.neo4j.kernel.impl.store.UnderlyingStorageException; +import org.neo4j.kernel.impl.store.record.ConstraintRule; import org.neo4j.kernel.impl.transaction.state.IndexUpdates; import org.neo4j.kernel.lifecycle.LifecycleAdapter; import org.neo4j.logging.Log; @@ -79,6 +81,7 @@ import org.neo4j.storageengine.api.EntityType; import org.neo4j.storageengine.api.NodePropertyAccessor; import org.neo4j.storageengine.api.schema.IndexDescriptor; +import org.neo4j.storageengine.api.schema.SchemaRule; import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; import org.neo4j.values.storable.Value; @@ -111,7 +114,7 @@ public class IndexingService extends LifecycleAdapter implements IndexingUpdateS private final IndexStoreView storeView; private final IndexProviderMap providerMap; private final IndexMapReference indexMapRef; - private final Iterable indexDescriptors; + private final Iterable schemaRules; private final Log internalLog; private final Log userLog; private final TokenNameLookup tokenNameLookup; @@ -179,7 +182,7 @@ public void awaitingPopulationOfRecoveredIndex( StoreIndexDescriptor descriptor IndexProviderMap providerMap, IndexMapReference indexMapRef, IndexStoreView storeView, - Iterable indexDescriptors, + Iterable schemaRules, IndexSamplingController samplingController, TokenNameLookup tokenNameLookup, JobScheduler scheduler, @@ -193,7 +196,7 @@ public void awaitingPopulationOfRecoveredIndex( StoreIndexDescriptor descriptor this.providerMap = providerMap; this.indexMapRef = indexMapRef; this.storeView = storeView; - this.indexDescriptors = indexDescriptors; + this.schemaRules = schemaRules; this.samplingController = samplingController; this.tokenNameLookup = tokenNameLookup; this.schemaState = schemaState; @@ -216,8 +219,19 @@ public void init() indexMapRef.modify( indexMap -> { Map> indexStates = new EnumMap<>( InternalIndexState.class ); - for ( StoreIndexDescriptor indexDescriptor : indexDescriptors ) + for ( SchemaRule rule : schemaRules ) { + if ( rule instanceof ConstraintRule ) + { + ConstraintRule constraintRule = (ConstraintRule) rule; + if ( constraintRule.getConstraintDescriptor().enforcesUniqueness() ) + { + indexMap.putUniquenessConstraint( constraintRule ); + } + continue; + } + + StoreIndexDescriptor indexDescriptor = (StoreIndexDescriptor) rule; IndexProxy indexProxy; IndexProviderDescriptor providerDescriptor = indexDescriptor.providerDescriptor(); @@ -621,6 +635,27 @@ else if ( index != null ) } ); } + public void putConstraint( ConstraintRule rule ) + { + indexMapRef.modify( indexMap -> + { + if ( rule.getConstraintDescriptor().enforcesUniqueness() ) + { + indexMap.putUniquenessConstraint( rule ); + } + return indexMap; + } ); + } + + public void removeConstraint( long ruleId ) + { + indexMapRef.modify( indexMap -> + { + indexMap.removeUniquenessConstraint( ruleId ); + return indexMap; + } ); + } + public void triggerIndexSampling( IndexSamplingMode mode ) { internalLog.info( "Manual trigger for sampling all indexes [" + mode + "]" ); @@ -829,7 +864,32 @@ private void logIndexProviderSummary( Map getRelatedIndexes( long[] labels, int propertyKeyId, EntityType entityType ) { - return indexMapRef.getRelatedIndexes( PrimitiveLongCollections.EMPTY_LONG_ARRAY, labels, new int[] {propertyKeyId}, false, entityType ); + return indexMapRef.getRelatedIndexes( PrimitiveLongCollections.EMPTY_LONG_ARRAY, labels, new int[]{propertyKeyId}, false, entityType ); + } + + public Collection getRelatedIndexes( long[] labels, int[] propertyKeyIds, EntityType entityType ) + { + return indexMapRef.getRelatedIndexes( labels, PrimitiveLongCollections.EMPTY_LONG_ARRAY, propertyKeyIds, true, entityType ); + } + + public Collection getRelatedUniquenessConstraints( long[] labels, int propertyKeyId, EntityType entityType ) + { + return indexMapRef.getRelatedConstraints( PrimitiveLongCollections.EMPTY_LONG_ARRAY, labels, new int[] {propertyKeyId}, false, entityType ); + } + + public Collection getRelatedUniquenessConstraints( long[] labels, int[] propertyKeyIds, EntityType entityType ) + { + return indexMapRef.getRelatedConstraints( labels, PrimitiveLongCollections.EMPTY_LONG_ARRAY, propertyKeyIds, true, entityType ); + } + + public boolean hasRelatedSchema( long[] labels, int propertyKey, EntityType entityType ) + { + return indexMapRef.hasRelatedSchema( labels, propertyKey, entityType ); + } + + public boolean hasRelatedSchema( int label, EntityType entityType ) + { + return indexMapRef.hasRelatedSchema( label, entityType ); } private final class IndexPopulationStarter implements Function diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingServiceFactory.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingServiceFactory.java index 7029a9ee62eb0..9dc99e1666208 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingServiceFactory.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingServiceFactory.java @@ -27,7 +27,7 @@ import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingControllerFactory; import org.neo4j.logging.LogProvider; import org.neo4j.scheduler.JobScheduler; -import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; +import org.neo4j.storageengine.api.schema.SchemaRule; /** * Factory to create {@link IndexingService} @@ -43,7 +43,7 @@ public static IndexingService createIndexingService( Config config, IndexProviderMap providerMap, IndexStoreView storeView, TokenNameLookup tokenNameLookup, - Iterable indexRules, + Iterable schemaRules, LogProvider internalLogProvider, LogProvider userLogProvider, IndexingService.Monitor monitor, @@ -58,7 +58,7 @@ public static IndexingService createIndexingService( Config config, IndexProxyCreator proxySetup = new IndexProxyCreator( samplingConfig, storeView, providerMap, tokenNameLookup, internalLogProvider ); - return new IndexingService( proxySetup, providerMap, indexMapRef, storeView, indexRules, + return new IndexingService( proxySetup, providerMap, indexMapRef, storeView, schemaRules, indexSamplingController, tokenNameLookup, scheduler, schemaState, multiPopulatorFactory, internalLogProvider, userLogProvider, monitor ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/LabelPropertyMultiSet.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/LabelPropertyMultiSet.java index 90307197d071d..c0c255eb5dd6f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/LabelPropertyMultiSet.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/LabelPropertyMultiSet.java @@ -28,6 +28,9 @@ import java.util.Set; import org.neo4j.internal.kernel.api.schema.SchemaDescriptor; +import org.neo4j.internal.kernel.api.schema.SchemaDescriptorSupplier; + +import static java.lang.Math.toIntExact; /** * Collects and provides efficient access to {@link SchemaDescriptor}, based on label ids and partial or full list of properties. @@ -62,18 +65,51 @@ * -> property[7]: E * */ -public class LabelPropertyMultiSet +public class LabelPropertyMultiSet { private final MutableIntObjectMap byLabel = IntObjectMaps.mutable.empty(); - public void add( SchemaDescriptor schemaDescriptor ) + boolean isEmpty() + { + return byLabel.isEmpty(); + } + + boolean has( long[] labels, int propertyKey ) + { + if ( byLabel.isEmpty() ) + { + return false; + } + + for ( long label : labels ) + { + PropertyMultiSet byProperty = byLabel.get( toIntExact( label ) ); + if ( byProperty != null && byProperty.has( propertyKey ) ) + { + return true; + } + } + return false; + } + + boolean has( int label ) + { + if ( byLabel.isEmpty() ) + { + return false; + } + + return byLabel.containsKey( label ); + } + + public void add( T schemaDescriptor ) { - if ( schemaDescriptor.getEntityTokenIds().length > 1 ) + if ( schemaDescriptor.schema().getEntityTokenIds().length > 1 ) { throw new UnsupportedOperationException(); } - int label = schemaDescriptor.keyId(); + int label = schemaDescriptor.schema().keyId(); PropertyMultiSet byProperty = byLabel.get( label ); if ( byProperty == null ) { @@ -83,9 +119,9 @@ public void add( SchemaDescriptor schemaDescriptor ) byProperty.add( schemaDescriptor ); } - public void remove( SchemaDescriptor schemaDescriptor ) + public void remove( T schemaDescriptor ) { - int label = schemaDescriptor.keyId(); + int label = schemaDescriptor.schema().keyId(); PropertyMultiSet byProperty = byLabel.get( label ); if ( byProperty != null && byProperty.remove( schemaDescriptor ) ) { @@ -93,7 +129,7 @@ public void remove( SchemaDescriptor schemaDescriptor ) } } - void matchingDescriptorsForCompleteListOfProperties( Collection into, long[] labels, int[] sortedProperties ) + void matchingDescriptorsForCompleteListOfProperties( Collection into, long[] labels, int[] sortedProperties ) { for ( long label : labels ) { @@ -105,7 +141,7 @@ void matchingDescriptorsForCompleteListOfProperties( Collection into, long[] labels, int[] sortedProperties ) + void matchingDescriptorsForPartialListOfProperties( Collection into, long[] labels, int[] sortedProperties ) { for ( long label : labels ) { @@ -117,7 +153,7 @@ void matchingDescriptorsForPartialListOfProperties( Collection } } - void matchingDescriptors( Collection into, long[] labels ) + void matchingDescriptors( Collection into, long[] labels ) { for ( long label : labels ) { @@ -129,17 +165,17 @@ void matchingDescriptors( Collection into, long[] labels ) } } - private static class PropertyMultiSet + private class PropertyMultiSet { - private final Set descriptors = new HashSet<>(); + private final Set descriptors = new HashSet<>(); private final MutableIntObjectMap next = IntObjectMaps.mutable.empty(); - private final MutableIntObjectMap> byAnyProperty = IntObjectMaps.mutable.empty(); + private final MutableIntObjectMap> byAnyProperty = IntObjectMaps.mutable.empty(); - void add( SchemaDescriptor schemaDescriptor ) + void add( T schemaDescriptor ) { // Add optimized path for when property list is fully known descriptors.add( schemaDescriptor ); - int[] propertyKeyIds = sortedPropertyKeyIds( schemaDescriptor ); + int[] propertyKeyIds = sortedPropertyKeyIds( schemaDescriptor.schema() ); int propertyKeyId = propertyKeyIds[0]; PropertySet firstPropertySet = next.get( propertyKeyId ); if ( firstPropertySet == null ) @@ -152,7 +188,7 @@ void add( SchemaDescriptor schemaDescriptor ) // Add fall-back path for when property list is only partly known for ( int keyId : propertyKeyIds ) { - Set byProperty = byAnyProperty.get( keyId ); + Set byProperty = byAnyProperty.get( keyId ); if ( byProperty == null ) { byProperty = new HashSet<>(); @@ -167,11 +203,11 @@ void add( SchemaDescriptor schemaDescriptor ) * @param schemaDescriptor the {@link SchemaDescriptor} to remove. * @return {@code true} if this multi-set ended up empty after removing this descriptor. */ - boolean remove( SchemaDescriptor schemaDescriptor ) + boolean remove( T schemaDescriptor ) { // Remove from the optimized path descriptors.remove( schemaDescriptor ); - int[] propertyKeyIds = sortedPropertyKeyIds( schemaDescriptor ); + int[] propertyKeyIds = sortedPropertyKeyIds( schemaDescriptor.schema() ); int propertyKeyId = propertyKeyIds[0]; PropertySet firstPropertySet = next.get( propertyKeyId ); if ( firstPropertySet != null && firstPropertySet.remove( schemaDescriptor, propertyKeyIds, 0 ) ) @@ -182,7 +218,7 @@ boolean remove( SchemaDescriptor schemaDescriptor ) // Remove from the fall-back path for ( int keyId : propertyKeyIds ) { - Set byProperty = byAnyProperty.get( keyId ); + Set byProperty = byAnyProperty.get( keyId ); if ( byProperty != null ) { byProperty.remove( schemaDescriptor ); @@ -195,7 +231,7 @@ boolean remove( SchemaDescriptor schemaDescriptor ) return descriptors.isEmpty() && next.isEmpty(); } - void collectForCompleteListOfProperties( Collection descriptors, int[] sortedProperties ) + void collectForCompleteListOfProperties( Collection descriptors, int[] sortedProperties ) { for ( int i = 0; i < sortedProperties.length; i++ ) { @@ -207,11 +243,11 @@ void collectForCompleteListOfProperties( Collection descriptor } } - void collectForPartialListOfProperties( Collection descriptors, int[] sortedProperties ) + void collectForPartialListOfProperties( Collection descriptors, int[] sortedProperties ) { for ( int propertyKeyId : sortedProperties ) { - Set propertyDescriptors = byAnyProperty.get( propertyKeyId ); + Set propertyDescriptors = byAnyProperty.get( propertyKeyId ); if ( propertyDescriptors != null ) { descriptors.addAll( propertyDescriptors ); @@ -219,12 +255,12 @@ void collectForPartialListOfProperties( Collection descriptors } } - void collectAll( Collection descriptors ) + void collectAll( Collection descriptors ) { descriptors.addAll( this.descriptors ); } - private static int[] sortedPropertyKeyIds( SchemaDescriptor schemaDescriptor ) + private int[] sortedPropertyKeyIds( SchemaDescriptor schemaDescriptor ) { int[] propertyKeyIds = schemaDescriptor.getPropertyIds(); if ( propertyKeyIds.length > 1 ) @@ -234,12 +270,17 @@ private static int[] sortedPropertyKeyIds( SchemaDescriptor schemaDescriptor ) } return propertyKeyIds; } + + boolean has( int propertyKey ) + { + return byAnyProperty.containsKey( propertyKey ); + } } - private static class PropertySet + private class PropertySet { private final int propertyKey; - private final Set fullDescriptors = new HashSet<>(); + private final Set fullDescriptors = new HashSet<>(); private final MutableIntObjectMap next = IntObjectMaps.mutable.empty(); PropertySet( int propertyKey ) @@ -247,7 +288,7 @@ private static class PropertySet this.propertyKey = propertyKey; } - void add( SchemaDescriptor schemaDescriptor, int[] propertyKeyIds, int cursor ) + void add( T schemaDescriptor, int[] propertyKeyIds, int cursor ) { if ( cursor == propertyKeyIds.length - 1 ) { @@ -272,7 +313,7 @@ void add( SchemaDescriptor schemaDescriptor, int[] propertyKeyIds, int cursor ) * @param cursor which property key among the sorted property keys that this set deals with. * @return {@code true} if this {@link PropertySet} ends up empty after this removal. */ - boolean remove( SchemaDescriptor schemaDescriptor, int[] propertyKeyIds, int cursor ) + boolean remove( T schemaDescriptor, int[] propertyKeyIds, int cursor ) { if ( cursor == propertyKeyIds.length - 1 ) { @@ -290,7 +331,7 @@ boolean remove( SchemaDescriptor schemaDescriptor, int[] propertyKeyIds, int cur return fullDescriptors.isEmpty() && next.isEmpty(); } - void collectForCompleteListOfProperties( Collection descriptors, int[] sortedProperties, int cursor ) + void collectForCompleteListOfProperties( Collection descriptors, int[] sortedProperties, int cursor ) { assert sortedProperties[cursor] == propertyKey; descriptors.addAll( fullDescriptors ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java index 9adef31b15ac4..3c23e428093ca 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java @@ -89,7 +89,6 @@ import static org.neo4j.helpers.collection.Iterators.filter; import static org.neo4j.helpers.collection.Iterators.iterator; import static org.neo4j.helpers.collection.Iterators.singleOrNull; -import static org.neo4j.internal.kernel.api.schema.SchemaDescriptorPredicates.hasProperty; import static org.neo4j.register.Registers.newDoubleLongRegister; import static org.neo4j.storageengine.api.txstate.TxStateVisitor.EMPTY; @@ -722,18 +721,6 @@ public Iterator constraintsGetAll() return Iterators.map( this::lockConstraint, constraints ); } - Iterator constraintsGetForProperty( int propertyKey ) - { - ktx.assertOpen(); - Iterator constraints = storageReader.constraintsGetAll(); - if ( ktx.hasTxStateWithChanges() ) - { - constraints = ktx.txState().constraintsChanges().apply( constraints ); - } - return Iterators.map( this::lockConstraint, - Iterators.filter( hasProperty( propertyKey ), constraints ) ); - } - @Override public Iterator constraintsGetForRelationshipType( int typeId ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/IndexTxStateUpdater.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/IndexTxStateUpdater.java index 1b2ebf7a3aae0..415a0e1d4deca 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/IndexTxStateUpdater.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/IndexTxStateUpdater.java @@ -21,20 +21,14 @@ import org.apache.commons.lang3.ArrayUtils; import org.eclipse.collections.api.map.primitive.MutableIntObjectMap; -import org.eclipse.collections.api.set.primitive.IntSet; -import org.eclipse.collections.api.set.primitive.MutableIntSet; import org.eclipse.collections.impl.factory.primitive.IntObjectMaps; -import org.eclipse.collections.impl.set.mutable.primitive.IntHashSet; import java.util.Collection; -import java.util.Iterator; import org.neo4j.internal.kernel.api.NodeCursor; import org.neo4j.internal.kernel.api.PropertyCursor; import org.neo4j.internal.kernel.api.schema.SchemaDescriptor; import org.neo4j.kernel.impl.api.index.IndexingService; -import org.neo4j.storageengine.api.StorageReader; -import org.neo4j.storageengine.api.schema.IndexDescriptor; import org.neo4j.values.storable.Value; import org.neo4j.values.storable.ValueTuple; @@ -47,15 +41,11 @@ */ public class IndexTxStateUpdater { - private final StorageReader storageReader; private final Read read; private final IndexingService indexingService; - // We can use the StorageReader directly instead of the SchemaReadOps, because we know that in transactions - // where this class is needed we will never have index changes. - public IndexTxStateUpdater( StorageReader storageReader, Read read, IndexingService indexingService ) + public IndexTxStateUpdater( Read read, IndexingService indexingService ) { - this.storageReader = storageReader; this.read = read; this.indexingService = indexingService; } @@ -72,38 +62,23 @@ public enum LabelChangeType * A label has been changed, figure out what updates are needed to tx state. * * @param labelId The id of the changed label + * @param existingPropertyKeyIds all property key ids the node has, sorted by id * @param node cursor to the node where the change was applied * @param propertyCursor cursor to the properties of node * @param changeType The type of change event */ - void onLabelChange( int labelId, NodeCursor node, PropertyCursor propertyCursor, LabelChangeType changeType ) + void onLabelChange( int labelId, int[] existingPropertyKeyIds, NodeCursor node, PropertyCursor propertyCursor, LabelChangeType changeType ) { assert noSchemaChangedInTx(); // Check all indexes of the changed label - Iterator indexes = storageReader.indexesGetForLabel( labelId ); - - if ( !indexes.hasNext() ) - { - return; - } - - // Find properties of the changed node - final MutableIntSet nodePropertyIds = new IntHashSet(); - node.properties( propertyCursor ); - while ( propertyCursor.next() ) - { - nodePropertyIds.add( propertyCursor.propertyKey() ); - } - - // Check all indexes of the changed label - MutableIntObjectMap materializedProperties = IntObjectMaps.mutable.empty(); - while ( indexes.hasNext() ) + Collection indexes = indexingService.getRelatedIndexes( new long[]{labelId}, existingPropertyKeyIds, NODE ); + if ( !indexes.isEmpty() ) { - IndexDescriptor index = indexes.next(); - int[] indexPropertyIds = index.schema().getPropertyIds(); - if ( nodeHasIndexProperties( nodePropertyIds, indexPropertyIds ) ) + MutableIntObjectMap materializedProperties = IntObjectMaps.mutable.empty(); + for ( SchemaDescriptor index : indexes ) { + int[] indexPropertyIds = index.schema().getPropertyIds(); Value[] values = getValueTuple( node, propertyCursor, NO_SUCH_PROPERTY_KEY, NO_VALUE, indexPropertyIds, materializedProperties ); switch ( changeType ) { @@ -128,15 +103,15 @@ private boolean noSchemaChangedInTx() //PROPERTY CHANGES - void onPropertyAdd( NodeCursor node, PropertyCursor propertyCursor, long[] labels, int propertyKeyId, Value value ) + void onPropertyAdd( NodeCursor node, PropertyCursor propertyCursor, long[] labels, int propertyKeyId, int[] existingPropertyKeyIds, Value value ) { assert noSchemaChangedInTx(); Collection indexes = indexingService.getRelatedIndexes( labels, propertyKeyId, NODE ); if ( !indexes.isEmpty() ) { MutableIntObjectMap materializedProperties = IntObjectMaps.mutable.empty(); - NodeSchemaMatcher.onMatchingSchema( indexes.iterator(), node, propertyCursor, propertyKeyId, - ( index, propertyKeyIds ) -> + NodeSchemaMatcher.onMatchingSchema( indexes.iterator(), propertyKeyId, existingPropertyKeyIds, + index -> { Value[] values = getValueTuple( node, propertyCursor, propertyKeyId, value, index.schema().getPropertyIds(), materializedProperties ); indexingService.validateBeforeCommit( index.schema(), values ); @@ -145,15 +120,15 @@ void onPropertyAdd( NodeCursor node, PropertyCursor propertyCursor, long[] label } } - void onPropertyRemove( NodeCursor node, PropertyCursor propertyCursor, long[] labels, int propertyKeyId, Value value ) + void onPropertyRemove( NodeCursor node, PropertyCursor propertyCursor, long[] labels, int propertyKeyId, int[] existingPropertyKeyIds, Value value ) { assert noSchemaChangedInTx(); Collection indexes = indexingService.getRelatedIndexes( labels, propertyKeyId, NODE ); if ( !indexes.isEmpty() ) { MutableIntObjectMap materializedProperties = IntObjectMaps.mutable.empty(); - NodeSchemaMatcher.onMatchingSchema( indexes.iterator(), node, propertyCursor, propertyKeyId, - ( index, propertyKeyIds ) -> + NodeSchemaMatcher.onMatchingSchema( indexes.iterator(), propertyKeyId, existingPropertyKeyIds, + index -> { Value[] values = getValueTuple( node, propertyCursor, propertyKeyId, value, index.schema().getPropertyIds(), materializedProperties ); read.txState().indexDoUpdateEntry( index.schema(), node.nodeReference(), ValueTuple.of( values ), null ); @@ -161,7 +136,7 @@ void onPropertyRemove( NodeCursor node, PropertyCursor propertyCursor, long[] la } } - void onPropertyChange( NodeCursor node, PropertyCursor propertyCursor, long[] labels, int propertyKeyId, + void onPropertyChange( NodeCursor node, PropertyCursor propertyCursor, long[] labels, int propertyKeyId, int[] existingPropertyKeyIds, Value beforeValue, Value afterValue ) { assert noSchemaChangedInTx(); @@ -169,8 +144,8 @@ void onPropertyChange( NodeCursor node, PropertyCursor propertyCursor, long[] la if ( !indexes.isEmpty() ) { MutableIntObjectMap materializedProperties = IntObjectMaps.mutable.empty(); - NodeSchemaMatcher.onMatchingSchema( indexes.iterator(), node, propertyCursor, propertyKeyId, - ( index, propertyKeyIds ) -> + NodeSchemaMatcher.onMatchingSchema( indexes.iterator(), propertyKeyId, existingPropertyKeyIds, + index -> { int[] propertyIds = index.getPropertyIds(); Value[] valuesAfter = getValueTuple( node, propertyCursor, propertyKeyId, afterValue, propertyIds, materializedProperties ); @@ -229,16 +204,4 @@ private Value[] getValueTuple( NodeCursor node, PropertyCursor propertyCursor, return values; } - - private static boolean nodeHasIndexProperties( IntSet nodeProperties, int[] indexPropertyIds ) - { - for ( int indexPropertyId : indexPropertyIds ) - { - if ( !nodeProperties.contains( indexPropertyId ) ) - { - return false; - } - } - return true; - } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/NodeSchemaMatcher.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/NodeSchemaMatcher.java index 523c17a5800e1..37e00d9adfb62 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/NodeSchemaMatcher.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/NodeSchemaMatcher.java @@ -19,15 +19,10 @@ */ package org.neo4j.kernel.impl.newapi; -import org.eclipse.collections.api.set.primitive.IntSet; -import org.eclipse.collections.api.set.primitive.MutableIntSet; -import org.eclipse.collections.impl.set.mutable.primitive.IntHashSet; - +import java.util.Arrays; import java.util.Iterator; -import org.neo4j.function.ThrowingBiConsumer; -import org.neo4j.internal.kernel.api.NodeCursor; -import org.neo4j.internal.kernel.api.PropertyCursor; +import org.neo4j.function.ThrowingConsumer; import org.neo4j.internal.kernel.api.schema.SchemaDescriptor; import org.neo4j.internal.kernel.api.schema.SchemaDescriptorSupplier; @@ -52,8 +47,6 @@ private NodeSchemaMatcher() * @param the type to match. Must implement SchemaDescriptorSupplier * @param The type of exception that can be thrown when taking the action * @param schemaSuppliers The suppliers to match - * @param node The node cursor - * @param property The property cursor * @param specialPropertyId This property id will always count as a match for the descriptor, regardless of * whether the node has this property or not * @param callback The action to take on match @@ -61,32 +54,19 @@ private NodeSchemaMatcher() */ static void onMatchingSchema( Iterator schemaSuppliers, - NodeCursor node, - PropertyCursor property, int specialPropertyId, - ThrowingBiConsumer callback + int[] existingPropertyIds, + ThrowingConsumer callback ) throws EXCEPTION { - MutableIntSet nodePropertyIds = null; while ( schemaSuppliers.hasNext() ) { SUPPLIER schemaSupplier = schemaSuppliers.next(); SchemaDescriptor schema = schemaSupplier.schema(); - // Get the property key set the first time it's needed - if ( nodePropertyIds == null ) + if ( nodeHasSchemaProperties( existingPropertyIds, schema.getPropertyIds(), specialPropertyId ) ) { - nodePropertyIds = new IntHashSet(); - node.properties( property ); - while ( property.next() ) - { - nodePropertyIds.add( property.propertyKey() ); - } - } - - if ( nodeHasSchemaProperties( nodePropertyIds, schema.getPropertyIds(), specialPropertyId ) ) - { - callback.accept( schemaSupplier, nodePropertyIds ); + callback.accept( schemaSupplier ); } } } @@ -103,8 +83,6 @@ static * @param the type to match. Must implement SchemaDescriptorSupplier * @param The type of exception that can be thrown when taking the action * @param schemaSuppliers The suppliers to match - * @param node The node cursor - * @param property The property cursor * @param specialPropertyId This property id will always count as a match for the descriptor, regardless of * whether the node has this property or not * @param callback The action to take on match @@ -112,14 +90,12 @@ static */ static void onMatchingSchema( Iterator schemaSuppliers, - NodeCursor node, - PropertyCursor property, long[] labels, int specialPropertyId, - ThrowingBiConsumer callback + int[] existingPropertyIds, + ThrowingConsumer callback ) throws EXCEPTION { - MutableIntSet nodePropertyIds = null; while ( schemaSuppliers.hasNext() ) { SUPPLIER schemaSupplier = schemaSuppliers.next(); @@ -130,34 +106,28 @@ static continue; } - // Get the property key set the first time it's needed - if ( nodePropertyIds == null ) - { - nodePropertyIds = new IntHashSet(); - node.properties( property ); - while ( property.next() ) - { - nodePropertyIds.add( property.propertyKey() ); - } - } - - if ( nodeHasSchemaProperties( nodePropertyIds, schema.getPropertyIds(), specialPropertyId ) ) + if ( nodeHasSchemaProperties( existingPropertyIds, schema.getPropertyIds(), specialPropertyId ) ) { - callback.accept( schemaSupplier, nodePropertyIds ); + callback.accept( schemaSupplier ); } } } private static boolean nodeHasSchemaProperties( - IntSet nodeProperties, int[] indexPropertyIds, int changedPropertyId ) + int[] existingPropertyIds, int[] indexPropertyIds, int changedPropertyId ) { for ( int indexPropertyId : indexPropertyIds ) { - if ( indexPropertyId != changedPropertyId && !nodeProperties.contains( indexPropertyId ) ) + if ( indexPropertyId != changedPropertyId && !contains( existingPropertyIds, indexPropertyId ) ) { return false; } } return true; } + + private static boolean contains( int[] existingPropertyIds, int indexPropertyId ) + { + return Arrays.binarySearch( existingPropertyIds, indexPropertyId ) >= 0; + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java index f2b1e549be710..1097666a76c6c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java @@ -23,12 +23,12 @@ import org.apache.commons.lang3.mutable.MutableInt; import java.util.Arrays; +import java.util.Collection; import java.util.Iterator; import java.util.Map; import java.util.Optional; import org.neo4j.graphdb.factory.GraphDatabaseSettings; -import org.neo4j.helpers.collection.CastingIterator; import org.neo4j.internal.kernel.api.CursorFactory; import org.neo4j.internal.kernel.api.ExplicitIndexRead; import org.neo4j.internal.kernel.api.ExplicitIndexWrite; @@ -80,7 +80,7 @@ import org.neo4j.kernel.api.txstate.TransactionState; import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.impl.api.KernelTransactionImplementation; -import org.neo4j.kernel.impl.api.index.IndexingProvidersService; +import org.neo4j.kernel.impl.api.index.IndexingService; import org.neo4j.kernel.impl.api.state.ConstraintIndexCreator; import org.neo4j.kernel.impl.constraints.ConstraintSemantics; import org.neo4j.kernel.impl.index.IndexEntityType; @@ -105,10 +105,10 @@ import static org.neo4j.kernel.impl.locking.ResourceTypes.indexEntryResourceId; import static org.neo4j.kernel.impl.newapi.IndexTxStateUpdater.LabelChangeType.ADDED_LABEL; import static org.neo4j.kernel.impl.newapi.IndexTxStateUpdater.LabelChangeType.REMOVED_LABEL; +import static org.neo4j.storageengine.api.EntityType.NODE; import static org.neo4j.storageengine.api.schema.IndexDescriptor.Type.UNIQUE; import static org.neo4j.values.storable.Values.NO_VALUE; - /** * Collects all Kernel API operations and guards them from being used outside of transaction. * @@ -117,6 +117,8 @@ */ public class Operations implements Write, ExplicitIndexWrite, SchemaWrite { + private static final int[] EMPTY_INT_ARRAY = new int[0]; + private final KernelTransactionImplementation ktx; private final AllStoreHolder allStoreHolder; private final KernelToken token; @@ -126,7 +128,7 @@ public class Operations implements Write, ExplicitIndexWrite, SchemaWrite private final DefaultCursors cursors; private final ConstraintIndexCreator constraintIndexCreator; private final ConstraintSemantics constraintSemantics; - private final IndexingProvidersService indexProviders; + private final IndexingService indexingService; private final Config config; private DefaultNodeCursor nodeCursor; private DefaultPropertyCursor propertyCursor; @@ -134,7 +136,7 @@ public class Operations implements Write, ExplicitIndexWrite, SchemaWrite public Operations( AllStoreHolder allStoreHolder, IndexTxStateUpdater updater, StorageReader statement, KernelTransactionImplementation ktx, KernelToken token, DefaultCursors cursors, AutoIndexing autoIndexing, ConstraintIndexCreator constraintIndexCreator, - ConstraintSemantics constraintSemantics, IndexingProvidersService indexProviders, Config config ) + ConstraintSemantics constraintSemantics, IndexingService indexingService, Config config ) { this.token = token; this.autoIndexing = autoIndexing; @@ -145,7 +147,7 @@ public Operations( AllStoreHolder allStoreHolder, IndexTxStateUpdater updater, S this.cursors = cursors; this.constraintIndexCreator = constraintIndexCreator; this.constraintSemantics = constraintSemantics; - this.indexProviders = indexProviders; + this.indexingService = indexingService; this.config = config; } @@ -275,28 +277,57 @@ public boolean nodeAddLabel( long node, int nodeLabel ) private void checkConstraintsAndAddLabelToNode( long node, int nodeLabel ) throws UniquePropertyValueValidationException, UnableToValidateConstraintException { + // Load the property key id list for this node. We may need it for constraint validation if there are any related constraints, + // but regardless we need it for tx state updating + int[] existingPropertyKeyIds = loadSortedPropertyKeyList(); + //Check so that we are not breaking uniqueness constraints //We do this by checking if there is an existing node in the index that //with the same label and property combination. - Iterator constraints = allStoreHolder.constraintsGetForLabel( nodeLabel ); - while ( constraints.hasNext() ) + if ( existingPropertyKeyIds.length > 0 ) { - ConstraintDescriptor constraint = constraints.next(); - if ( constraint.enforcesUniqueness() ) + for ( IndexBackedConstraintDescriptor uniquenessConstraint : indexingService.getRelatedUniquenessConstraints( new long[]{nodeLabel}, + existingPropertyKeyIds, NODE ) ) { - IndexBackedConstraintDescriptor uniqueConstraint = (IndexBackedConstraintDescriptor) constraint; - IndexQuery.ExactPredicate[] propertyValues = getAllPropertyValues( uniqueConstraint.schema(), + IndexQuery.ExactPredicate[] propertyValues = getAllPropertyValues( uniquenessConstraint.schema(), StatementConstants.NO_SUCH_PROPERTY_KEY, Values.NO_VALUE ); if ( propertyValues != null ) { - validateNoExistingNodeWithExactValues( uniqueConstraint, propertyValues, node ); + validateNoExistingNodeWithExactValues( uniquenessConstraint, propertyValues, node ); } } } //node is there and doesn't already have the label, let's add ktx.txState().nodeDoAddLabel( nodeLabel, node ); - updater.onLabelChange( nodeLabel, nodeCursor, propertyCursor, ADDED_LABEL ); + updater.onLabelChange( nodeLabel, existingPropertyKeyIds, nodeCursor, propertyCursor, ADDED_LABEL ); + } + + private int[] loadSortedPropertyKeyList() + { + nodeCursor.properties( propertyCursor ); + if ( !propertyCursor.next() ) + { + return EMPTY_INT_ARRAY; + } + + int[] propertyKeyIds = new int[4]; // just some arbitrary starting point, it grows on demand + int cursor = 0; + do + { + if ( cursor == propertyKeyIds.length ) + { + propertyKeyIds = Arrays.copyOf( propertyKeyIds, cursor * 2 ); + } + propertyKeyIds[cursor++] = propertyCursor.propertyKey(); + } + while ( propertyCursor.next() ); + if ( cursor != propertyKeyIds.length ) + { + propertyKeyIds = Arrays.copyOf( propertyKeyIds, cursor ); + } + Arrays.sort( propertyKeyIds ); + return propertyKeyIds; } private boolean nodeDelete( long node, boolean lock ) throws AutoIndexingKernelException @@ -390,7 +421,7 @@ private void singleNode( long node ) throws EntityNotFoundException allStoreHolder.singleNode( node, nodeCursor ); if ( !nodeCursor.next() ) { - throw new EntityNotFoundException( EntityType.NODE, node ); + throw new EntityNotFoundException( NODE, node ); } } @@ -510,7 +541,10 @@ public boolean nodeRemoveLabel( long node, int labelId ) throws EntityNotFoundEx sharedSchemaLock( ResourceTypes.LABEL, labelId ); ktx.txState().nodeDoRemoveLabel( labelId, node ); - updater.onLabelChange( labelId, nodeCursor, propertyCursor, REMOVED_LABEL ); + if ( indexingService.hasRelatedSchema( labelId, NODE ) ) + { + updater.onLabelChange( labelId, loadSortedPropertyKeyList(), nodeCursor, propertyCursor, REMOVED_LABEL ); + } return true; } @@ -524,27 +558,23 @@ public Value nodeSetProperty( long node, int propertyKey, Value value ) singleNode( node ); long[] labels = acquireSharedNodeLabelLocks(); Value existingValue = readNodeProperty( propertyKey ); + int[] existingPropertyKeyIds = null; + boolean hasRelatedSchema = indexingService.hasRelatedSchema( labels, propertyKey, NODE ); + if ( hasRelatedSchema ) + { + existingPropertyKeyIds = loadSortedPropertyKeyList(); + } + if ( !existingValue.equals( value ) ) { - // The value changed so let's check uniqueness constraints. - Iterator constraints = allStoreHolder.constraintsGetForProperty( propertyKey ); - Iterator uniquenessConstraints = - new CastingIterator<>( constraints, IndexBackedConstraintDescriptor.class ); - NodeSchemaMatcher.onMatchingSchema( uniquenessConstraints, nodeCursor, propertyCursor, labels, propertyKey, - ( constraint, propertyIds ) -> + // The value changed and there may be relevant constraints to check so let's check those now. + Collection uniquenessConstraints = indexingService.getRelatedUniquenessConstraints( labels, propertyKey, NODE ); + NodeSchemaMatcher.onMatchingSchema( uniquenessConstraints.iterator(), propertyKey, existingPropertyKeyIds, + uniquenessConstraint -> { - if ( propertyIds.contains( propertyKey ) ) - { - Value previousValue = readNodeProperty( propertyKey ); - if ( value.equals( previousValue ) ) - { - // since we are changing to the same value, there is no need to check - return; - } - } - validateNoExistingNodeWithExactValues( constraint, - getAllPropertyValues( constraint.schema(), propertyKey, value ), node ); - } ); + validateNoExistingNodeWithExactValues( uniquenessConstraint, getAllPropertyValues( uniquenessConstraint.schema(), propertyKey, value ), + node ); + }); } if ( existingValue == NO_VALUE ) @@ -552,7 +582,10 @@ public Value nodeSetProperty( long node, int propertyKey, Value value ) //no existing value, we just add it autoIndexing.nodes().propertyAdded( this, node, propertyKey, value ); ktx.txState().nodeDoAddProperty( node, propertyKey, value ); - updater.onPropertyAdd( nodeCursor, propertyCursor, labels, propertyKey, value ); + if ( hasRelatedSchema ) + { + updater.onPropertyAdd( nodeCursor, propertyCursor, labels, propertyKey, existingPropertyKeyIds, value ); + } return NO_VALUE; } else @@ -563,7 +596,10 @@ public Value nodeSetProperty( long node, int propertyKey, Value value ) { //the value has changed to a new value ktx.txState().nodeDoChangeProperty( node, propertyKey, value ); - updater.onPropertyChange( nodeCursor, propertyCursor, labels, propertyKey, existingValue, value ); + if ( hasRelatedSchema ) + { + updater.onPropertyChange( nodeCursor, propertyCursor, labels, propertyKey, existingPropertyKeyIds, existingValue, value ); + } } return existingValue; } @@ -583,7 +619,10 @@ public Value nodeRemoveProperty( long node, int propertyKey ) long[] labels = acquireSharedNodeLabelLocks(); autoIndexing.nodes().propertyRemoved( this, node, propertyKey ); ktx.txState().nodeDoRemoveProperty( node, propertyKey ); - updater.onPropertyRemove( nodeCursor, propertyCursor, labels, propertyKey, existingValue ); + if ( indexingService.hasRelatedSchema( labels, propertyKey, NODE ) ) + { + updater.onPropertyRemove( nodeCursor, propertyCursor, labels, propertyKey, loadSortedPropertyKeyList(), existingValue ); + } } return existingValue; @@ -939,9 +978,9 @@ public IndexReference indexCreate( SchemaDescriptor descriptor, assertValidDescriptor( descriptor, SchemaKernelException.OperationContext.INDEX_CREATION ); assertIndexDoesNotExist( SchemaKernelException.OperationContext.INDEX_CREATION, descriptor, name ); - IndexProviderDescriptor providerDescriptor = indexProviders.indexProviderByName( provider ); + IndexProviderDescriptor providerDescriptor = indexingService.indexProviderByName( provider ); IndexDescriptor index = IndexDescriptorFactory.forSchema( descriptor, name, providerDescriptor ); - index = indexProviders.getBlessedDescriptorFromProvider( index ); + index = indexingService.getBlessedDescriptorFromProvider( index ); ktx.txState().indexDoAdd( index ); return index; } @@ -949,12 +988,12 @@ public IndexReference indexCreate( SchemaDescriptor descriptor, // Note: this will be sneakily executed by an internal transaction, so no additional locking is required. public IndexDescriptor indexUniqueCreate( SchemaDescriptor schema, String provider ) throws SchemaKernelException { - IndexProviderDescriptor providerDescriptor = indexProviders.indexProviderByName( provider ); + IndexProviderDescriptor providerDescriptor = indexingService.indexProviderByName( provider ); IndexDescriptor index = IndexDescriptorFactory.uniqueForSchema( schema, Optional.empty(), providerDescriptor ); - index = indexProviders.getBlessedDescriptorFromProvider( index ); + index = indexingService.getBlessedDescriptorFromProvider( index ); ktx.txState().indexDoAdd( index ); return index; } @@ -1226,7 +1265,7 @@ private void assertNodeExists( long sourceNode ) throws EntityNotFoundException { if ( !allStoreHolder.nodeExists( sourceNode ) ) { - throw new EntityNotFoundException( EntityType.NODE, sourceNode ); + throw new EntityNotFoundException( NODE, sourceNode ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java index 0fc34d061068f..be522371a2283 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java @@ -201,7 +201,7 @@ public RecordStorageEngine( this.indexProviderMap = indexProviderMap; indexingService = IndexingServiceFactory.createIndexingService( config, scheduler, indexProviderMap, indexStoreView, tokenNameLookup, - Iterators.asList( schemaStorage.indexesGetAll() ), logProvider, userLogProvider, + Iterators.asList( schemaStorage.loadAllSchemaRules() ), logProvider, userLogProvider, indexingServiceMonitor, schemaState ); integrityValidator = new IntegrityValidator( neoStores, indexingService ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplier.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplier.java index 69822af40159c..4f86280db0d58 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplier.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplier.java @@ -37,11 +37,13 @@ import org.neo4j.kernel.impl.store.NodeStore; import org.neo4j.kernel.impl.store.PropertyStore; import org.neo4j.kernel.impl.store.RelationshipStore; +import org.neo4j.kernel.impl.store.record.ConstraintRule; import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.kernel.impl.transaction.command.Command.PropertyCommand; import org.neo4j.kernel.impl.transaction.state.IndexUpdates; import org.neo4j.kernel.impl.transaction.state.OnlineIndexUpdates; import org.neo4j.storageengine.api.CommandsToApply; +import org.neo4j.storageengine.api.schema.SchemaRule; import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; import org.neo4j.util.concurrent.AsyncApply; import org.neo4j.util.concurrent.WorkSync; @@ -215,8 +217,10 @@ public boolean visitPropertyCommand( PropertyCommand command ) @Override public boolean visitSchemaRuleCommand( Command.SchemaRuleCommand command ) throws IOException { + SchemaRule schemaRule = command.getSchemaRule(); if ( command.getSchemaRule() instanceof StoreIndexDescriptor ) { + StoreIndexDescriptor indexRule = (StoreIndexDescriptor) schemaRule; // Why apply index updates here? Here's the thing... this is a batch applier, which means that // index updates are gathered throughout the batch and applied in the end of the batch. // Assume there are some transactions creating or modifying nodes that may not be covered @@ -231,22 +235,39 @@ public boolean visitSchemaRuleCommand( Command.SchemaRuleCommand command ) throw case UPDATE: // Shouldn't we be more clear about that we are waiting for an index to come online here? // right now we just assume that an update to index records means wait for it to be online. - if ( ((StoreIndexDescriptor) command.getSchemaRule()).canSupportUniqueConstraint() ) + if ( indexRule.canSupportUniqueConstraint() ) { // Register activations into the IndexActivator instead of IndexingService to avoid deadlock // that could insue for applying batches of transactions where a previous transaction in the same // batch acquires a low-level commit lock that prevents the very same index population to complete. - indexActivator.activateIndex( command.getSchemaRule().getId() ); + indexActivator.activateIndex( schemaRule.getId() ); } break; case CREATE: // Add to list so that all these indexes will be created in one call later createdIndexes = createdIndexes == null ? new ArrayList<>() : createdIndexes; - createdIndexes.add( (StoreIndexDescriptor) command.getSchemaRule() ); + createdIndexes.add( indexRule ); break; case DELETE: - indexingService.dropIndex( (StoreIndexDescriptor) command.getSchemaRule() ); - indexActivator.indexDropped( command.getSchemaRule().getId() ); + indexingService.dropIndex( indexRule ); + indexActivator.indexDropped( schemaRule.getId() ); + break; + default: + throw new IllegalStateException( command.getMode().name() ); + } + } + // Keep IndexingService updated on constraint changes + else if ( schemaRule instanceof ConstraintRule ) + { + ConstraintRule constraintRule = (ConstraintRule) schemaRule; + switch ( command.getMode() ) + { + case CREATE: + case UPDATE: + indexingService.putConstraint( constraintRule ); + break; + case DELETE: + indexingService.removeConstraint( constraintRule.getId() ); break; default: throw new IllegalStateException( command.getMode().name() ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexMapTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexMapTest.java index ce4bfbb03f6f2..31c36d95ed803 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexMapTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexMapTest.java @@ -19,8 +19,6 @@ */ package org.neo4j.kernel.impl.api.index; -import org.eclipse.collections.api.map.primitive.MutableLongObjectMap; -import org.eclipse.collections.impl.map.mutable.primitive.LongObjectHashMap; import org.junit.Before; import org.junit.Test; @@ -51,13 +49,12 @@ public class IndexMapTest @Before public void setup() { - MutableLongObjectMap map = new LongObjectHashMap<>(); - map.put( 1L, new TestIndexProxy( forSchema( schema3_4 ).withId( 1 ).withoutCapabilities() ) ); - map.put( 2L, new TestIndexProxy( forSchema( schema5_6_7 ).withId( 2 ).withoutCapabilities() ) ); - map.put( 3L, new TestIndexProxy( forSchema( schema5_8 ).withId( 3 ).withoutCapabilities() ) ); - map.put( 4L, new TestIndexProxy( forSchema( node35_8 ).withId( 4 ).withoutCapabilities() ) ); - map.put( 5L, new TestIndexProxy( forSchema( rel35_8 ).withId( 5 ).withoutCapabilities() ) ); - indexMap = new IndexMap( map ); + indexMap = new IndexMap(); + indexMap.putIndexProxy( new TestIndexProxy( forSchema( schema3_4 ).withId( 1 ).withoutCapabilities() ) ); + indexMap.putIndexProxy( new TestIndexProxy( forSchema( schema5_6_7 ).withId( 2 ).withoutCapabilities() ) ); + indexMap.putIndexProxy( new TestIndexProxy( forSchema( schema5_8 ).withId( 3 ).withoutCapabilities() ) ); + indexMap.putIndexProxy( new TestIndexProxy( forSchema( node35_8 ).withId( 4 ).withoutCapabilities() ) ); + indexMap.putIndexProxy( new TestIndexProxy( forSchema( rel35_8 ).withId( 5 ).withoutCapabilities() ) ); } @Test diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java index 071c9a2bdaabc..c361a124ce5ad 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java @@ -96,6 +96,7 @@ import org.neo4j.storageengine.api.schema.IndexReader; import org.neo4j.storageengine.api.schema.IndexSample; import org.neo4j.storageengine.api.schema.PopulationProgress; +import org.neo4j.storageengine.api.schema.SchemaRule; import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; import org.neo4j.test.Barrier; import org.neo4j.test.DoubleLatch; @@ -1145,7 +1146,7 @@ public void shouldLogIndexStateOutliersOnInit() throws Exception IndexProviderMap providerMap = life.add( new DefaultIndexProviderMap( buildIndexDependencies( provider ), config ) ); TokenNameLookup mockLookup = mock( TokenNameLookup.class ); - List indexes = new ArrayList<>(); + List indexes = new ArrayList<>(); int nextIndexId = 1; StoreIndexDescriptor populatingIndex = storeIndex( nextIndexId, nextIndexId++, 1, PROVIDER_DESCRIPTOR ); when( provider.getInitialState( populatingIndex ) ).thenReturn( POPULATING ); @@ -1192,7 +1193,7 @@ public void shouldLogIndexStateOutliersOnStart() throws Exception providerMap.init(); TokenNameLookup mockLookup = mock( TokenNameLookup.class ); - List indexes = new ArrayList<>(); + List indexes = new ArrayList<>(); int nextIndexId = 1; StoreIndexDescriptor populatingIndex = storeIndex( nextIndexId, nextIndexId++, 1, PROVIDER_DESCRIPTOR ); when( provider.getInitialState( populatingIndex ) ).thenReturn( POPULATING ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/LabelPropertyMultiSetTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/LabelPropertyMultiSetTest.java index 83a8cab6bc106..535503d7b4026 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/LabelPropertyMultiSetTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/LabelPropertyMultiSetTest.java @@ -50,7 +50,7 @@ public class LabelPropertyMultiSetTest public void shouldLookupSingleKeyDescriptors() { // given - LabelPropertyMultiSet set = new LabelPropertyMultiSet(); + LabelPropertyMultiSet set = new LabelPropertyMultiSet<>(); LabelSchemaDescriptor expected = SchemaDescriptorFactory.forLabel( 1, 2 ); set.add( expected ); @@ -66,7 +66,7 @@ public void shouldLookupSingleKeyDescriptors() public void shouldLookupSingleKeyAndSharedCompositeKeyDescriptors() { // given - LabelPropertyMultiSet set = new LabelPropertyMultiSet(); + LabelPropertyMultiSet set = new LabelPropertyMultiSet<>(); LabelSchemaDescriptor expected1 = SchemaDescriptorFactory.forLabel( 1, 2 ); LabelSchemaDescriptor expected2 = SchemaDescriptorFactory.forLabel( 1, 2, 3 ); set.add( expected1 ); @@ -84,7 +84,7 @@ public void shouldLookupSingleKeyAndSharedCompositeKeyDescriptors() public void shouldLookupCompositeKeyDescriptor() { // given - LabelPropertyMultiSet set = new LabelPropertyMultiSet(); + LabelPropertyMultiSet set = new LabelPropertyMultiSet<>(); LabelSchemaDescriptor descriptor1 = SchemaDescriptorFactory.forLabel( 1, 2, 3 ); LabelSchemaDescriptor descriptor2 = SchemaDescriptorFactory.forLabel( 1, 2, 4 ); LabelSchemaDescriptor descriptor3 = SchemaDescriptorFactory.forLabel( 1, 2, 5, 6 ); @@ -104,7 +104,7 @@ public void shouldLookupCompositeKeyDescriptor() public void shouldLookupAllByLabel() { // given - LabelPropertyMultiSet set = new LabelPropertyMultiSet(); + LabelPropertyMultiSet set = new LabelPropertyMultiSet<>(); LabelSchemaDescriptor descriptor1 = SchemaDescriptorFactory.forLabel( 1, 2, 3 ); LabelSchemaDescriptor descriptor2 = SchemaDescriptorFactory.forLabel( 1, 2, 4 ); LabelSchemaDescriptor descriptor3 = SchemaDescriptorFactory.forLabel( 1, 2, 5, 6 ); @@ -140,7 +140,7 @@ private void shouldAddRemoveAndLookupRandomDescriptors( boolean includeIdempoten { // given List all = new ArrayList<>(); - LabelPropertyMultiSet set = new LabelPropertyMultiSet(); + LabelPropertyMultiSet set = new LabelPropertyMultiSet<>(); int highLabelId = 10; int highPropertyKeyId = 10; int maxNumberOfPropertyKeys = 3; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/IndexTxStateUpdaterTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/IndexTxStateUpdaterTest.java index a7585a3506e33..1bd05a4e7066d 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/IndexTxStateUpdaterTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/IndexTxStateUpdaterTest.java @@ -36,7 +36,6 @@ import org.neo4j.kernel.api.txstate.TransactionState; import org.neo4j.kernel.impl.api.index.IndexProxy; import org.neo4j.kernel.impl.api.index.IndexingService; -import org.neo4j.storageengine.api.StorageReader; import org.neo4j.storageengine.api.schema.IndexDescriptor; import org.neo4j.values.storable.Value; import org.neo4j.values.storable.ValueTuple; @@ -53,9 +52,6 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.neo4j.helpers.collection.Iterators.filter; -import static org.neo4j.internal.kernel.api.schema.SchemaDescriptorPredicates.hasLabel; -import static org.neo4j.internal.kernel.api.schema.SchemaDescriptorPredicates.hasProperty; import static org.neo4j.kernel.impl.newapi.IndexTxStateUpdater.LabelChangeType.ADDED_LABEL; import static org.neo4j.kernel.impl.newapi.IndexTxStateUpdater.LabelChangeType.REMOVED_LABEL; @@ -69,6 +65,7 @@ public class IndexTxStateUpdaterTest private static final int propId3 = 22; private static final int newPropId = 23; private static final int unIndexedPropId = 24; + private static final int[] props = new int[]{propId1, propId2, propId3}; private TransactionState txState; private IndexTxStateUpdater indexTxUpdater; @@ -89,22 +86,6 @@ public void setup() throws IndexNotFoundKernelException { txState = mock( TransactionState.class ); - StorageReader storageReader = mock( StorageReader.class ); - when( storageReader.indexesGetAll() ).thenAnswer( x -> indexes.iterator() ); - when( storageReader.indexesGetForLabel( anyInt() ) ) - .thenAnswer( x -> - { - Integer argument = x.getArgument( 0 ); - return filter( hasLabel( argument ), indexes.iterator() ); - } ); - - when( storageReader.indexesGetRelatedToProperty( anyInt() ) ) - .thenAnswer( x -> - { - Integer argument = x.getArgument( 0 ); - return filter( hasProperty( argument ), indexes.iterator() ); - } ); - HashMap map = new HashMap<>(); map.put( propId1, Values.of( "hi1" ) ); map.put( propId2, Values.of( "hi2" ) ); @@ -134,8 +115,30 @@ public void setup() throws IndexNotFoundKernelException } return descriptors; } ); + when( indexingService.getRelatedIndexes( any(), any( int[].class ), any() ) ).thenAnswer( invocationOnMock -> + { + long[] labels = invocationOnMock.getArgument( 0 ); + int[] propertyKeyIds = invocationOnMock.getArgument( 1 ); + Set descriptors = new HashSet<>(); + for ( IndexDescriptor index : indexes ) + { + if ( contains( labels, index.schema().keyId() ) ) + { + boolean containsAll = true; + for ( int propertyId : index.schema().getPropertyIds() ) + { + containsAll &= contains( propertyKeyIds, propertyId ); + } + if ( containsAll ) + { + descriptors.add( index.schema() ); + } + } + } + return descriptors; + } ); - indexTxUpdater = new IndexTxStateUpdater( storageReader, readOps, indexingService ); + indexTxUpdater = new IndexTxStateUpdater( readOps, indexingService ); } // LABELS @@ -144,8 +147,8 @@ public void setup() throws IndexNotFoundKernelException public void shouldNotUpdateIndexesOnChangedIrrelevantLabel() { // WHEN - indexTxUpdater.onLabelChange( unIndexedLabelId, node, propertyCursor, ADDED_LABEL ); - indexTxUpdater.onLabelChange( unIndexedLabelId, node, propertyCursor, REMOVED_LABEL ); + indexTxUpdater.onLabelChange( unIndexedLabelId, props, node, propertyCursor, ADDED_LABEL ); + indexTxUpdater.onLabelChange( unIndexedLabelId, props, node, propertyCursor, REMOVED_LABEL ); // THEN verify( txState, never() ).indexDoUpdateEntry( any(), anyInt(), any(), any() ); @@ -155,7 +158,7 @@ public void shouldNotUpdateIndexesOnChangedIrrelevantLabel() public void shouldUpdateIndexesOnAddedLabel() { // WHEN - indexTxUpdater.onLabelChange( labelId1, node, propertyCursor, ADDED_LABEL ); + indexTxUpdater.onLabelChange( labelId1, props, node, propertyCursor, ADDED_LABEL ); // THEN verifyIndexUpdate( indexOn1_1.schema(), node.nodeReference(), null, values( "hi1" ) ); @@ -167,7 +170,7 @@ public void shouldUpdateIndexesOnAddedLabel() public void shouldUpdateIndexesOnRemovedLabel() { // WHEN - indexTxUpdater.onLabelChange( labelId2, node, propertyCursor, REMOVED_LABEL ); + indexTxUpdater.onLabelChange( labelId2, props, node, propertyCursor, REMOVED_LABEL ); // THEN verifyIndexUpdate( uniqueOn2_2_3.schema(), node.nodeReference(), values( "hi2", "hi3" ), null ); @@ -178,9 +181,9 @@ public void shouldUpdateIndexesOnRemovedLabel() public void shouldNotUpdateIndexesOnChangedIrrelevantProperty() { // WHEN - indexTxUpdater.onPropertyAdd( node, propertyCursor, node.labels().all(), unIndexedPropId, Values.of( "whAt" ) ); - indexTxUpdater.onPropertyRemove( node, propertyCursor, node.labels().all(), unIndexedPropId, Values.of( "whAt" ) ); - indexTxUpdater.onPropertyChange( node, propertyCursor, node.labels().all(), unIndexedPropId, Values.of( "whAt" ), Values.of( "whAt2" ) ); + indexTxUpdater.onPropertyAdd( node, propertyCursor, node.labels().all(), unIndexedPropId, props, Values.of( "whAt" ) ); + indexTxUpdater.onPropertyRemove( node, propertyCursor, node.labels().all(), unIndexedPropId, props, Values.of( "whAt" ) ); + indexTxUpdater.onPropertyChange( node, propertyCursor, node.labels().all(), unIndexedPropId, props, Values.of( "whAt" ), Values.of( "whAt2" ) ); // THEN verify( txState, never() ).indexDoUpdateEntry( any(), anyInt(), any(), any() ); @@ -190,7 +193,7 @@ public void shouldNotUpdateIndexesOnChangedIrrelevantProperty() public void shouldUpdateIndexesOnAddedProperty() { // WHEN - indexTxUpdater.onPropertyAdd( node, propertyCursor, node.labels().all(), newPropId, Values.of( "newHi" ) ); + indexTxUpdater.onPropertyAdd( node, propertyCursor, node.labels().all(), newPropId, props, Values.of( "newHi" ) ); // THEN verifyIndexUpdate( indexOn2_new.schema(), node.nodeReference(), null, values( "newHi" ) ); @@ -202,7 +205,7 @@ public void shouldUpdateIndexesOnAddedProperty() public void shouldUpdateIndexesOnRemovedProperty() { // WHEN - indexTxUpdater.onPropertyRemove( node, propertyCursor, node.labels().all(), propId2, Values.of( "hi2" ) ); + indexTxUpdater.onPropertyRemove( node, propertyCursor, node.labels().all(), propId2, props, Values.of( "hi2" ) ); // THEN verifyIndexUpdate( uniqueOn1_2.schema(), node.nodeReference(), values( "hi2" ), null ); @@ -214,7 +217,7 @@ public void shouldUpdateIndexesOnRemovedProperty() public void shouldUpdateIndexesOnChangesProperty() { // WHEN - indexTxUpdater.onPropertyChange( node, propertyCursor, node.labels().all(), propId2, Values.of( "hi2" ), Values.of( "new2" ) ); + indexTxUpdater.onPropertyChange( node, propertyCursor, node.labels().all(), propId2, props, Values.of( "hi2" ), Values.of( "new2" ) ); // THEN verifyIndexUpdate( uniqueOn1_2.schema(), node.nodeReference(), values( "hi2" ), values( "new2" ) ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/NodeSchemaMatcherTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/NodeSchemaMatcherTest.java index 44a0ba92dc9f6..63e530d4e9fc4 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/NodeSchemaMatcherTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/NodeSchemaMatcherTest.java @@ -28,7 +28,6 @@ import java.util.List; import org.neo4j.internal.kernel.api.helpers.StubNodeCursor; -import org.neo4j.internal.kernel.api.helpers.StubPropertyCursor; import org.neo4j.storageengine.api.schema.IndexDescriptor; import org.neo4j.values.storable.Value; @@ -50,6 +49,7 @@ public class NodeSchemaMatcherTest private static final int unIndexedPropId = 22; private static final int nonExistentPropId = 23; private static final int specialPropId = 24; + private static final int[] props = new int[]{propId1, propId2, unIndexedPropId}; IndexDescriptor index1 = forLabel( labelId1, propId1 ); IndexDescriptor index1_2 = forLabel( labelId1, propId1, propId2 ); @@ -75,8 +75,7 @@ public void shouldMatchOnSingleProperty() { // when List matched = new ArrayList<>(); - NodeSchemaMatcher.onMatchingSchema( iterator( index1 ), node, - new StubPropertyCursor(), unIndexedPropId, ( schema, props ) -> matched.add( schema ) ); + NodeSchemaMatcher.onMatchingSchema( iterator( index1 ), unIndexedPropId, props, matched::add ); // then assertThat( matched, contains( index1 ) ); @@ -87,8 +86,7 @@ public void shouldMatchOnTwoProperties() { // when List matched = new ArrayList<>(); - NodeSchemaMatcher.onMatchingSchema( iterator( index1_2 ), node, new StubPropertyCursor(), - unIndexedPropId, ( schema, props ) -> matched.add( schema ) ); + NodeSchemaMatcher.onMatchingSchema( iterator( index1_2 ), unIndexedPropId, props, matched::add ); // then assertThat( matched, contains( index1_2 ) ); @@ -99,8 +97,7 @@ public void shouldNotMatchIfNodeIsMissingProperty() { // when List matched = new ArrayList<>(); - NodeSchemaMatcher.onMatchingSchema( iterator( indexWithMissingProperty ), node, new StubPropertyCursor(), - unIndexedPropId, ( schema, props ) -> matched.add( schema ) ); + NodeSchemaMatcher.onMatchingSchema( iterator( indexWithMissingProperty ), unIndexedPropId, props, matched::add ); // then assertThat( matched, empty() ); @@ -111,8 +108,7 @@ public void shouldNotMatchIfNodeIsMissingLabel() { // when List matched = new ArrayList<>(); - NodeSchemaMatcher.onMatchingSchema( iterator( indexWithMissingLabel ), node, new StubPropertyCursor(), - node.labels().all(), unIndexedPropId, ( schema, props ) -> matched.add( schema ) ); + NodeSchemaMatcher.onMatchingSchema( iterator( indexWithMissingLabel ), node.labels().all(), unIndexedPropId, props, matched::add ); // then assertThat( matched, empty() ); @@ -123,8 +119,7 @@ public void shouldMatchOnSpecialProperty() { // when List matched = new ArrayList<>(); - NodeSchemaMatcher.onMatchingSchema( iterator( indexOnSpecialProperty ), node, new StubPropertyCursor(), - specialPropId, ( schema, props ) -> matched.add( schema ) ); + NodeSchemaMatcher.onMatchingSchema( iterator( indexOnSpecialProperty ), specialPropId, props, matched::add ); // then assertThat( matched, contains( indexOnSpecialProperty ) ); @@ -138,9 +133,7 @@ public void shouldMatchSeveralTimes() // when final List matched = new ArrayList<>(); - NodeSchemaMatcher.onMatchingSchema( - indexes.iterator(), node, new StubPropertyCursor(), unIndexedPropId, - ( schema, props ) -> matched.add( schema ) ); + NodeSchemaMatcher.onMatchingSchema( indexes.iterator(), unIndexedPropId, props, matched::add ); // then assertThat( matched, equalTo( indexes ) ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/OperationsLockTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/OperationsLockTest.java index e8e9d4da47d9d..6d40413bcb78b 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/OperationsLockTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/OperationsLockTest.java @@ -52,7 +52,7 @@ import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.impl.api.KernelTransactionImplementation; import org.neo4j.kernel.impl.api.SchemaState; -import org.neo4j.kernel.impl.api.index.IndexingProvidersService; +import org.neo4j.kernel.impl.api.index.IndexingService; import org.neo4j.kernel.impl.api.state.ConstraintIndexCreator; import org.neo4j.kernel.impl.api.state.TxState; import org.neo4j.kernel.impl.constraints.ConstraintSemantics; @@ -140,7 +140,7 @@ public void setUp() throws InvalidTransactionTypeKernelException constraintIndexCreator = mock( ConstraintIndexCreator.class ); operations = new Operations( allStoreHolder, mock( IndexTxStateUpdater.class ),storageReader, transaction, new KernelToken( storageReader, transaction, mockedTokenHolders() ), cursors, autoindexing, - constraintIndexCreator, mock( ConstraintSemantics.class ), mock( IndexingProvidersService.class ), Config.defaults() ); + constraintIndexCreator, mock( ConstraintSemantics.class ), mock( IndexingService.class ), Config.defaults() ); operations.initialize(); this.order = inOrder( locks, txState, storageReader ); @@ -308,19 +308,17 @@ public void shouldAcquireEntityWriteLockBeforeSettingPropertyOnNode() throws Exc public void shouldAcquireSchemaReadLockBeforeSettingPropertyOnNode() throws Exception { // given - when( nodeCursor.next() ).thenReturn( true ); - when( nodeCursor.labels() ).thenReturn( LabelSet.NONE ); int relatedLabelId = 50; int unrelatedLabelId = 51; int propertyKeyId = 8; - int unrelatedPropertyKeyId = 88; + when( nodeCursor.next() ).thenReturn( true ); + LabelSet labelSet = mock( LabelSet.class ); + when( labelSet.all() ).thenReturn( new long[]{relatedLabelId} ); + when( nodeCursor.labels() ).thenReturn( labelSet ); Value value = Values.of( 9 ); when( propertyCursor.next() ).thenReturn( true ); when( propertyCursor.propertyKey() ).thenReturn( propertyKeyId ); when( propertyCursor.propertyValue() ).thenReturn( NO_VALUE ); - when( storageReader.constraintsGetAll() ).thenReturn( - Iterators.iterator( ConstraintDescriptorFactory.uniqueForLabel( relatedLabelId, propertyKeyId ), - ConstraintDescriptorFactory.uniqueForLabel( unrelatedLabelId, unrelatedPropertyKeyId )) ); // when operations.nodeSetProperty( 123, propertyKeyId, value );