Skip to content

Commit

Permalink
Specialize NodeProxy.getRelationships for dense nodes
Browse files Browse the repository at this point in the history
Also improve traversal & group cursor robustness against concurrent
modification by record loading using RecordLoad.FORCE. Now if a record
is deleted after the cursor got the reference, the whole record is loaded
anyway, and we can continue to the next link in the chain by reading the
now unused record. This works because deleting a record R redirects the previous
and next records in the chain and sets R.inUse = false, but doesn't modify
any other contents of R.
  • Loading branch information
fickludd authored and pontusmelke committed Feb 1, 2018
1 parent f86ac82 commit 9f81e14
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 79 deletions.
Expand Up @@ -372,25 +372,25 @@ public boolean isPropertyRemoved( int propertyKeyId )
@Override
public PrimitiveLongIterator getAddedRelationships( Direction direction )
{
return null;
return PrimitiveLongCollections.emptyIterator();
}

@Override
public PrimitiveLongIterator getAddedRelationships( Direction direction, int[] relTypes )
{
return null;
return PrimitiveLongCollections.emptyIterator();
}

@Override
public PrimitiveLongIterator getAddedRelationships()
{
return null;
return PrimitiveLongCollections.emptyIterator();
}

@Override
public PrimitiveLongIterator getAddedRelationships( RelationshipDirection direction, int relType )
{
return null;
return PrimitiveLongCollections.emptyIterator();
}

};
Expand Down
Expand Up @@ -42,6 +42,9 @@
import org.neo4j.internal.kernel.api.LabelSet;
import org.neo4j.internal.kernel.api.NodeCursor;
import org.neo4j.internal.kernel.api.PropertyCursor;
import org.neo4j.internal.kernel.api.RelationshipDenseSelectionCursor;
import org.neo4j.internal.kernel.api.RelationshipGroupCursor;
import org.neo4j.internal.kernel.api.RelationshipSparseSelectionCursor;
import org.neo4j.internal.kernel.api.RelationshipTraversalCursor;
import org.neo4j.internal.kernel.api.TokenRead;
import org.neo4j.internal.kernel.api.exceptions.InvalidTransactionTypeKernelException;
Expand All @@ -50,15 +53,15 @@
import org.neo4j.internal.kernel.api.exceptions.PropertyKeyIdNotFoundKernelException;
import org.neo4j.internal.kernel.api.exceptions.explicitindex.AutoIndexingKernelException;
import org.neo4j.internal.kernel.api.exceptions.schema.ConstraintValidationException;
import org.neo4j.internal.kernel.api.exceptions.schema.IllegalTokenNameException;
import org.neo4j.internal.kernel.api.exceptions.schema.TooManyLabelsException;
import org.neo4j.kernel.api.KernelTransaction;
import org.neo4j.kernel.api.ReadOperations;
import org.neo4j.kernel.api.Statement;
import org.neo4j.kernel.api.StatementTokenNameLookup;
import org.neo4j.kernel.api.exceptions.EntityNotFoundException;
import org.neo4j.kernel.api.exceptions.RelationshipTypeIdNotFoundKernelException;
import org.neo4j.kernel.api.exceptions.Status;
import org.neo4j.internal.kernel.api.exceptions.schema.IllegalTokenNameException;
import org.neo4j.internal.kernel.api.exceptions.schema.TooManyLabelsException;
import org.neo4j.kernel.impl.api.operations.KeyReadOperations;
import org.neo4j.storageengine.api.EntityType;
import org.neo4j.values.storable.Value;
Expand Down Expand Up @@ -167,16 +170,74 @@ private ResourceIterable<Relationship> innerGetRelationships( final Direction di
throw new NotFoundException( format( "Node %d not found", nodeId ) );
}

