Load entity data on write after having acquired an exclusive lock

The problem was that multiple threads modifying the same entity could corrupt the data. As an example consider the following case with 2 threads: - both threads load the same node record from disk (having no properties) - first thread grabs the lock first and add a property "a" with value 42 - second thread waiting on the lock - first thread release the lock - second thread add a property "a" with vale 33 to the same node since the second thread thinks there are no properties on the node it adds a new property instead modifying the one already there - there are duplicated properties in the property chain Similar problems happened for writing properties on relationships and adding same labels on nodes.
neo4j · Aug 4, 2015 · 26c903c · 26c903c
1 parent 3b57dd2
commit 26c903c
Show file tree

Hide file tree

Showing 16 changed files with 812 additions and 610 deletions.
diff --git a/...y/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java b/...y/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java
@@ -36,7 +36,6 @@
 import org.neo4j.kernel.api.constraints.UniquenessConstraint;
 import org.neo4j.kernel.api.cursor.LabelItem;
 import org.neo4j.kernel.api.cursor.NodeItem;
-import org.neo4j.kernel.api.cursor.PropertyItem;
 import org.neo4j.kernel.api.cursor.RelationshipItem;
 import org.neo4j.kernel.api.exceptions.EntityNotFoundException;
 import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException;
@@ -90,48 +89,59 @@ public ConstraintEnforcingEntityOperations(
     }
 
     @Override
-    public boolean nodeAddLabel( KernelStatement state, NodeItem node, int labelId )
+    public boolean nodeAddLabel( KernelStatement state, long nodeId, int labelId )
-            throws ConstraintValidationKernelException
+            throws ConstraintValidationKernelException, EntityNotFoundException
     {
         Iterator<NodePropertyConstraint> allConstraints = schemaReadOperations.constraintsGetForLabel( state, labelId );
         Iterator<NodePropertyConstraint> constraints = uniquePropertyConstraints( allConstraints );
         while ( constraints.hasNext() )
         {
             PropertyConstraint constraint = constraints.next();
             int propertyKeyId = constraint.propertyKey();
-            Object propertyValue = node.getProperty( propertyKeyId );
+            try ( Cursor<NodeItem> cursor = nodeCursorById( state, nodeId ) )
-            if ( propertyValue != null )
             {
-                validateNoExistingNodeWithLabelAndProperty( state, labelId, propertyKeyId, propertyValue, node.id() );
+                NodeItem node = cursor.get();
+                Object propertyValue = node.getProperty( propertyKeyId );
+                if ( propertyValue != null )
+                {
+                    validateNoExistingNodeWithLabelAndProperty( state, labelId, propertyKeyId, propertyValue, node.id() );
+                }
             }
 
         }
-        return entityWriteOperations.nodeAddLabel( state, node, labelId );
+        return entityWriteOperations.nodeAddLabel( state, nodeId, labelId );
     }
 
     @Override
