Don't take locks for entities created in the same transaction.

There is no need to take locks for new entities that have not yet been
committed because they cannot yet be visible to any other transaction.

There should have been no contention for these locks, so this change
should not have a significant impact, except where the cost of acquiring
any lock is high, for example when acquiring locks for a slave in an HA
cluster, where each lock acquisition requires a network call.

community/kernel/src/main/java/org/neo4j/kernel/impl/api/LockingStatementOperations.java

@@ -109,7 +109,7 @@ public boolean nodeAddLabel( KernelStatement state, long nodeId, int labelId )
         // by ConstraintEnforcingEntityOperations included the full cake, with locking included.
         acquireSharedSchemaLock( state );
 
-        state.locks().optimistic().acquireExclusive( ResourceTypes.NODE, nodeId );
+        acquireExclusiveNodeLock( state, nodeId );
         state.assertOpen();
 
         return entityWriteDelegate.nodeAddLabel( state, nodeId, labelId );
@@ -118,7 +118,7 @@ public boolean nodeAddLabel( KernelStatement state, long nodeId, int labelId )
     @Override
     public boolean nodeRemoveLabel( KernelStatement state, long nodeId, int labelId ) throws EntityNotFoundException
     {
-        state.locks().optimistic().acquireExclusive( ResourceTypes.NODE, nodeId );
+        acquireExclusiveNodeLock( state, nodeId );
         state.assertOpen();
         return entityWriteDelegate.nodeRemoveLabel( state, nodeId, labelId );
     }
@@ -258,7 +258,7 @@ public Iterator<IndexDescriptor> uniqueIndexesGetAll( KernelStatement state )
     @Override
     public void nodeDelete( KernelStatement state, long nodeId ) throws EntityNotFoundException
     {
-        state.locks().optimistic().acquireExclusive( ResourceTypes.NODE, nodeId );
+        acquireExclusiveNodeLock( state, nodeId );
         state.assertOpen();
         entityWriteDelegate.nodeDelete( state, nodeId );
     }
@@ -319,18 +319,18 @@ public void visit(long relId, int type, long startNode, long endNode) {
                 lockRelationshipNodes(state, startNode, endNode);
             }
         });
-        state.locks().optimistic().acquireExclusive(ResourceTypes.RELATIONSHIP, relationshipId);
+        acquireExclusiveRelationshipLock( state, relationshipId );
         state.assertOpen();
         entityWriteDelegate.relationshipDelete(state, relationshipId);
     }
 
     private void lockRelationshipNodes( KernelStatement state, long startNodeId, long endNodeId )
     {
         // Order the locks to lower the risk of deadlocks with other threads creating/deleting rels concurrently
-        state.locks().optimistic().acquireExclusive( ResourceTypes.NODE, min( startNodeId, endNodeId ) );
+        acquireExclusiveNodeLock( state, min( startNodeId, endNodeId ) );
         if ( startNodeId != endNodeId )
         {
-            state.locks().optimistic().acquireExclusive( ResourceTypes.NODE, max( startNodeId, endNodeId ) );
+            acquireExclusiveNodeLock( state, max( startNodeId, endNodeId ) );
         }
     }
 
@@ -473,7 +473,7 @@ public Property nodeSetProperty( KernelStatement state, long nodeId, DefinedProp
         // by ConstraintEnforcingEntityOperations included the full cake, with locking included.
         acquireSharedSchemaLock( state );
 
-        state.locks().optimistic().acquireExclusive( ResourceTypes.NODE, nodeId );
+        acquireExclusiveNodeLock( state, nodeId );
         state.assertOpen();
         return entityWriteDelegate.nodeSetProperty( state, nodeId, property );
     }