RelationshipTraversalCursor relationship = transaction.cursors().allocateRelationshipTraversalCursor();
try
if ( node.isDense() )
{
node.allRelationships( relationship );
return new RelationshipConversion( spi, relationship, direction, typeIds );
RelationshipTraversalCursor relationship = transaction.cursors().allocateRelationshipTraversalCursor();
RelationshipGroupCursor relationshipGroup = transaction.cursors().allocateRelationshipGroupCursor();
try
{
node.relationships( relationshipGroup );
RelationshipDenseSelectionCursor selectionCursor = new RelationshipDenseSelectionCursor();

switch ( direction )
{
case OUTGOING:
selectionCursor.outgoing( relationshipGroup, relationship, typeIds );
break;

case INCOMING:
selectionCursor.incoming( relationshipGroup, relationship, typeIds );
break;

case BOTH:
selectionCursor.all( relationshipGroup, relationship, typeIds );
break;

default:
throw new IllegalStateException( "Unsupported direction: "+direction );
}

return new RelationshipConversion( spi, selectionCursor );
}
catch ( Throwable e )
{
relationshipGroup.close();
throw e;
}
}
catch ( Throwable e )
else
{
relationship.close();
throw e;
RelationshipTraversalCursor relationship = transaction.cursors().allocateRelationshipTraversalCursor();
try
{
node.allRelationships( relationship );
RelationshipSparseSelectionCursor selectionCursor = new RelationshipSparseSelectionCursor();

switch ( direction )
{
case OUTGOING:
selectionCursor.outgoing( relationship, typeIds );
break;

case INCOMING:
selectionCursor.incoming( relationship, typeIds );
break;

case BOTH:
selectionCursor.all( relationship, typeIds );
break;

default:
throw new IllegalStateException( "Unsupported direction: "+direction );
}

return new RelationshipConversion( spi, selectionCursor );
}
catch ( Throwable e )
{
relationship.close();
throw e;
}
}
};
}
Expand Down
Expand Up @@ -19,70 +19,44 @@
*/
package org.neo4j.kernel.impl.core;

import org.apache.commons.lang3.ArrayUtils;

import java.util.NoSuchElementException;

import org.neo4j.graphdb.Direction;
import org.neo4j.graphdb.Relationship;
import org.neo4j.graphdb.ResourceIterator;
import org.neo4j.internal.kernel.api.RelationshipTraversalCursor;
import org.neo4j.internal.kernel.api.RelationshipSelectionCursor;

public class RelationshipConversion implements ResourceIterator<Relationship>
{
private final EmbeddedProxySPI actions;
private final RelationshipTraversalCursor cursor;
private final Direction direction;
private final int[] typeIds;
private final RelationshipSelectionCursor cursor;
private Relationship next;
private boolean closed;

public RelationshipConversion(
EmbeddedProxySPI actions,
RelationshipTraversalCursor cursor,
Direction direction,
int[] typeIds)
RelationshipConversion( EmbeddedProxySPI actions, RelationshipSelectionCursor cursor )
{
this.actions = actions;
this.cursor = cursor;
this.direction = direction;
this.typeIds = typeIds;
}

@Override
public boolean hasNext()
{
if ( next == null && !closed )
{
while ( cursor.next() )
if ( cursor.next() )
{
if ( correctDirection() && correctType() )
{
next = actions.newRelationshipProxy(
next = actions.newRelationshipProxy(
cursor.relationshipReference(),
cursor.sourceNodeReference(),
cursor.label(),
cursor.targetNodeReference() );
return true;
}
}
close();
}
return next != null;
}

private boolean correctDirection()
{
return direction == Direction.BOTH ||
(direction == Direction.OUTGOING && cursor.originNodeReference() == cursor.sourceNodeReference()) ||
(direction == Direction.INCOMING && cursor.originNodeReference() == cursor.targetNodeReference());
}

private boolean correctType()
{
return typeIds == null || ArrayUtils.contains( typeIds, cursor.label() );
}

@Override
public Relationship next()
{
Expand Down
Expand Up @@ -317,21 +317,37 @@ void node( NodeRecord record, long reference, PageCursor pageCursor )
@Override
void relationship( RelationshipRecord record, long reference, PageCursor pageCursor )
{
// When scanning, we inspect RelationshipRecord.inUse(), so using RecordLoad.CHECK is fine
relationships.getRecordByCursor( reference, record, RecordLoad.CHECK, pageCursor );
}

@Override
void relationshipFull( RelationshipRecord record, long reference, PageCursor pageCursor )
{
// We need to load forcefully for relationship chain traversal since otherwise we cannot
// traverse over relationship records which have been concurrently deleted
// (flagged as inUse = false).
// see
// org.neo4j.kernel.impl.store.RelationshipChainPointerChasingTest
// org.neo4j.kernel.impl.locking.RelationshipCreateDeleteIT
relationships.getRecordByCursor( reference, record, RecordLoad.FORCE, pageCursor );
}

@Override
void property( PropertyRecord record, long reference, PageCursor pageCursor )
{
//We need to load forcefully here since otherwise we can have inconsistent reads
//for properties across blocks, see org.neo4j.graphdb.ConsistentPropertyReadsIT
// We need to load forcefully here since otherwise we can have inconsistent reads
// for properties across blocks, see org.neo4j.graphdb.ConsistentPropertyReadsIT
properties.getRecordByCursor( reference, record, RecordLoad.FORCE, pageCursor );
}

@Override
void group( RelationshipGroupRecord record, long reference, PageCursor page )
{
groups.getRecordByCursor( reference, record, RecordLoad.NORMAL, page );
// We need to load forcefully here since otherwise we cannot traverse over groups
// records which have been concurrently deleted (flagged as inUse = false).
// @see #org.neo4j.kernel.impl.store.RelationshipChainPointerChasingTest
groups.getRecordByCursor( reference, record, RecordLoad.FORCE, page );
}

@Override
Expand Down
Expand Up @@ -59,7 +59,7 @@ void buffer( long nodeReference, long relationshipReference, Read read )
BufferedGroup current = null;
while ( relationshipReference != NO_ID )
{
read.relationship( edge, relationshipReference, edgePage );
read.relationshipFull( edge, relationshipReference, edgePage );
// find the group
BufferedGroup group = buffer.get( edge.getType() );
if ( group == null )
Expand Down Expand Up @@ -135,11 +135,16 @@ public boolean next()
setFirstLoop( bufferedGroup.loops() );
return true;
}
if ( getNext() == NO_ID )

do
{
return false;
}
read.group( this, getNext(), page );
if ( getNext() == NO_ID )
{
return false;
}
read.group( this, getNext(), page );
} while ( !inUse() );

return true;
}

Expand Down
Expand Up @@ -271,7 +271,7 @@ public boolean next()
// - Return first relationship if it's not deleted
// Subsequent relationships need to have same type and direction

read.relationship( this, next, pageCursor );
read.relationshipFull( this, next, pageCursor );
setupFilterState();

hasChanges = hasChanges();
Expand Down Expand Up @@ -313,7 +313,7 @@ public boolean next()
return false;
}