-    public Property nodeSetProperty( KernelStatement state, NodeItem node, DefinedProperty property )
+    public Property nodeSetProperty( KernelStatement state, long nodeId, DefinedProperty property )
-            throws ConstraintValidationKernelException
+            throws ConstraintValidationKernelException, EntityNotFoundException
     {
-        try ( Cursor<LabelItem> labels = node.labels() )
+        try ( Cursor<NodeItem> cursor = nodeCursorById( state, nodeId ) )
         {
-            while ( labels.next() )
+
+            NodeItem node = cursor.get();
+
+            try ( Cursor<LabelItem> labels = node.labels() )
             {
-                int labelId = labels.get().getAsInt();
+                while ( labels.next() )
-                int propertyKeyId = property.propertyKeyId();
-                Iterator<NodePropertyConstraint> constraintIterator =
-                        uniquePropertyConstraints(
-                                schemaReadOperations.constraintsGetForLabelAndPropertyKey( state, labelId,
-                                        propertyKeyId ) );
-                if ( constraintIterator.hasNext() )
                 {
-                    validateNoExistingNodeWithLabelAndProperty( state, labelId, property.propertyKeyId(),
+                    int labelId = labels.get().getAsInt();
-                            property.value(), node.id() );
+                    int propertyKeyId = property.propertyKeyId();
+                    Iterator<NodePropertyConstraint> constraintIterator =
+                            uniquePropertyConstraints(
+                                    schemaReadOperations.constraintsGetForLabelAndPropertyKey( state, labelId,
+                                            propertyKeyId ) );
+                    if ( constraintIterator.hasNext() )
+                    {
+                        validateNoExistingNodeWithLabelAndProperty(
+                                state, labelId, property.propertyKeyId(), property.value(), node.id() );
+                    }
                 }
             }
+
         }
 
-        return entityWriteOperations.nodeSetProperty( state, node, property );
+        return entityWriteOperations.nodeSetProperty( state, nodeId, property );
     }
 
     private void validateNoExistingNodeWithLabelAndProperty( KernelStatement state, int labelId,
@@ -177,39 +187,39 @@ private Iterator<NodePropertyConstraint> uniquePropertyConstraints( Iterator<Nod
     // Simply delegate the rest of the invocations
 
     @Override
-    public void nodeDelete( KernelStatement state, NodeItem node )
+    public void nodeDelete( KernelStatement state, long nodeId ) throws EntityNotFoundException
     {
-        entityWriteOperations.nodeDelete( state, node );
+        entityWriteOperations.nodeDelete( state, nodeId );
     }
 
     @Override
     public long relationshipCreate( KernelStatement statement,
             int relationshipTypeId,
-            NodeItem startNodeId,
+            long startNodeId,
-            NodeItem endNodeId )
+            long endNodeId )
             throws EntityNotFoundException
     {
         return entityWriteOperations.relationshipCreate( statement, relationshipTypeId, startNodeId, endNodeId );
     }
 
     @Override
-    public void relationshipDelete( KernelStatement state, RelationshipItem relationship )
+    public void relationshipDelete( KernelStatement state, long relationshipId ) throws EntityNotFoundException
     {
-        entityWriteOperations.relationshipDelete( state, relationship );
+        entityWriteOperations.relationshipDelete( state, relationshipId );
     }
 
     @Override
-    public boolean nodeRemoveLabel( KernelStatement state, NodeItem node, int labelId )
+    public boolean nodeRemoveLabel( KernelStatement state, long nodeId, int labelId ) throws EntityNotFoundException
     {
-        return entityWriteOperations.nodeRemoveLabel( state, node, labelId );
+        return entityWriteOperations.nodeRemoveLabel( state, nodeId, labelId );
     }
 
     @Override
     public Property relationshipSetProperty( KernelStatement state,
-            RelationshipItem relationship,
+            long relationshipId,
-            DefinedProperty property )
+            DefinedProperty property ) throws EntityNotFoundException
     {
-        return entityWriteOperations.relationshipSetProperty( state, relationship, property );
+        return entityWriteOperations.relationshipSetProperty( state, relationshipId, property );
     }
 
     @Override
@@ -219,17 +229,18 @@ public Property graphSetProperty( KernelStatement state, DefinedProperty propert
     }
 
     @Override
-    public Property nodeRemoveProperty( KernelStatement state, NodeItem node, int propertyKeyId )
+    public Property nodeRemoveProperty( KernelStatement state, long nodeId, int propertyKeyId )
+            throws EntityNotFoundException
     {
-        return entityWriteOperations.nodeRemoveProperty( state, node, propertyKeyId );
+        return entityWriteOperations.nodeRemoveProperty( state, nodeId, propertyKeyId );
     }
 
     @Override
     public Property relationshipRemoveProperty( KernelStatement state,
-            RelationshipItem relationship,
+            long relationshipId,
-            int propertyKeyId )
+            int propertyKeyId ) throws EntityNotFoundException
     {
-        return entityWriteOperations.relationshipRemoveProperty( state, relationship, propertyKeyId );
+        return entityWriteOperations.relationshipRemoveProperty( state, relationshipId, propertyKeyId );
     }
 
     @Override
@@ -373,6 +384,12 @@ public <EXCEPTION extends Exception> void relationshipVisit( KernelStatement sta
         entityReadOperations.relationshipVisit( statement, relId, visitor );
     }
 