@@ -482,7 +482,7 @@ public Property nodeSetProperty( KernelStatement state, long nodeId, DefinedProp
     public Property nodeRemoveProperty( KernelStatement state, long nodeId, int propertyKeyId )
             throws EntityNotFoundException
     {
-        state.locks().optimistic().acquireExclusive( ResourceTypes.NODE, nodeId );
+        acquireExclusiveNodeLock( state, nodeId );
         state.assertOpen();
         return entityWriteDelegate.nodeRemoveProperty( state, nodeId, propertyKeyId );
     }
@@ -492,7 +492,7 @@ public Property relationshipSetProperty( KernelStatement state,
             long relationshipId,
             DefinedProperty property ) throws EntityNotFoundException
     {
-        state.locks().optimistic().acquireExclusive( ResourceTypes.RELATIONSHIP, relationshipId );
+        acquireExclusiveRelationshipLock( state, relationshipId );
         state.assertOpen();
         return entityWriteDelegate.relationshipSetProperty( state, relationshipId, property );
     }
@@ -502,7 +502,7 @@ public Property relationshipRemoveProperty( KernelStatement state,
             long relationshipId,
             int propertyKeyId ) throws EntityNotFoundException
     {
-        state.locks().optimistic().acquireExclusive( ResourceTypes.RELATIONSHIP, relationshipId );
+        acquireExclusiveRelationshipLock( state, relationshipId );
         state.assertOpen();
         return entityWriteDelegate.relationshipRemoveProperty( state, relationshipId, propertyKeyId );
     }
@@ -559,6 +559,22 @@ public String indexGetFailure( Statement state, IndexDescriptor descriptor )
         return schemaReadDelegate.indexGetFailure( state, descriptor );
     }
 
+    private void acquireExclusiveNodeLock( KernelStatement state, long nodeId )
+    {
+        if ( !state.txState().nodeIsAddedInThisTx( nodeId ) )
+        {
+            state.locks().optimistic().acquireExclusive( ResourceTypes.NODE, nodeId );
+        }
+    }
+
+    private void acquireExclusiveRelationshipLock( KernelStatement state, long relationshipId )
+    {
+        if ( !state.txState().relationshipIsAddedInThisTx( relationshipId ) )
+        {
+            state.locks().optimistic().acquireExclusive( ResourceTypes.RELATIONSHIP, relationshipId );
+        }
+    }
+
     private void acquireSharedSchemaLock( KernelStatement state )
     {
         state.locks().optimistic().acquireShared( ResourceTypes.SCHEMA, schemaResource() );

community/kernel/src/test/java/org/neo4j/kernel/impl/api/LockingStatementOperationsTest.java

@@ -35,11 +35,15 @@
 import org.neo4j.kernel.api.index.IndexDescriptor;
 import org.neo4j.kernel.api.properties.DefinedProperty;
 import org.neo4j.kernel.api.properties.Property;
+import org.neo4j.kernel.api.txstate.LegacyIndexTransactionState;
+import org.neo4j.kernel.api.txstate.TransactionState;
+import org.neo4j.kernel.api.txstate.TxStateHolder;
 import org.neo4j.kernel.impl.api.operations.EntityReadOperations;
 import org.neo4j.kernel.impl.api.operations.EntityWriteOperations;
 import org.neo4j.kernel.impl.api.operations.SchemaReadOperations;
 import org.neo4j.kernel.impl.api.operations.SchemaStateOperations;
 import org.neo4j.kernel.impl.api.operations.SchemaWriteOperations;
+import org.neo4j.kernel.impl.api.state.TxState;
 import org.neo4j.kernel.impl.locking.Locks;
 import org.neo4j.kernel.impl.locking.ResourceTypes;
 import org.neo4j.kernel.impl.locking.SimpleStatementLocks;
@@ -50,6 +54,7 @@
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.when;
 import static org.neo4j.function.Functions.constant;
@@ -65,8 +70,9 @@ public class LockingStatementOperationsTest
     private final Locks.Client locks = mock( Locks.Client.class );
     private final InOrder order;
     private final KernelTransactionImplementation transaction = mock( KernelTransactionImplementation.class );
-    private final KernelStatement state = new KernelStatement( transaction, null, null, null,
-            new SimpleStatementLocks( locks ), null, null );
+    private final TxState txState = new TxState();
+    private final KernelStatement state = new KernelStatement( transaction, null, null,
+            new SimpleTxStateHolder( txState ), new SimpleStatementLocks( locks ), null, null );
     private final SchemaStateOperations schemaStateOps;
 
     public LockingStatementOperationsTest()
@@ -83,25 +89,25 @@ public LockingStatementOperationsTest()
     }
 
     @Test
