diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/cursor/NodeItemHelper.java b/community/kernel/src/main/java/org/neo4j/kernel/api/cursor/NodeItemHelper.java index a9ec8110b62f5..726d058e6e314 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/cursor/NodeItemHelper.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/cursor/NodeItemHelper.java @@ -43,12 +43,6 @@ public boolean hasLabel( int labelId ) } } - @Override - public PrimitiveIntIterator getLabels() - { - return Cursors.intIterator( labels(), GET_LABEL ); - } - @Override public RelationshipIterator getRelationships( Direction direction, int[] relTypes ) { @@ -63,12 +57,6 @@ public RelationshipIterator getRelationships( Direction direction ) return new CursorRelationshipIterator( relationships( direction ) ); } - @Override - public PrimitiveIntIterator getRelationshipTypes() - { - return Cursors.intIterator( relationshipTypes(), GET_RELATIONSHIP_TYPE ); - } - private static int[] deduplicate( int[] types ) { int unique = 0; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/txstate/TransactionCountingStateVisitor.java b/community/kernel/src/main/java/org/neo4j/kernel/api/txstate/TransactionCountingStateVisitor.java index b1009ae6a77c6..4147904f221b2 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/txstate/TransactionCountingStateVisitor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/txstate/TransactionCountingStateVisitor.java @@ -20,15 +20,17 @@ package org.neo4j.kernel.api.txstate; import java.util.Set; +import java.util.function.Consumer; +import org.neo4j.collection.primitive.Primitive; import org.neo4j.collection.primitive.PrimitiveIntCollections; -import org.neo4j.collection.primitive.PrimitiveIntIterator; +import org.neo4j.collection.primitive.PrimitiveIntSet; import org.neo4j.cursor.Cursor; import org.neo4j.kernel.api.exceptions.EntityNotFoundException; import org.neo4j.kernel.api.exceptions.schema.ConstraintValidationKernelException; import org.neo4j.kernel.impl.api.CountsRecordState; import org.neo4j.kernel.impl.api.RelationshipDataExtractor; -import org.neo4j.storageengine.api.DegreeItem; +import org.neo4j.storageengine.api.LabelItem; import org.neo4j.storageengine.api.NodeItem; import org.neo4j.storageengine.api.StorageStatement; import org.neo4j.storageengine.api.StoreReadLayer; @@ -46,8 +48,7 @@ public class TransactionCountingStateVisitor extends TxStateVisitor.Delegator private final CountsRecordState counts; private final ReadableTransactionState txState; - public TransactionCountingStateVisitor( TxStateVisitor next, - StoreReadLayer storeLayer, StorageStatement statement, + public TransactionCountingStateVisitor( TxStateVisitor next, StoreReadLayer storeLayer, StorageStatement statement, ReadableTransactionState txState, CountsRecordState counts ) { super( next ); @@ -68,35 +69,27 @@ public void visitCreatedNode( long id ) public void visitDeletedNode( long id ) { counts.incrementNodeCount( ANY_LABEL, -1 ); - try ( Cursor node = statement.acquireSingleNodeCursor( id ) ) + statement.acquireSingleNodeCursor( id ).forAll( node -> { - if ( node.next() ) - { - // TODO Rewrite this to use cursors directly instead of iterator - PrimitiveIntIterator labels = node.get().getLabels(); - if ( labels.hasNext() ) - { - final int[] removed = PrimitiveIntCollections.asArray( labels ); - for ( int label : removed ) + PrimitiveIntSet set = + node.labels().mapReduce( Primitive.intSet(), LabelItem::getAsInt, ( labelId, current ) -> { - counts.incrementNodeCount( label, -1 ); - } + current.add( labelId ); + counts.incrementNodeCount( labelId, -1 ); + return current; + } ); - try ( Cursor degrees = node.get().degrees() ) - { - while ( degrees.next() ) - { - DegreeItem degree = degrees.get(); - for ( int label : removed ) - { - updateRelationshipsCountsFromDegrees( degree.type(), label, -degree.outgoing(), - -degree.incoming() ); - } - } - } + int[] labelIds = PrimitiveIntCollections.asArray( set.iterator() ); + node.degrees().forAll( degree -> + { + for ( int labelId : labelIds ) + { + updateRelationshipsCountsFromDegrees( degree.type(), labelId, -degree.outgoing(), + -degree.incoming() ); } - } - } + } ); + + } ); super.visitDeletedNode( id ); } @@ -118,8 +111,7 @@ public void visitDeletedRelationship( long id ) } catch ( EntityNotFoundException e ) { - throw new IllegalStateException( - "Relationship being deleted should exist along with its nodes.", e ); + throw new IllegalStateException( "Relationship being deleted should exist along with its nodes.", e ); } super.visitDeletedRelationship( id ); } @@ -141,30 +133,18 @@ public void visitNodeLabelChanges( long id, final Set added, final Set< } // get the relationship counts from *before* this transaction, // the relationship changes will compensate for what happens during the transaction - try ( Cursor node = statement.acquireSingleNodeCursor( id ) ) + statement.acquireSingleNodeCursor( id ).forAll( node -> node.degrees().forAll( degree -> { - if ( node.next() ) + for ( Integer label : added ) { - try ( Cursor degrees = node.get().degrees() ) - { - while ( degrees.next() ) - { - DegreeItem degree = degrees.get(); - - for ( Integer label : added ) - { - updateRelationshipsCountsFromDegrees( degree.type(), label, degree.outgoing(), - degree.incoming() ); - } - for ( Integer label : removed ) - { - updateRelationshipsCountsFromDegrees( degree.type(), label, -degree.outgoing(), - -degree.incoming() ); - } - } - } + updateRelationshipsCountsFromDegrees( degree.type(), label, degree.outgoing(), degree.incoming() ); } - } + for ( Integer label : removed ) + { + updateRelationshipsCountsFromDegrees( degree.type(), label, -degree.outgoing(), + -degree.incoming() ); + } + } ) ); } super.visitNodeLabelChanges( id, added, removed ); } @@ -182,31 +162,17 @@ private void updateRelationshipsCountsFromDegrees( int type, int label, long out private void updateRelationshipCount( long startNode, int type, long endNode, int delta ) { updateRelationshipsCountsFromDegrees( type, ANY_LABEL, delta, 0 ); - for ( PrimitiveIntIterator startLabels = labelsOf( startNode ); startLabels.hasNext(); ) - { - updateRelationshipsCountsFromDegrees( type, startLabels.next(), delta, 0 ); - } - for ( PrimitiveIntIterator endLabels = labelsOf( endNode ); endLabels.hasNext(); ) - { - updateRelationshipsCountsFromDegrees( type, endLabels.next(), 0, delta ); - } + visitLabels( startNode, ( label ) -> updateRelationshipsCountsFromDegrees( type, label.getAsInt(), delta, 0 ) ); + visitLabels( endNode, ( label ) -> updateRelationshipsCountsFromDegrees( type, label.getAsInt(), 0, delta ) ); } - private PrimitiveIntIterator labelsOf( long nodeId ) + private void visitLabels( long nodeId, Consumer consumer ) { - try ( Cursor node = nodeCursor( statement, nodeId ) ) - { - if ( node.next() ) - { - return node.get().getLabels(); - } - return PrimitiveIntCollections.emptyIterator(); - } + nodeCursor( statement, nodeId ).forAll( node -> node.labels().forAll( consumer ) ); } private Cursor nodeCursor( StorageStatement statement, long nodeId ) { - Cursor cursor = statement.acquireSingleNodeCursor( nodeId ); - return txState.augmentSingleNodeCursor( cursor, nodeId ); + return txState.augmentSingleNodeCursor( statement.acquireSingleNodeCursor( nodeId ), nodeId ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java index 6a6bcc2cad065..d211934f2c31d 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java @@ -24,6 +24,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.function.IntSupplier; import java.util.stream.Stream; import org.neo4j.collection.RawIterator; @@ -98,7 +99,9 @@ import org.neo4j.kernel.impl.api.store.RelationshipIterator; import org.neo4j.kernel.impl.proc.Procedures; import org.neo4j.kernel.impl.query.QuerySource; +import org.neo4j.kernel.impl.util.Cursors; import org.neo4j.register.Register.DoubleLongRegister; +import org.neo4j.storageengine.api.LabelItem; import org.neo4j.storageengine.api.NodeItem; import org.neo4j.storageengine.api.RelationshipItem; import org.neo4j.storageengine.api.Token; @@ -319,7 +322,7 @@ public PrimitiveIntIterator nodeGetLabels( long nodeId ) throws EntityNotFoundEx statement.assertOpen(); try ( Cursor node = dataRead().nodeCursorById( statement, nodeId ) ) { - return node.get().getLabels(); + return Cursors.intIterator( node.get().labels(), LabelItem::getAsInt ); } } @@ -425,7 +428,7 @@ public PrimitiveIntIterator nodeGetRelationshipTypes( long nodeId ) throws Entit statement.assertOpen(); try ( Cursor node = dataRead().nodeCursorById( statement, nodeId ) ) { - return node.get().getRelationshipTypes(); + return Cursors.intIterator( node.get().relationshipTypes(), IntSupplier::getAsInt ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/TwoPhaseNodeForRelationshipLocking.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/TwoPhaseNodeForRelationshipLocking.java index 3c4f72261c6a0..49a60b12916a8 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/TwoPhaseNodeForRelationshipLocking.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/TwoPhaseNodeForRelationshipLocking.java @@ -26,11 +26,10 @@ import org.neo4j.function.ThrowingConsumer; import org.neo4j.kernel.api.exceptions.KernelException; import org.neo4j.kernel.impl.api.operations.EntityReadOperations; -import org.neo4j.kernel.impl.api.store.RelationshipIterator; -import org.neo4j.kernel.impl.locking.Locks; import org.neo4j.kernel.impl.locking.ResourceTypes; import org.neo4j.storageengine.api.Direction; import org.neo4j.storageengine.api.NodeItem; +import org.neo4j.storageengine.api.RelationshipItem; class TwoPhaseNodeForRelationshipLocking { @@ -38,43 +37,10 @@ class TwoPhaseNodeForRelationshipLocking private final EntityReadOperations entityReadOperations; private final ThrowingConsumer relIdAction; - private boolean retry = true; private long firstRelId; - private boolean first; - private final RelationshipVisitor collectNodeIdVisitor = - (relId, type, startNode, endNode) -> { - if ( firstRelId == -1 ) - { - firstRelId = relId; - } - nodeIds.add( startNode ); - nodeIds.add( endNode ); - }; - - private final RelationshipVisitor relationshipConsumingVisitor = - new RelationshipVisitor() - { - @Override - public void visit( long relId, int type, long startNode, long endNode ) throws KernelException - { - if ( first ) - { - first = false; - if ( relId != firstRelId ) - { - // if the first relationship is not the same someone added some new rels, so we need to - // lock them all again - retry = true; - return; - } - } - - relIdAction.accept( relId ); - } - }; - - TwoPhaseNodeForRelationshipLocking( EntityReadOperations entityReadOperations, ThrowingConsumer relIdAction ) + TwoPhaseNodeForRelationshipLocking( EntityReadOperations entityReadOperations, + ThrowingConsumer relIdAction ) { this.entityReadOperations = entityReadOperations; this.relIdAction = relIdAction; @@ -82,48 +48,82 @@ public void visit( long relId, int type, long startNode, long endNode ) throws K void lockAllNodesAndConsumeRelationships( long nodeId, final KernelStatement state ) throws KernelException { + boolean retry = true; nodeIds.add( nodeId ); while ( retry ) { retry = false; - first = true; firstRelId = -1; // lock all the nodes involved by following the node id ordering - try ( Cursor cursor = entityReadOperations.nodeCursorById( state, nodeId ) ) + try( Cursor node = entityReadOperations.nodeCursorById( state, nodeId ) ) { - RelationshipIterator relationships = cursor.get().getRelationships( Direction.BOTH ); - while ( relationships.hasNext() ) - { - entityReadOperations.relationshipVisit( state, relationships.next(), collectNodeIdVisitor ); - } + node.get().relationships( Direction.BOTH ).forAll( this::collectNodeId ); } - PrimitiveLongIterator nodeIdIterator = nodeIds.iterator(); - while ( nodeIdIterator.hasNext() ) - { - state.locks().optimistic().acquireExclusive( state.lockTracer(), ResourceTypes.NODE, nodeIdIterator.next() ); - } + lockAllNodes( state ); // perform the action on each relationship, we will retry if the the relationship iterator contains new relationships - try ( Cursor cursor = entityReadOperations.nodeCursorById( state, nodeId ) ) + try ( Cursor node = entityReadOperations.nodeCursorById( state, nodeId ) ) { - RelationshipIterator relationships = cursor.get().getRelationships( Direction.BOTH ); - while ( relationships.hasNext() ) + try ( Cursor relationships = node.get().relationships( Direction.BOTH ) ) { - entityReadOperations.relationshipVisit( state, relationships.next(), relationshipConsumingVisitor ); - if ( retry ) + boolean first = true; + while ( relationships.next() && !retry ) { - PrimitiveLongIterator iterator = nodeIds.iterator(); - while ( iterator.hasNext() ) - { - state.locks().optimistic().releaseExclusive( ResourceTypes.NODE, iterator.next() ); - } - nodeIds.clear(); - break; + retry = performAction( state, relationships.get(), first ); + first = false; } } } } } + + private void lockAllNodes( KernelStatement state ) + { + PrimitiveLongIterator nodeIdIterator = nodeIds.iterator(); + while ( nodeIdIterator.hasNext() ) + { + state.locks().optimistic() + .acquireExclusive( state.lockTracer(), ResourceTypes.NODE, nodeIdIterator.next() ); + } + } + + private void unlockAllNodes( KernelStatement state ) + { + PrimitiveLongIterator iterator = nodeIds.iterator(); + while ( iterator.hasNext() ) + { + state.locks().optimistic().releaseExclusive( ResourceTypes.NODE, iterator.next() ); + } + nodeIds.clear(); + } + + private boolean performAction( KernelStatement state, RelationshipItem rel, boolean first ) throws KernelException + { + if ( first ) + { + if ( rel.id() != firstRelId ) + { + // if the first relationship is not the same someone added some new rels, so we need to + // lock them all again + unlockAllNodes( state ); + return true; + } + } + + relIdAction.accept( rel.id() ); + return false; + } + + private void collectNodeId( RelationshipItem rel ) + { + if ( firstRelId == -1 ) + { + firstRelId = rel.id(); + } + + nodeIds.add( rel.startNode() ); + nodeIds.add( rel.endNode() ); + } } diff --git a/community/kernel/src/main/java/org/neo4j/storageengine/api/NodeItem.java b/community/kernel/src/main/java/org/neo4j/storageengine/api/NodeItem.java index 5452629625132..f93e4d670c5e1 100644 --- a/community/kernel/src/main/java/org/neo4j/storageengine/api/NodeItem.java +++ b/community/kernel/src/main/java/org/neo4j/storageengine/api/NodeItem.java @@ -32,11 +32,6 @@ public interface NodeItem extends EntityItem { - /** - * Convenience function for extracting a label id from a {@link LabelItem}. - */ - ToIntFunction GET_LABEL = IntSupplier::getAsInt; - /** * Convenience function for extracting a relationship type id from a {@link IntSupplier}. */ @@ -110,11 +105,6 @@ public interface NodeItem */ boolean hasLabel( int labelId ); - /** - * @return label ids attached to this node. - */ - PrimitiveIntIterator getLabels(); - /** * @param direction {@link Direction} to filter on. * @param typeIds relationship type ids to filter on. @@ -127,9 +117,4 @@ public interface NodeItem * @return relationship ids for the given direction. */ RelationshipIterator getRelationships( Direction direction ); - - /** - * @return relationship type ids for all relationships attached to this node. - */ - PrimitiveIntIterator getRelationshipTypes(); } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/LockingStatementOperationsTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/LockingStatementOperationsTest.java index c136f333c9e84..6dc3472209862 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/LockingStatementOperationsTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/LockingStatementOperationsTest.java @@ -40,14 +40,13 @@ 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.TwoPhaseNodeForRelationshipLockingTest.RelationshipData; 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.StubCursors; import org.neo4j.kernel.impl.api.state.TxState; -import org.neo4j.kernel.impl.api.store.CursorRelationshipIterator; import org.neo4j.kernel.impl.api.store.RelationshipIterator; import org.neo4j.kernel.impl.api.store.StoreSingleNodeCursor; import org.neo4j.kernel.impl.factory.CanWrite; @@ -56,16 +55,13 @@ import org.neo4j.kernel.impl.locking.ResourceTypes; import org.neo4j.kernel.impl.locking.SimpleStatementLocks; import org.neo4j.kernel.impl.proc.Procedures; -import org.neo4j.kernel.impl.util.Cursors; import org.neo4j.storageengine.api.Direction; import org.neo4j.storageengine.api.NodeItem; -import org.neo4j.storageengine.api.RelationshipItem; import org.neo4j.storageengine.api.StorageStatement; import static org.junit.Assert.assertSame; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyLong; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -73,6 +69,7 @@ import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; +import static org.neo4j.kernel.impl.api.TwoPhaseNodeForRelationshipLockingTest.returnRelationships; import static org.neo4j.kernel.impl.locking.ResourceTypes.schemaResource; public class LockingStatementOperationsTest @@ -496,12 +493,7 @@ public void shouldNotAcquireEntityWriteLockBeforeSettingPropertyOnJustCreatedRel public void detachDeleteNodeWithoutRelationshipsExclusivelyLockNode() throws KernelException { long nodeId = 1L; - - NodeItem nodeItem = mock( NodeItem.class ); - when( nodeItem.getRelationships( Direction.BOTH ) ).thenReturn( RelationshipIterator.EMPTY ); - StoreSingleNodeCursor nodeCursor = mock( StoreSingleNodeCursor.class ); - when( nodeCursor.get() ).thenReturn( nodeItem ); - when( entityReadOps.nodeCursorById( state, nodeId ) ).thenReturn( nodeCursor ); + returnRelationships( entityReadOps, state, nodeId, false ); lockingOps.nodeDetachDelete( state, nodeId ); @@ -513,33 +505,17 @@ public void detachDeleteNodeWithoutRelationshipsExclusivelyLockNode() throws Ker @Test public void detachDeleteNodeExclusivelyLockNodes() throws KernelException { - long startNodeId = 1L; - long endNodeId = 2L; - - RelationshipItem relationshipItem = StubCursors.asRelationship( 1L, 0, startNodeId, endNodeId, null ); - CursorRelationshipIterator relationshipIterator = - new CursorRelationshipIterator( Cursors.cursor( relationshipItem ) ); - - NodeItem nodeItem = mock( NodeItem.class ); - when( nodeItem.getRelationships( Direction.BOTH ) ).thenReturn( relationshipIterator ); - StoreSingleNodeCursor nodeCursor = mock( StoreSingleNodeCursor.class ); - when( nodeCursor.get() ).thenReturn( nodeItem ); - when( entityReadOps.nodeCursorById( state, startNodeId ) ).thenReturn( nodeCursor ); - doAnswer( invocation -> - { - RelationshipVisitor visitor = invocation.getArgumentAt( 2, RelationshipVisitor.class ); - visitor.visit( relationshipItem.id(), relationshipItem.type(), relationshipItem.startNode(), - relationshipItem.endNode() ); - return null; - } ).when( entityReadOps ).relationshipVisit( eq(state), anyLong(), any() ); + long nodeId = 1L; + RelationshipData relationship = new RelationshipData( 1, nodeId, 2L ); + returnRelationships( entityReadOps, state, nodeId, false, relationship ); - lockingOps.nodeDetachDelete( state, startNodeId ); + lockingOps.nodeDetachDelete( state, nodeId ); - order.verify( locks ).acquireExclusive( LockTracer.NONE, ResourceTypes.NODE, startNodeId ); - order.verify( locks ).acquireExclusive( LockTracer.NONE, ResourceTypes.NODE, endNodeId ); - order.verify( locks, times( 0 ) ).releaseExclusive( ResourceTypes.NODE, startNodeId ); - order.verify( locks, times( 0 ) ).releaseExclusive( ResourceTypes.NODE, endNodeId ); - order.verify( entityWriteOps ).nodeDetachDelete( state, startNodeId ); + order.verify( locks ).acquireExclusive( LockTracer.NONE, ResourceTypes.NODE, relationship.startNodeId ); + order.verify( locks ).acquireExclusive( LockTracer.NONE, ResourceTypes.NODE, relationship.endNodeId ); + order.verify( locks, times( 0 ) ).releaseExclusive( ResourceTypes.NODE, relationship.startNodeId ); + order.verify( locks, times( 0 ) ).releaseExclusive( ResourceTypes.NODE, relationship.endNodeId ); + order.verify( entityWriteOps ).nodeDetachDelete( state, nodeId ); } private static class SimpleTxStateHolder implements TxStateHolder diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/TwoPhaseNodeForRelationshipLockingTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/TwoPhaseNodeForRelationshipLockingTest.java index 67711cb7f8c4b..140655ca85058 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/TwoPhaseNodeForRelationshipLockingTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/TwoPhaseNodeForRelationshipLockingTest.java @@ -19,34 +19,31 @@ */ package org.neo4j.kernel.impl.api; -import java.util.HashSet; -import java.util.NoSuchElementException; -import java.util.Set; - -import org.apache.commons.lang3.NotImplementedException; import org.junit.Test; import org.mockito.InOrder; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import java.util.HashSet; +import java.util.NoSuchElementException; +import java.util.Set; + import org.neo4j.cursor.Cursor; import org.neo4j.function.ThrowingConsumer; import org.neo4j.kernel.api.exceptions.EntityNotFoundException; import org.neo4j.kernel.api.exceptions.KernelException; import org.neo4j.kernel.impl.api.operations.EntityReadOperations; -import org.neo4j.kernel.impl.api.store.RelationshipIterator; import org.neo4j.kernel.impl.locking.LockTracer; import org.neo4j.kernel.impl.locking.Locks; import org.neo4j.kernel.impl.locking.ResourceTypes; import org.neo4j.kernel.impl.locking.SimpleStatementLocks; import org.neo4j.storageengine.api.Direction; +import org.neo4j.storageengine.api.EntityType; import org.neo4j.storageengine.api.NodeItem; +import org.neo4j.storageengine.api.RelationshipItem; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -73,10 +70,10 @@ public void shouldLockNodesInOrderAndConsumeTheRelationships() throws Throwable Collector collector = new Collector(); TwoPhaseNodeForRelationshipLocking locking = new TwoPhaseNodeForRelationshipLocking( ops, collector ); - returnRelationships( nodeId, false, 21L, 22L, 23L ); - returnNodesForRelationship( 21L, nodeId, 43L ); - returnNodesForRelationship( 22L, 40L, nodeId ); - returnNodesForRelationship( 23L, nodeId, 41L ); + RelationshipData relationship1 = new RelationshipData( 21L, nodeId, 43L ); + RelationshipData relationship2 = new RelationshipData( 22L, 40L, nodeId ); + RelationshipData relationship3 = new RelationshipData( 23L, nodeId, 41L ); + returnRelationships( ops, state, nodeId, false, relationship1, relationship2, relationship3 ); InOrder inOrder = inOrder( locks ); @@ -92,16 +89,17 @@ public void shouldLockNodesInOrderAndConsumeTheRelationships() throws Throwable } @Test - public void shouldLockNodesInOrderAndConsumeTheRelationshipsAndRetryIfTheNewRelationshipsAreCreated() throws Throwable + public void shouldLockNodesInOrderAndConsumeTheRelationshipsAndRetryIfTheNewRelationshipsAreCreated() + throws Throwable { // given Collector collector = new Collector(); TwoPhaseNodeForRelationshipLocking locking = new TwoPhaseNodeForRelationshipLocking( ops, collector ); - returnRelationships( nodeId, true, 21L, 22L, 23L ); - returnNodesForRelationship( 21L, nodeId, 43L ); - returnNodesForRelationship( 22L, 40L, nodeId ); - returnNodesForRelationship( 23L, nodeId, 41L ); + RelationshipData relationship1 = new RelationshipData( 21L, nodeId, 43L ); + RelationshipData relationship2 = new RelationshipData( 22L, 40L, nodeId ); + RelationshipData relationship3 = new RelationshipData( 23L, nodeId, 41L ); + returnRelationships( ops, state, nodeId, true, relationship1, relationship2, relationship3 ); InOrder inOrder = inOrder( locks ); @@ -129,7 +127,7 @@ public void lockNodeWithoutRelationships() throws Exception { Collector collector = new Collector(); TwoPhaseNodeForRelationshipLocking locking = new TwoPhaseNodeForRelationshipLocking( ops, collector ); - returnRelationships( nodeId, false ); + returnRelationships( ops, state, nodeId, false ); locking.lockAllNodesAndConsumeRelationships( nodeId, state ); @@ -137,65 +135,69 @@ public void lockNodeWithoutRelationships() throws Exception verifyNoMoreInteractions( locks ); } - private void returnNodesForRelationship( final long relId, final long startNodeId, final long endNodeId ) - throws Exception + public static class RelationshipData { - doAnswer( new Answer() + public final long relId; + public final long startNodeId; + public final long endNodeId; + + RelationshipData( long relId, long startNodeId, long endNodeId ) { - @Override - public Void answer( InvocationOnMock invocation ) throws Throwable - { - @SuppressWarnings( "unchecked" ) - RelationshipVisitor visitor = - (RelationshipVisitor) invocation.getArguments()[2]; - visitor.visit( relId, 6, startNodeId, endNodeId ); - return null; - } - } ).when( ops ).relationshipVisit( eq( state ), eq( relId ), any( RelationshipVisitor.class ) ); + this.relId = relId; + this.startNodeId = startNodeId; + this.endNodeId = endNodeId; + } + + RelationshipItem asRelationshipItem() + { + RelationshipItem rel = mock( RelationshipItem.class ); + when( rel.id() ).thenReturn( relId ); + when( rel.startNode() ).thenReturn( startNodeId ); + when( rel.endNode() ).thenReturn( endNodeId ); + return rel; + } } - private void returnRelationships( long nodeId, final boolean skipFirst, final long... relIds ) - throws EntityNotFoundException + static void returnRelationships( EntityReadOperations ops, KernelStatement state, long nodeId, + final boolean skipFirst, final RelationshipData... relIds ) throws EntityNotFoundException { - //noinspection unchecked - Cursor cursor = mock( Cursor.class ); - when( ops.nodeCursorById( state, nodeId ) ).thenReturn( cursor ); NodeItem nodeItem = mock( NodeItem.class ); - when( cursor.get() ).thenReturn( nodeItem ); - when( nodeItem.getRelationships( Direction.BOTH ) ).thenAnswer( new Answer() + when( nodeItem.relationships( Direction.BOTH ) ).thenAnswer( new Answer>() { private boolean first = skipFirst; @Override - public RelationshipIterator answer( InvocationOnMock invocation ) throws Throwable + public Cursor answer( InvocationOnMock invocation ) throws Throwable { try { - return new RelationshipIterator() + return new Cursor() { private int i = first ? 1 : 0; + private RelationshipData relationshipData = null; @Override - public boolean relationshipVisit( long relationshipId, - RelationshipVisitor visitor ) + public boolean next() { - throw new NotImplementedException( "don't call this!" ); + boolean next = i < relIds.length; + relationshipData = next ? relIds[i++] : null; + return next; } @Override - public boolean hasNext() + public RelationshipItem get() { - return i < relIds.length; + if ( relationshipData == null ) + { + throw new NoSuchElementException(); + } + + return relationshipData.asRelationshipItem(); } @Override - public long next() + public void close() { - if ( !hasNext() ) - { - throw new NoSuchElementException(); - } - return relIds[i++]; } }; } @@ -204,7 +206,42 @@ public long next() first = false; } } - }); + } ); + + when( ops.nodeCursorById( state, nodeId ) ).thenAnswer( invocationOnMock -> + { + Cursor cursor = new Cursor() + { + private int i = 0; + + @Override + public boolean next() + { + return i++ == 0; + } + + @Override + public NodeItem get() + { + if ( i != 1 ) + { + throw new NoSuchElementException(); + } + return nodeItem; + } + + @Override + public void close() + { + + } + }; + if ( !cursor.next() ) + { + throw new EntityNotFoundException( EntityType.NODE, nodeId ); + } + return cursor; + } ); } private static class Collector implements ThrowingConsumer diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/LabelTransactionStateTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/LabelTransactionStateTest.java index face7d8b6b964..6ee9a19874cc0 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/LabelTransactionStateTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/LabelTransactionStateTest.java @@ -26,11 +26,11 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.function.IntSupplier; -import org.neo4j.collection.primitive.PrimitiveIntCollections; import org.neo4j.collection.primitive.PrimitiveLongCollections; -import org.neo4j.cursor.Cursor; import org.neo4j.kernel.api.exceptions.EntityNotFoundException; import org.neo4j.kernel.api.index.IndexDescriptor; import org.neo4j.kernel.api.txstate.TransactionState; @@ -40,7 +40,6 @@ import org.neo4j.kernel.impl.api.legacyindex.InternalAutoIndexing; import org.neo4j.kernel.impl.api.store.StoreStatement; import org.neo4j.kernel.impl.index.LegacyIndexStore; -import org.neo4j.storageengine.api.NodeItem; import org.neo4j.storageengine.api.StoreReadLayer; import static org.junit.Assert.assertEquals; @@ -333,12 +332,7 @@ private void commitLabels( Labels... labels ) throws Exception for ( int label : nodeLabels.labelIds ) { - Collection nodes = allLabels.get( label ); - if ( nodes == null ) - { - nodes = new ArrayList<>(); - allLabels.put( label, nodes ); - } + Collection nodes = allLabels.computeIfAbsent( label, k -> new ArrayList<>() ); nodes.add( nodeLabels.nodeId ); } } @@ -362,23 +356,21 @@ private void commitLabels( Integer... labels ) throws Exception private void assertLabels( Integer... labels ) throws EntityNotFoundException { - try ( Cursor cursor = txContext.nodeCursorById( state, nodeId ) ) - { - if ( cursor.next() ) - { - assertEquals( asSet( labels ), PrimitiveIntCollections.toSet( cursor.get().getLabels() ) ); - } - } + txContext.nodeCursorById( state, nodeId ).forAll( node -> assertEquals( asSet( labels ), + node.labels().mapReduce( new HashSet<>(), IntSupplier::getAsInt, this::addToCollection ) ) ); - for ( int label : labels ) + txContext.nodeCursorById( state, nodeId ).forAll( node -> { - try ( Cursor cursor = txContext.nodeCursorById( state, nodeId ) ) + for ( int label : labels ) { - if ( cursor.next() ) - { - assertTrue( "Expected labels not found on node", cursor.get().hasLabel( label ) ); - } + assertTrue( "Expected labels not found on node", node.hasLabel( label ) ); } - } + } ); + } + + private > C addToCollection( T value, C collection) + { + collection.add( value ); + return collection; } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/DiskLayerLabelTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/DiskLayerLabelTest.java index 6ebcc66934518..976d50b40445e 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/DiskLayerLabelTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/DiskLayerLabelTest.java @@ -21,10 +21,11 @@ import org.junit.Test; +import java.util.Collection; import java.util.HashSet; +import java.util.Set; +import java.util.function.IntSupplier; -import org.neo4j.collection.primitive.PrimitiveIntCollections; -import org.neo4j.collection.primitive.PrimitiveIntIterator; import org.neo4j.collection.primitive.PrimitiveLongCollections; import org.neo4j.collection.primitive.PrimitiveLongIterator; import org.neo4j.cursor.Cursor; @@ -63,11 +64,18 @@ public void should_be_able_to_list_labels_for_node() throws Exception } // THEN - Cursor node = disk.newStatement().acquireSingleNodeCursor( nodeId ); - node.next(); - PrimitiveIntIterator readLabels = node.get().getLabels(); - assertEquals( new HashSet<>( asList( labelId1, labelId2 ) ), - PrimitiveIntCollections.addToCollection( readLabels, new HashSet() ) ); + disk.newStatement().acquireSingleNodeCursor( nodeId ).forAll( node -> + { + Set actual = node.labels().mapReduce( new HashSet<>(), IntSupplier::getAsInt, + this::addToCollection ); + assertEquals( new HashSet<>( asList( labelId1, labelId2 ) ), actual ); + } ); + } + + private > C addToCollection( T value, C collection) + { + collection.add( value ); + return collection; } @Test diff --git a/community/primitive-collections/src/main/java/org/neo4j/cursor/RawCursor.java b/community/primitive-collections/src/main/java/org/neo4j/cursor/RawCursor.java index 5efde36cd9eb1..6ec594c234802 100644 --- a/community/primitive-collections/src/main/java/org/neo4j/cursor/RawCursor.java +++ b/community/primitive-collections/src/main/java/org/neo4j/cursor/RawCursor.java @@ -19,7 +19,9 @@ */ package org.neo4j.cursor; +import java.util.function.BiFunction; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Supplier; /** @@ -49,12 +51,34 @@ public interface RawCursor extends Supplier, void close() throws EXCEPTION; default void forAll( Consumer consumer ) throws EXCEPTION + { + mapForAll( Function.identity(), consumer ); + } + + default E mapReduce( E initialValue, Function map, BiFunction reduce ) throws EXCEPTION + { + try + { + E current = initialValue; + while ( next() ) + { + current = reduce.apply( map.apply( get() ), current ); + } + return current; + } + finally + { + close(); + } + } + + default void mapForAll( Function function, Consumer consumer ) throws EXCEPTION { try { while ( next() ) { - consumer.accept( get() ); + consumer.accept( function.apply( get() ) ); } } finally