+    @Override
+    public Cursor<NodeItem> nodeCursorById( KernelStatement statement, long nodeId ) throws EntityNotFoundException
+    {
+        return entityReadOperations.nodeCursorById( statement, nodeId );
+    }
+
     @Override
     public Cursor<NodeItem> nodeCursor( KernelStatement statement, long nodeId )
     {
@@ -385,6 +402,12 @@ public Cursor<NodeItem> nodeCursor( TxStateHolder txStateHolder, StoreStatement
         return entityReadOperations.nodeCursor( txStateHolder, statement, nodeId );
     }
 
+    @Override
+    public Cursor<RelationshipItem> relationshipCursorById( KernelStatement statement, long relId ) throws EntityNotFoundException
+    {
+        return entityReadOperations.relationshipCursorById( statement, relId );
+    }
+
     @Override
     public Cursor<RelationshipItem> relationshipCursor( KernelStatement statement, long relId )
     {

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/GuardingStatementOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/GuardingStatementOperations.java
@@ -59,8 +59,8 @@ public GuardingStatementOperations(
     @Override
     public long relationshipCreate( KernelStatement statement,
             int relationshipTypeId,
-            NodeItem startNodeId,
+            long startNodeId,
-            NodeItem endNodeId )
+            long endNodeId )
             throws EntityNotFoundException
     {
         guard.check();
@@ -75,49 +75,49 @@ public long nodeCreate( KernelStatement statement )
     }
 
     @Override
-    public void nodeDelete( KernelStatement state, NodeItem node )
+    public void nodeDelete( KernelStatement state, long nodeId ) throws EntityNotFoundException
     {
         guard.check();
-        entityWriteDelegate.nodeDelete( state, node );
+        entityWriteDelegate.nodeDelete( state, nodeId );
     }
 
     @Override
-    public void relationshipDelete( KernelStatement state, RelationshipItem relationship )
+    public void relationshipDelete( KernelStatement state, long relationshipId ) throws EntityNotFoundException
     {
         guard.check();
-        entityWriteDelegate.relationshipDelete( state, relationship );
+        entityWriteDelegate.relationshipDelete( state, relationshipId );
     }
 
     @Override
-    public boolean nodeAddLabel( KernelStatement state, NodeItem node, int labelId )
+    public boolean nodeAddLabel( KernelStatement state, long nodeId, int labelId )
-            throws ConstraintValidationKernelException
+            throws ConstraintValidationKernelException, EntityNotFoundException
     {
         guard.check();
-        return entityWriteDelegate.nodeAddLabel( state, node, labelId );
+        return entityWriteDelegate.nodeAddLabel( state, nodeId, labelId );
     }
 
     @Override
-    public boolean nodeRemoveLabel( KernelStatement state, NodeItem node, int labelId )
+    public boolean nodeRemoveLabel( KernelStatement state, long nodeId, int labelId ) throws EntityNotFoundException
     {
         guard.check();
-        return entityWriteDelegate.nodeRemoveLabel( state, node, labelId );
+        return entityWriteDelegate.nodeRemoveLabel( state, nodeId, labelId );
     }
 
     @Override
-    public Property nodeSetProperty( KernelStatement state, NodeItem node, DefinedProperty property )
+    public Property nodeSetProperty( KernelStatement state, long nodeId, DefinedProperty property )
-            throws ConstraintValidationKernelException
+            throws ConstraintValidationKernelException, EntityNotFoundException
     {
         guard.check();
-        return entityWriteDelegate.nodeSetProperty( state, node, property );
+        return entityWriteDelegate.nodeSetProperty( state, nodeId, property );
     }
 
     @Override
     public Property relationshipSetProperty( KernelStatement state,
-            RelationshipItem relationship,
+            long relationshipId,
-            DefinedProperty property )
+            DefinedProperty property ) throws EntityNotFoundException
     {
         guard.check();
-        return entityWriteDelegate.relationshipSetProperty( state, relationship, property );
+        return entityWriteDelegate.relationshipSetProperty( state, relationshipId, property );
     }
 
     @Override
@@ -128,19 +128,20 @@ public Property graphSetProperty( KernelStatement state, DefinedProperty propert
     }
 
     @Override
-    public Property nodeRemoveProperty( KernelStatement state, NodeItem node, int propertyKeyId )
+    public Property nodeRemoveProperty( KernelStatement state, long nodeId, int propertyKeyId )
+            throws EntityNotFoundException
     {
         guard.check();
-        return entityWriteDelegate.nodeRemoveProperty( state, node, propertyKeyId );
+        return entityWriteDelegate.nodeRemoveProperty( state, nodeId, propertyKeyId );
     }
 
     @Override
     public Property relationshipRemoveProperty( KernelStatement state,
-            RelationshipItem relationship,
+            long relationshipId,
-            int propertyKeyId )
+            int propertyKeyId ) throws EntityNotFoundException
     {
         guard.check();
-        return entityWriteDelegate.relationshipRemoveProperty( state, relationship, propertyKeyId );
+        return entityWriteDelegate.relationshipRemoveProperty( state, relationshipId, propertyKeyId );
     }
 
     @Override
@@ -259,6 +260,13 @@ public <EXCEPTION extends Exception> void relationshipVisit( KernelStatement sta
         entityReadDelegate.relationshipVisit( statement, relId, visitor );
     }
 
+    @Override
+    public Cursor<NodeItem> nodeCursorById( KernelStatement statement, long nodeId ) throws EntityNotFoundException
+    {
+        guard.check();
+        return entityReadDelegate.nodeCursorById( statement, nodeId );
+    }
+
     @Override
     public Cursor<NodeItem> nodeCursor( KernelStatement statement, long nodeId )
     {
@@ -273,6 +281,14 @@ public Cursor<NodeItem> nodeCursor( TxStateHolder txStateHolder, StoreStatement
         return entityReadDelegate.nodeCursor( txStateHolder, statement, nodeId );
     }
 
+    @Override
+    public Cursor<RelationshipItem> relationshipCursorById( KernelStatement statement, long relId )
+            throws EntityNotFoundException
+    {
+        guard.check();
+        return entityReadDelegate.relationshipCursorById( statement, relId );
+    }
+
     @Override
     public Cursor<RelationshipItem> relationshipCursor( KernelStatement statement, long relId )
     {