-    public void shouldAcquireEntityWriteLockCreatingRelationship() throws Exception
+    public void shouldAcquireEntityWriteLockBeforeAddingLabelToNode() throws Exception
     {
         // when
-        lockingOps.relationshipCreate( state, 1, 2, 3 );
+        lockingOps.nodeAddLabel( state, 123, 456 );
 
         // then
-        order.verify( locks ).acquireExclusive( ResourceTypes.NODE, 2 );
-        order.verify( locks ).acquireExclusive( ResourceTypes.NODE, 3 );
-        order.verify( entityWriteOps ).relationshipCreate( state, 1, 2, 3 );
+        order.verify( locks ).acquireExclusive( ResourceTypes.NODE, 123 );
+        order.verify( entityWriteOps ).nodeAddLabel( state, 123, 456 );
     }
 
     @Test
-    public void shouldAcquireEntityWriteLockBeforeAddingLabelToNode() throws Exception
+    public void shouldNotAcquireEntityWriteLockBeforeAddingLabelToJustCreatedNode() throws Exception
     {
         // when
+        txState.nodeDoCreate( 123 );
         lockingOps.nodeAddLabel( state, 123, 456 );
 
         // then
-        order.verify( locks ).acquireExclusive( ResourceTypes.NODE, 123 );
+        order.verify( locks, never() ).acquireExclusive( ResourceTypes.NODE, 123 );
         order.verify( entityWriteOps ).nodeAddLabel( state, 123, 456 );
     }
 
@@ -130,6 +136,21 @@ public void shouldAcquireEntityWriteLockBeforeSettingPropertyOnNode() throws Exc
         order.verify( entityWriteOps ).nodeSetProperty( state, 123, property );
     }
 
+    @Test
+    public void shouldNotAcquireEntityWriteLockBeforeSettingPropertyOnJustCreatedNode() throws Exception
+    {
+        // given
+        txState.nodeDoCreate( 123 );
+        DefinedProperty property = Property.property( 8, 9 );
+
+        // when
+        lockingOps.nodeSetProperty( state, 123, property );
+
+        // then
+        order.verify( locks, never() ).acquireExclusive( ResourceTypes.NODE, 123 );
+        order.verify( entityWriteOps ).nodeSetProperty( state, 123, property );
+    }
+
     @Test
     public void shouldAcquireSchemaReadLockBeforeSettingPropertyOnNode() throws Exception
     {
@@ -155,6 +176,18 @@ public void shouldAcquireEntityWriteLockBeforeDeletingNode() throws EntityNotFou
         order.verify( entityWriteOps ).nodeDelete( state, 123 );
     }
 
+    @Test
+    public void shouldNotAcquireEntityWriteLockBeforeDeletingJustCreatedNode() throws EntityNotFoundException
+    {
+        // WHEN
+        txState.nodeDoCreate( 123 );
+        lockingOps.nodeDelete( state, 123 );
+
+        //THEN
+        order.verify( locks, never() ).acquireExclusive( ResourceTypes.NODE, 123 );
+        order.verify( entityWriteOps ).nodeDelete( state, 123 );
+    }
+
     @Test
     public void shouldAcquireSchemaWriteLockBeforeAddingIndexRule() throws Exception
     {
@@ -315,6 +348,18 @@ public void shouldAcquireSchemaReadLockBeforeFlushingSchemaState() throws Except
         order.verify( schemaStateOps ).schemaStateFlush( state );
     }
 
+    @Test
+    public void shouldAcquireEntityWriteLockCreatingRelationship() throws Exception
+    {
+        // when
+        lockingOps.relationshipCreate( state, 1, 2, 3 );
+
+        // then
+        order.verify( locks ).acquireExclusive( ResourceTypes.NODE, 2 );
+        order.verify( locks ).acquireExclusive( ResourceTypes.NODE, 3 );
+        order.verify( entityWriteOps ).relationshipCreate( state, 1, 2, 3 );
+    }
+
     @Test
     public void shouldAcquireNodeLocksWhenCreatingRelationshipInOrderOfAscendingId() throws Exception
     {
@@ -408,4 +453,58 @@ public Void answer( InvocationOnMock invocation ) throws Throwable
             lockingOrder.verifyNoMoreInteractions();
         }
     }