read.relationship( this, next, pageCursor );
read.relationshipFull( this, next, pageCursor );
computeNext();

} while ( ( filterStore && !correctTypeAndDirection() ) ||
Expand Down Expand Up @@ -445,7 +445,7 @@ else if ( target == originNodeReference )
}
else
{
throw new IllegalStateException( "NOT PART OF CHAIN" );
throw new IllegalStateException( "NOT PART OF CHAIN! " + this );
}
}

Expand Down
Expand Up @@ -500,6 +500,8 @@ public final void futureRelationshipPropertyReferenceRead( long reference )

abstract void relationship( RelationshipRecord record, long reference, PageCursor pageCursor );

abstract void relationshipFull( RelationshipRecord record, long reference, PageCursor pageCursor );

abstract void property( PropertyRecord record, long reference, PageCursor pageCursor );

abstract void group( RelationshipGroupRecord record, long reference, PageCursor page );
Expand Down
Expand Up @@ -22,8 +22,7 @@
import org.junit.Before;
import org.junit.Test;

import org.neo4j.graphdb.Direction;
import org.neo4j.internal.kernel.api.RelationshipTraversalCursor;
import org.neo4j.internal.kernel.api.RelationshipSelectionCursor;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
Expand All @@ -39,16 +38,16 @@ public class RelationshipConversionTest

private EmbeddedProxySPI nodeActions = mock( EmbeddedProxySPI.class );
private RelationshipConversion relationshipConversion;
private RelationshipTraversalCursor cursor;
private RelationshipSelectionCursor cursor;

@Before
public void setUp()
{
when( nodeActions.newRelationshipProxy( anyLong(), anyLong(), anyInt(), anyLong() ) )
.thenReturn( new RelationshipProxy( null, 1 ) );

cursor = mock( RelationshipTraversalCursor.class );
relationshipConversion = new RelationshipConversion( nodeActions, cursor, Direction.BOTH, null );
cursor = mock( RelationshipSelectionCursor.class );
relationshipConversion = new RelationshipConversion( nodeActions, cursor );
}

@Test
Expand Down

0 comments on commit 9f81e14

Please sign in to comment.