Skip to content

Commit

Permalink
Make sure relationship not deleted on separate thread
Browse files Browse the repository at this point in the history
We need to make sure that relationship hasn't been deleted once we
hold the relationship lock when we are trying to delete relationship.
  • Loading branch information
pontusmelke committed Feb 23, 2018
1 parent dbc1952 commit 99f7041
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 14 deletions.
Expand Up @@ -48,6 +48,7 @@ import org.neo4j.kernel.api.exceptions.schema.{AlreadyConstrainedException, Alre
import org.neo4j.kernel.api.schema.SchemaDescriptorFactory
import org.neo4j.kernel.api.schema.constaints.ConstraintDescriptorFactory
import org.neo4j.kernel.api.schema.index.IndexDescriptorFactory
import org.neo4j.kernel.api.{SilentTokenNameLookup, StatementConstants}
import org.neo4j.kernel.impl.core.EmbeddedProxySPI
import org.neo4j.values.storable.Values

Expand Down Expand Up @@ -521,7 +522,7 @@ final class TransactionBoundQueryContext(tc: TransactionalContextWrapper)

def relationshipEndNode(rel: Relationship) = rel.getEndNode

private val tokenNameLookup = new SilentTokenNameLookup(tc.kernelTransaction.tokenRead())
private val tokenNameLookup = new SilentTokenNameLookup(tc.tokenRead)

override def commitAndRestartTx() { tc.commitAndRestartTx() }

Expand Down
Expand Up @@ -56,7 +56,7 @@ public interface Write
* Delete a relationship
* @param relationship the internal id of the relationship to delete
*/
boolean relationshipDelete( long relationship ) throws AutoIndexingKernelException, EntityNotFoundException;;
boolean relationshipDelete( long relationship ) throws AutoIndexingKernelException, EntityNotFoundException;

/**
* Add a label to a node
Expand Down
Expand Up @@ -140,24 +140,22 @@ public void delete()
boolean deleted = transaction.dataWrite().relationshipDelete( id );
if ( !deleted )
{
throw new NotFoundException( "Unable to delete Relationship[" + id +
"] since it has already been deleted." );
alreadyDeleted();
}
}
catch ( InvalidTransactionTypeKernelException e )
{
throw new ConstraintViolationException( e.getMessage(), e );
}
catch ( EntityNotFoundException e )
{
alreadyDeleted();
}
catch ( AutoIndexingKernelException e )
{
throw new IllegalStateException( "Auto indexing encountered a failure while deleting the relationship: "
+ e.getMessage(), e );
}
catch ( EntityNotFoundException e )
{
throw new NotFoundException( "Unable to delete relationship[" +
getId() + "] since it is already deleted." );
}
}

@Override
Expand Down Expand Up @@ -560,4 +558,10 @@ private void assertInUnterminatedTransaction()
{
actions.assertInUnterminatedTransaction();
}

private void alreadyDeleted()
{
throw new NotFoundException( "Unable to delete relationship[" +
getId() + "] since it is already deleted." );
}
}
Expand Up @@ -52,6 +52,7 @@
import org.neo4j.kernel.api.explicitindex.AutoIndexing;
import org.neo4j.kernel.api.schema.constaints.IndexBackedConstraintDescriptor;
import org.neo4j.kernel.api.schema.index.IndexDescriptor;
import org.neo4j.kernel.api.txstate.TransactionState;
import org.neo4j.kernel.impl.api.KernelTransactionImplementation;
import org.neo4j.kernel.impl.index.IndexEntityType;
import org.neo4j.kernel.impl.locking.ResourceTypes;
Expand Down Expand Up @@ -162,8 +163,8 @@ public long relationshipCreate( long sourceNode, int relationshipType, long targ
sharedRelationshipTypeLock( relationshipType );
lockRelationshipNodes( sourceNode, targetNode );

nodeExists( sourceNode );
nodeExists( targetNode );
assertNodeExists( sourceNode );
assertNodeExists( targetNode );

long id = statement.reserveRelationship();
ktx.txState().relationshipDoCreate( id, relationshipType, sourceNode, targetNode );
Expand All @@ -181,11 +182,22 @@ public boolean relationshipDelete( long relationship ) throws AutoIndexingKernel
{
lockRelationshipNodes( relationshipCursor.sourceNodeReference(), relationshipCursor.targetNodeReference() );
acquireExclusiveRelationshipLock( relationship );
assertRelationshipExists( relationship );

ktx.assertOpen();

autoIndexing.relationships().entityRemoved( this, relationship );
ktx.txState().relationshipDoDelete( relationship, relationshipCursor.getType(),
relationshipCursor.sourceNodeReference(), relationshipCursor.targetNodeReference() );

TransactionState txState = ktx.txState();
if ( txState.relationshipIsAddedInThisTx( relationship ) )
{
txState.relationshipDoDeleteAddedInThisTx( relationship );
}
else
{
txState.relationshipDoDelete( relationship, relationshipCursor.getType(),
relationshipCursor.sourceNodeReference(), relationshipCursor.targetNodeReference() );
}
return true;
}

Expand Down Expand Up @@ -694,14 +706,22 @@ private boolean propertyHasChanged( Value lhs, Value rhs )
return lhs.getClass() != rhs.getClass() || !lhs.equals( rhs );
}

private void nodeExists( long sourceNode ) throws EntityNotFoundException
private void assertNodeExists( long sourceNode ) throws EntityNotFoundException
{
if ( !allStoreHolder.nodeExists( sourceNode ) )
{
throw new EntityNotFoundException( EntityType.NODE, sourceNode );
}
}

private void assertRelationshipExists( long relationship ) throws EntityNotFoundException
{
if ( !allStoreHolder.relationshipExists( relationship ) )
{
throw new EntityNotFoundException( EntityType.RELATIONSHIP, relationship );
}
}

public ExplicitIndexRead indexRead()
{
return allStoreHolder;
Expand Down

0 comments on commit 99f7041

Please sign in to comment.