From 4ae00f479d69fceb0ca0be71546ca585a7571c59 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Wed, 20 Jun 2018 17:48:03 +0200 Subject: [PATCH] Add locking coordination between unlabelled nodes and "any label" indexes. --- .../org/neo4j/internal/kernel/api/Locks.java | 10 ++++ .../org/neo4j/internal/kernel/api/Write.java | 32 ++++++++++++- .../kernel/api/schema/SchemaDescriptor.java | 10 ++-- .../impl/factory/GraphDatabaseFacade.java | 16 ++----- .../kernel/impl/locking/ResourceTypes.java | 23 ++++++++- .../neo4j/kernel/impl/newapi/Operations.java | 48 ++++++++++++++++++- .../org/neo4j/kernel/impl/newapi/Read.java | 16 ++++++- 7 files changed, 133 insertions(+), 22 deletions(-) diff --git a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Locks.java b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Locks.java index 620009752b96d..925253787a72d 100644 --- a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Locks.java +++ b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Locks.java @@ -67,4 +67,14 @@ public interface Locks * This is useful for when a lock on "all tokens" are needed. */ void acquireExclusiveTokenLock(); + + /** + * Shared unlabelled node locks are taken when nodes are created without any label. + */ + void acquireSharedUnlabelledNodeLock(); + + /** + * Exclusive unlabelled node locks are taken when "any label" indexes are dropped or created. + */ + void acquireExclusiveUnlabelledNodeLock(); } diff --git a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java index 9a4f2c03bcd68..e33f42dbf2f68 100644 --- a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java +++ b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java @@ -32,12 +32,26 @@ public interface Write { /** * Create a node. + * * @return The internal id of the created node */ long nodeCreate(); + /** + * Create a node, and assign it the given array of labels. + *

+ * This method differs from a {@link #nodeCreate()} and {@link #nodeAddLabel(long, int)} sequence, in that we will + * avoid taking the "unlabelled node lock" of the {@code nodeCreate}, and we will avoid taking the exclusive node + * lock in the {@code nodeAddLabel} method. + * + * @param labels The labels to assign to the newly created node. + * @return The internal id of the created node. + */ + long nodeCreateWithLabels( int[] labels ) throws ConstraintValidationException; + /** * Delete a node. + * * @param node the internal id of the node to delete * @return returns true if it deleted a node or false if no node was found for this id */ @@ -45,6 +59,7 @@ public interface Write /** * Deletes the node and all relationships connecting the node + * * @param node the node to delete * @return the number of deleted relationships */ @@ -52,6 +67,7 @@ public interface Write /** * Create a relationship between two nodes. + * * @param sourceNode the source internal node id * @param relationshipType the type of the relationship to create * @param targetNode the target internal node id @@ -61,11 +77,14 @@ public interface Write /** * Delete a relationship + * * @param relationship the internal id of the relationship to delete */ boolean relationshipDelete( long relationship ) throws AutoIndexingKernelException; + /** * Add a label to a node + * * @param node the internal node id * @param nodeLabel the internal id of the label to add * @return {@code true} if a label was added otherwise {@code false} @@ -75,6 +94,7 @@ public interface Write /** * Remove a label from a node + * * @param node the internal node id * @param nodeLabel the internal id of the label to remove * @return {@code true} if node was removed otherwise {@code false} @@ -83,6 +103,7 @@ public interface Write /** * Set a property on a node + * * @param node the internal node id * @param propertyKey the property key id * @param value the value to set @@ -93,6 +114,7 @@ Value nodeSetProperty( long node, int propertyKey, Value value ) /** * Remove a property from a node + * * @param node the internal node id * @param propertyKey the property key id * @return The removed value, or Values.NO_VALUE if the node did not have the property before @@ -101,23 +123,28 @@ Value nodeSetProperty( long node, int propertyKey, Value value ) /** * Set a property on a relationship + * * @param relationship the internal relationship id * @param propertyKey the property key id * @param value the value to set * @return The replaced value, or Values.NO_VALUE if the relationship did not have the property before */ - Value relationshipSetProperty( long relationship, int propertyKey, Value value ) throws EntityNotFoundException, AutoIndexingKernelException; + Value relationshipSetProperty( long relationship, int propertyKey, Value value ) + throws EntityNotFoundException, AutoIndexingKernelException; /** * Remove a property from a relationship + * * @param relationship the internal relationship id * @param propertyKey the property key id * @return The removed value, or Values.NO_VALUE if the relationship did not have the property before */ - Value relationshipRemoveProperty( long relationship, int propertyKey ) throws EntityNotFoundException, AutoIndexingKernelException; + Value relationshipRemoveProperty( long relationship, int propertyKey ) + throws EntityNotFoundException, AutoIndexingKernelException; /** * Set a property on the graph + * * @param propertyKey the property key id * @param value the value to set * @return The replaced value, or Values.NO_VALUE if the graph did not have the property before @@ -126,6 +153,7 @@ Value nodeSetProperty( long node, int propertyKey, Value value ) /** * Remove a property from the graph + * * @param propertyKey the property key id * @return The removed value, or Values.NO_VALUE if the graph did not have the property before */ diff --git a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/schema/SchemaDescriptor.java b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/schema/SchemaDescriptor.java index b715bb3a3c08f..be0f2f4e9e083 100644 --- a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/schema/SchemaDescriptor.java +++ b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/schema/SchemaDescriptor.java @@ -117,11 +117,15 @@ static boolean isAnyEntityTokenSchema( SchemaDescriptor schema ) static long[] schemaTokenLockingIds( SchemaDescriptor schema ) { // TODO make getEntityTokenIds produce a long array directly, and avoid this extra copying. - int[] entityTokenIds = schema.getEntityTokenIds(); - long[] lockingIds = new long[entityTokenIds.length]; + return schemaTokenLockingIds( schema.getEntityTokenIds() ); + } + + static long[] schemaTokenLockingIds( int[] tokenIds ) + { + long[] lockingIds = new long[tokenIds.length]; for ( int i = 0; i < lockingIds.length; i++ ) { - lockingIds[i] = entityTokenIds[i]; + lockingIds[i] = tokenIds[i]; } return lockingIds; } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java index 60257c8e72fff..9c33f83dd284f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java @@ -258,8 +258,6 @@ public Node createNode( Label... labels ) KernelTransaction transaction = statementContext.getKernelTransactionBoundToThisThread( true ); try ( Statement ignore = transaction.acquireStatement() ) { - Write write = transaction.dataWrite(); - long nodeId = write.nodeCreate(); TokenWrite tokenWrite = transaction.tokenWrite(); int[] labelIds = new int[labels.length]; String[] labelNames = new String[labels.length]; @@ -268,17 +266,9 @@ public Node createNode( Label... labels ) labelNames[i] = labels[i].name(); } tokenWrite.labelGetOrCreateForNames( labelNames, labelIds ); - for ( int labelId : labelIds ) - { - try - { - write.nodeAddLabel( nodeId, labelId ); - } - catch ( EntityNotFoundException e ) - { - throw new NotFoundException( "No node with id " + nodeId + " found.", e ); - } - } + + Write write = transaction.dataWrite(); + long nodeId = write.nodeCreateWithLabels( labelIds ); return newNodeProxy( nodeId ); } catch ( ConstraintValidationException e ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/ResourceTypes.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/ResourceTypes.java index 2218c9da206e0..6833b461bbc52 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/ResourceTypes.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/ResourceTypes.java @@ -42,7 +42,13 @@ public enum ResourceTypes implements ResourceType EXPLICIT_INDEX( 5, LockWaitStrategies.INCREMENTAL_BACKOFF ), LABEL( 6, LockWaitStrategies.INCREMENTAL_BACKOFF ), RELATIONSHIP_TYPE( 7, LockWaitStrategies.INCREMENTAL_BACKOFF ), - TOKEN_CREATE( 8, LockWaitStrategies.INCREMENTAL_BACKOFF ); + /** + * Resources ids of the type SPECIAL_SINGLETON each represent a distinct (singleton) resource that is used for a + * special purpose in the database kernel. The individual ids otherwise have nothing in common. + *

+ * Each resource is given by a SINGLETON_* constant in this file. + */ + SPECIAL_SINGLETON( 8, LockWaitStrategies.INCREMENTAL_BACKOFF ); private static final boolean useStrongHashing = FeatureToggles.flag( ResourceTypes.class, "useStrongHashing", false ); @@ -51,6 +57,21 @@ public enum ResourceTypes implements ResourceType private static final HashFunction indexEntryHash_2_2_0 = HashFunction.xorShift32(); private static final HashFunction indexEntryHash_4_x = HashFunction.incrementalXXH64(); + /** + * The SINGLETON_TOKEN_CREATE resource constant is used to coordinate the creation of new tokens, with locks taken + * by "any-token" indexes when they are created or dropped. These indexes need to lock all tokens of a particular + * type. To do this, they need to ensure that no new tokens are created concurrently with their critical section. + * Token creates take a shared lock on this resource, because token creation is internally synchronised and can be + * allowed to happen in parallel. Meanwhile, dropping or creating "any token" indexes will take exclusive locks on + * this resource to prevent new tokens from being allocated. + */ + public static final int SINGLETON_TOKEN_CREATE = 1; + /** + * Nodes can be created without any labels, but would still be indexed by "any-token" node indexes. To coordinate + * schema locking between these two cases, we use this special singleton lock. + */ + public static final int SINGLETON_UNLABELLED_NODE = 2; + static { for ( ResourceTypes resourceTypes : ResourceTypes.values() ) 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 897cea4fb9871..6b9185dd13385 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 @@ -101,6 +101,7 @@ import static org.neo4j.internal.kernel.api.schema.SchemaDescriptor.schemaTokenLockingIds; import static org.neo4j.internal.kernel.api.schema.SchemaDescriptorPredicates.hasProperty; +import static org.neo4j.kernel.api.StatementConstants.NO_SUCH_LABEL; import static org.neo4j.kernel.api.StatementConstants.NO_SUCH_NODE; import static org.neo4j.kernel.api.StatementConstants.NO_SUCH_PROPERTY_KEY; import static org.neo4j.kernel.api.schema.index.IndexDescriptor.Type.UNIQUE; @@ -168,11 +169,46 @@ public void initialize() public long nodeCreate() { ktx.assertOpen(); + sharedSchemaLock( ResourceTypes.SPECIAL_SINGLETON, ResourceTypes.SINGLETON_UNLABELLED_NODE ); long nodeId = statement.reserveNode(); ktx.txState().nodeDoCreate( nodeId ); return nodeId; } + @Override + public long nodeCreateWithLabels( int[] labels ) throws ConstraintValidationException + { + if ( labels == null || labels.length == 0 ) + { + return nodeCreate(); + } + + // We don't need to check the node for existence, like we do in nodeAddLabel, because we just created it. + // We also don't need to check if the node already has some of the labels, because we know it has none. + // And we don't need to take the exclusive lock on the node, because it was created in this transaction and + // isn't visible to anyone else yet. + ktx.assertOpen(); + long[] lockingIds = SchemaDescriptor.schemaTokenLockingIds( labels ); + Arrays.sort( lockingIds ); // Sort to ensure labels are locked and assigned in order. + ktx.statementLocks().optimistic().acquireShared( ktx.lockTracer(), ResourceTypes.LABEL, lockingIds ); + long nodeId = statement.reserveNode(); + ktx.txState().nodeDoCreate( nodeId ); + nodeCursor.single( nodeId, allStoreHolder ); + nodeCursor.next(); + + int prevLabel = NO_SUCH_LABEL; + for ( long lockingId : lockingIds ) + { + int label = (int) lockingId; + if ( label != prevLabel ) // Filter out duplicates. + { + checkConstraintsAndAddLabelToNode( nodeId, label ); + prevLabel = label; + } + } + return nodeId; + } + @Override public boolean nodeDelete( long node ) throws AutoIndexingKernelException { @@ -242,6 +278,13 @@ public boolean nodeAddLabel( long node, int nodeLabel ) return false; } + checkConstraintsAndAddLabelToNode( node, nodeLabel ); + return true; + } + + private void checkConstraintsAndAddLabelToNode( long node, int nodeLabel ) + throws UniquePropertyValueValidationException, UnableToValidateConstraintException + { //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. @@ -264,7 +307,6 @@ public boolean nodeAddLabel( long node, int nodeLabel ) //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 ); - return true; } private boolean nodeDelete( long node, boolean lock ) throws AutoIndexingKernelException @@ -1168,6 +1210,10 @@ private void exclusiveAnyEntityTokenSchema( SchemaDescriptor schema ) // After we get the exclusive token lock, no new tokens can be created. This allows us to grab a lock on all // the existing tokens, and be sure that we won't miss any updates. allStoreHolder.acquireExclusiveTokenLock(); + // We also need to coordinate with the creation of unlabelled nodes, + // since they are indexable by "any token" indexes. + allStoreHolder.acquireExclusiveUnlabelledNodeLock(); + ResourceType resourceType; long[] tokens; Iterator itr; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Read.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Read.java index d124c4dc4ae6d..66ab6932fe478 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Read.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Read.java @@ -586,13 +586,25 @@ public void acquireSharedLabelLock( long... ids ) @Override public void acquireSharedTokenLock() { - acquireSharedLock( ResourceTypes.TOKEN_CREATE, 1 ); + acquireSharedLock( ResourceTypes.SPECIAL_SINGLETON, ResourceTypes.SINGLETON_TOKEN_CREATE ); } @Override public void acquireExclusiveTokenLock() { - acquireSharedLock( ResourceTypes.TOKEN_CREATE, 1 ); + acquireSharedLock( ResourceTypes.SPECIAL_SINGLETON, ResourceTypes.SINGLETON_TOKEN_CREATE ); + } + + @Override + public void acquireSharedUnlabelledNodeLock() + { + acquireSharedLock( ResourceTypes.SPECIAL_SINGLETON, ResourceTypes.SINGLETON_UNLABELLED_NODE ); + } + + @Override + public void acquireExclusiveUnlabelledNodeLock() + { + acquireExclusiveLock( ResourceTypes.SPECIAL_SINGLETON, ResourceTypes.SINGLETON_UNLABELLED_NODE ); } @Override