+
+    @Test
+    public void shouldAcquireEntityWriteLockBeforeSettingPropertyOnRelationship() throws Exception
+    {
+        // given
+        DefinedProperty property = Property.property( 8, 9 );
+
+        // when
+        lockingOps.relationshipSetProperty( state, 123, property );
+
+        // then
+        order.verify( locks ).acquireExclusive( ResourceTypes.RELATIONSHIP, 123 );
+        order.verify( entityWriteOps ).relationshipSetProperty( state, 123, property );
+    }
+
+    @Test
+    public void shouldNotAcquireEntityWriteLockBeforeSettingPropertyOnJustCreatedRelationship() throws Exception
+    {
+        // given
+        txState.relationshipDoCreate( 123, 1, 2, 3 );
+        DefinedProperty property = Property.property( 8, 9 );
+
+        // when
+        lockingOps.relationshipSetProperty( state, 123, property );
+
+        // then
+        order.verify( locks, never() ).acquireExclusive( ResourceTypes.RELATIONSHIP, 123 );
+        order.verify( entityWriteOps ).relationshipSetProperty( state, 123, property );
+    }
+
+    private static class SimpleTxStateHolder implements TxStateHolder
+    {
+        private final TxState txState;
+
+        private SimpleTxStateHolder( TxState txState )
+        {
+            this.txState = txState;
+        }
+
+        @Override public TransactionState txState()
+        {
+            return txState;
+        }
+
+        @Override public LegacyIndexTransactionState legacyIndexTxState()
+        {
+            return null;
+        }
+
+        @Override public boolean hasTxStateWithChanges()
+        {
+            return false;
+        }
+    }
 }

community/kernel/src/test/java/org/neo4j/kernel/impl/core/NodeTest.java

@@ -45,8 +45,6 @@
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
-import static org.neo4j.helpers.Exceptions.launderedException;
-
 public class NodeTest
 {
     @ClassRule
@@ -426,61 +424,6 @@ public void testChangeProperty2()
         assertEquals( "test4", node.getProperty( "test" ) );
     }
 
-    @Test
-    public void testNodeLockingProblem() throws InterruptedException
-    {
-        testLockProblem( getGraphDb().createNode() );
-    }
-
-    @Test
-    public void testRelationshipLockingProblem() throws InterruptedException
-    {
-        Node node = getGraphDb().createNode();
-        Node node2 = getGraphDb().createNode();
-        testLockProblem( node.createRelationshipTo( node2,
-                DynamicRelationshipType.withName( "lock-rel" ) ) );
-    }
-
-    private void testLockProblem( final PropertyContainer entity ) throws InterruptedException
-    {
-        entity.setProperty( "key", "value" );
-        final AtomicBoolean gotTheLock = new AtomicBoolean();
-        Thread thread = new Thread()
-        {
-            @Override
-            public void run()
-            {
-                try( Transaction tx = getGraphDb().beginTx() )
-                {
-                    tx.acquireWriteLock( entity );
-                    gotTheLock.set( true );
-                    tx.success();
-                }
-                catch ( Exception e )
-                {
-                    e.printStackTrace();
-                    throw launderedException( e );
-                }
-            }
-        };
-        thread.start();
-        long endTime = System.currentTimeMillis() + 5000;
-        while ( thread.getState() != State.TERMINATED )
-        {
-            if ( thread.getState() == Thread.State.WAITING || thread.getState() == State.TIMED_WAITING)
-            {
-                break;
-            }
-            Thread.sleep( 1 );
-            if ( System.currentTimeMillis() > endTime ) break;
-        }
-        boolean gotLock = gotTheLock.get();
-        tx.success();
-        tx.begin();
-        assertFalse( gotLock );
-        thread.join();
-    }
-
     private GraphDatabaseService getGraphDb()
     {
         return db.getGraphDatabaseService();