diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java index 4f5e94d56c58c..7fba3ebe6d059 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java @@ -109,6 +109,9 @@ /** * Collects all Kernel API operations and guards them from being used outside of transaction. + * + * Many methods assume cursors to be initialized before use in private methods, even if they're not passed in explicitly. + * Keep that in mind: e.g. nodeCursor, propertyCursor and relationshipCursor */ public class Operations implements Write, ExplicitIndexWrite, SchemaWrite { @@ -295,7 +298,9 @@ private boolean nodeDelete( long node, boolean lock ) throws AutoIndexingKernelE return false; } - // Assuming that the nodeCursor have been initialized to the node that labels are retrieved from + /** + * Assuming that the nodeCursor have been initialized to the node that labels are retrieved from + */ private void acquireSharedNodeLabelLocks() { ktx.statementLocks().optimistic().acquireShared( ktx.lockTracer(), ResourceTypes.LABEL, nodeCursor.labels().all() ); diff --git a/community/kernel/src/test/java/org/neo4j/graphdb/schema/UpdateDeletedIndexIT.java b/community/kernel/src/test/java/org/neo4j/graphdb/schema/UpdateDeletedIndexIT.java index 3b59202b4f51f..23831beaa00e3 100644 --- a/community/kernel/src/test/java/org/neo4j/graphdb/schema/UpdateDeletedIndexIT.java +++ b/community/kernel/src/test/java/org/neo4j/graphdb/schema/UpdateDeletedIndexIT.java @@ -23,6 +23,8 @@ import org.junit.Test; import java.util.concurrent.TimeUnit; +import java.util.function.LongConsumer; +import java.util.function.LongFunction; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.Transaction; @@ -45,62 +47,32 @@ public class UpdateDeletedIndexIT @Test public void shouldHandleUpdateRemovalOfLabelConcurrentlyWithIndexDrop() throws Throwable { - // given - long[] nodes = createNodes(); - IndexDefinition indexDefinition = createIndex(); - - // when - Race race = new Race(); - race.addContestant( indexDropper( indexDefinition ), 1 ); - for ( int i = 0; i < NODES; i++ ) - { - race.addContestant( nodeLabelRemover( nodes[i] ) ); - } - - // then - race.go(); + shouldHandleIndexDropConcurrentlyWithOperation( nodeId -> db.getNodeById( nodeId ).removeLabel( LABEL ) ); } @Test public void shouldHandleDeleteNodeConcurrentlyWithIndexDrop() throws Throwable { - // given - long[] nodes = createNodes(); - IndexDefinition indexDefinition = createIndex(); - - // when - Race race = new Race(); - race.addContestant( indexDropper( indexDefinition ), 1 ); - for ( int i = 0; i < NODES; i++ ) - { - race.addContestant( nodeDeleter( nodes[i] ) ); - } - - // then - race.go(); + shouldHandleIndexDropConcurrentlyWithOperation( nodeId -> db.getNodeById( nodeId ).delete() ); } @Test public void shouldHandleRemovePropertyConcurrentlyWithIndexDrop() throws Throwable { - // given - long[] nodes = createNodes(); - IndexDefinition indexDefinition = createIndex(); - - // when - Race race = new Race(); - race.addContestant( indexDropper( indexDefinition ), 1 ); - for ( int i = 0; i < NODES; i++ ) - { - race.addContestant( nodePropertyRemover( nodes[i] ) ); - } - - // then - race.go(); + shouldHandleIndexDropConcurrentlyWithOperation( nodeId -> db.getNodeById( nodeId ).removeProperty( KEY ) ); } @Test public void shouldHandleNodeDetachDeleteConcurrentlyWithIndexDrop() throws Throwable + { + shouldHandleIndexDropConcurrentlyWithOperation( nodeId -> + { + ThreadToStatementContextBridge txBridge = db.getDependencyResolver().resolveDependency( ThreadToStatementContextBridge.class ); + txBridge.getKernelTransactionBoundToThisThread( true ).dataWrite().nodeDetachDelete( nodeId ); + } ); + } + + private void shouldHandleIndexDropConcurrentlyWithOperation( NodeOperation operation ) throws Throwable { // given long[] nodes = createNodes(); @@ -108,75 +80,29 @@ public void shouldHandleNodeDetachDeleteConcurrentlyWithIndexDrop() throws Throw // when Race race = new Race(); - race.addContestant( indexDropper( indexDefinition ), 1 ); - for ( int i = 0; i < NODES; i++ ) - { - race.addContestant( nodeDetachDeleter( nodes[i] ) ); - } - - // then - race.go(); - } - - private Runnable indexDropper( IndexDefinition indexDefinition ) - { - return () -> + race.addContestant( () -> { try ( Transaction tx = db.beginTx() ) { indexDefinition.drop(); tx.success(); } - }; - } - - private Runnable nodeLabelRemover( long nodeId ) - { - return () -> - { - try ( Transaction tx = db.beginTx() ) - { - db.getNodeById( nodeId ).removeLabel( LABEL ); - tx.success(); - } - }; - } - - private Runnable nodeDeleter( long nodeId ) - { - return () -> - { - try ( Transaction tx = db.beginTx() ) - { - db.getNodeById( nodeId ).delete(); - tx.success(); - } - }; - } - - private Runnable nodeDetachDeleter( long nodeId ) - { - return throwing( () -> + }, 1 ); + for ( int i = 0; i < NODES; i++ ) { - try ( Transaction tx = db.beginTx() ) + final long nodeId = nodes[i]; + race.addContestant( throwing( () -> { - db.getDependencyResolver().resolveDependency( ThreadToStatementContextBridge.class ).getKernelTransactionBoundToThisThread( - true ).dataWrite().nodeDetachDelete( nodeId ); - tx.success(); - } - } ); - } + try ( Transaction tx = db.beginTx() ) + { + operation.run( nodeId ); + tx.success(); + } + } ) ); + } - private Runnable nodePropertyRemover( long nodeId ) - { - return () -> - { - try ( Transaction tx = db.beginTx() ) - { - db.getNodeById( nodeId ).removeProperty( KEY ); - tx.success(); - } - }; + // then + race.go(); } private long[] createNodes() @@ -218,4 +144,9 @@ private IndexDefinition createIndex() } return indexDefinition; } + + private interface NodeOperation + { + void run( long nodeId ) throws Exception; + } }