From 573df2c2f8c930bc429ee141e2beb5fa3bc7993b Mon Sep 17 00:00:00 2001 From: Pontus Melke Date: Tue, 3 Apr 2018 13:28:54 +0200 Subject: [PATCH] Remove all external accesses to Statement#dataWriteOperations --- .../kernel/api/ExplicitIndexRead.java | 18 +++++++ .../kernel/api/ExplicitIndexWrite.java | 43 +++++++++++++++++ .../kernel/impl/coreapi/IndexManagerImpl.java | 48 +++++++++---------- .../impl/factory/GraphDatabaseFacade.java | 3 +- .../kernel/impl/index/ExplicitIndexStore.java | 8 ++-- .../kernel/impl/newapi/AllStoreHolder.java | 21 ++++++++ .../neo4j/kernel/impl/newapi/Operations.java | 32 +++++++++++++ .../impl/api/index/IndexStatisticsTest.java | 6 +-- .../neo4j/kernel/impl/newapi/MockStore.java | 15 ++++++ .../ConstraintIndexConcurrencyTest.java | 37 ++++++++------ 10 files changed, 183 insertions(+), 48 deletions(-) diff --git a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/ExplicitIndexRead.java b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/ExplicitIndexRead.java index 032ea2c6b4802..9d9be6dfaf4a4 100644 --- a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/ExplicitIndexRead.java +++ b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/ExplicitIndexRead.java @@ -74,6 +74,15 @@ void nodeExplicitIndexQuery( NodeExplicitIndexCursor cursor, String index, Strin */ boolean nodeExplicitIndexExists( String indexName, Map customConfiguration ); + /** + * Return the configuration of the given index + * @param indexName the name of the index + * @return the configuration of the index with the given + * @throws ExplicitIndexNotFoundKernelException if the index is not there + */ + Map nodeExplicitIndexGetConfiguration( String indexName ) + throws ExplicitIndexNotFoundKernelException; + /** * Finds item from explicit index * @@ -138,4 +147,13 @@ void relationshipExplicitIndexQuery( * @return the names of all relationship explicit indexes */ String[] relationshipExplicitIndexesGetAll(); + + /** + * Return the configuration of the given index + * @param indexName the name of the index + * @return the configuration of the index with the given + * @throws ExplicitIndexNotFoundKernelException if the index doesn't exist + */ + Map relationshipExplicitIndexGetConfiguration( String indexName ) + throws ExplicitIndexNotFoundKernelException; } diff --git a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/ExplicitIndexWrite.java b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/ExplicitIndexWrite.java index 43bd7cffe6918..1d45b47d10422 100644 --- a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/ExplicitIndexWrite.java +++ b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/ExplicitIndexWrite.java @@ -79,6 +79,27 @@ void nodeRemoveFromExplicitIndex( String indexName, long node, String key ) thro */ void nodeExplicitIndexDrop( String indexName ) throws ExplicitIndexNotFoundKernelException; + /** + * Updates configuration of the given index + * @param indexName the name of the index + * @param key the configuration key + * @param value the value to be associated with the key + * @return The old value associated with the key or null if nothing associated with the key. + * @throws ExplicitIndexNotFoundKernelException if no such index exists + */ + String nodeExplicitIndexSetConfiguration( String indexName, String key, String value ) + throws ExplicitIndexNotFoundKernelException; + + /** + * Remove a configuration of the given index + * @param indexName the name of the index + * @param key the configuration key + * @return The old value associated with the key or null if nothing associated with the key. + * @throws ExplicitIndexNotFoundKernelException if no such index exists + */ + String nodeExplicitIndexRemoveConfiguration( String indexName, String key ) + throws ExplicitIndexNotFoundKernelException; + /** * Adds relationship to explicit index. * @@ -161,4 +182,26 @@ void relationshipRemoveFromExplicitIndex( String indexName, long relationship ) * @param indexName the index to drop */ void relationshipExplicitIndexDrop( String indexName ) throws ExplicitIndexNotFoundKernelException; + + /** + * Updates configuration of the given index + * @param indexName the name of the index + * @param key the configuration key + * @param value the value to be associated with the key + * @return The old value associated with the key or null if nothing associated with the key. + * @throws ExplicitIndexNotFoundKernelException if no such index exists + */ + String relationshipExplicitIndexSetConfiguration( String indexName, String key, String value ) + throws ExplicitIndexNotFoundKernelException; + + /** + * Remove a configuration of the given index + * @param indexName the name of the index + * @param key the configuration key + * @return The old value associated with the key or null if nothing associated with the key. + * @throws ExplicitIndexNotFoundKernelException if no such index exists + */ + String relationshipExplicitIndexRemoveConfiguration( String indexName, String key ) + throws ExplicitIndexNotFoundKernelException; + } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/IndexManagerImpl.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/IndexManagerImpl.java index 1f1f76e0254f8..bc545ea398075 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/IndexManagerImpl.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/IndexManagerImpl.java @@ -32,19 +32,19 @@ import org.neo4j.graphdb.index.IndexManager; import org.neo4j.graphdb.index.RelationshipAutoIndexer; import org.neo4j.graphdb.index.RelationshipIndex; +import org.neo4j.internal.kernel.api.Transaction; import org.neo4j.internal.kernel.api.exceptions.InvalidTransactionTypeKernelException; import org.neo4j.internal.kernel.api.exceptions.explicitindex.ExplicitIndexNotFoundKernelException; -import org.neo4j.kernel.api.Statement; import org.neo4j.kernel.impl.api.explicitindex.InternalAutoIndexing; public class IndexManagerImpl implements IndexManager { - private final Supplier transactionBridge; + private final Supplier transactionBridge; private final IndexProvider provider; private final AutoIndexer nodeAutoIndexer; private final RelationshipAutoIndexer relAutoIndexer; - public IndexManagerImpl( Supplier bridge, + public IndexManagerImpl( Supplier bridge, IndexProvider provider, AutoIndexer nodeAutoIndexer, RelationshipAutoIndexer relAutoIndexer ) @@ -58,9 +58,9 @@ public IndexManagerImpl( Supplier bridge, @Override public boolean existsForNodes( String indexName ) { - try ( Statement statement = transactionBridge.get() ) + try { - statement.readOperations().nodeExplicitIndexGetConfiguration( indexName ); + transactionBridge.get().indexRead().nodeExplicitIndexGetConfiguration( indexName ); return true; } catch ( ExplicitIndexNotFoundKernelException e ) @@ -91,18 +91,16 @@ public Index forNodes( String indexName, Map customConfigur @Override public String[] nodeIndexNames() { - try ( Statement statement = transactionBridge.get() ) - { - return statement.readOperations().nodeExplicitIndexesGetAll(); - } + + return transactionBridge.get().indexRead().nodeExplicitIndexesGetAll(); } @Override public boolean existsForRelationships( String indexName ) { - try ( Statement statement = transactionBridge.get() ) + try { - statement.readOperations().relationshipExplicitIndexGetConfiguration( indexName ); + transactionBridge.get().indexRead().relationshipExplicitIndexGetConfiguration( indexName ); return true; } catch ( ExplicitIndexNotFoundKernelException e ) @@ -134,24 +132,23 @@ public RelationshipIndex forRelationships( String indexName, @Override public String[] relationshipIndexNames() { - try ( Statement statement = transactionBridge.get() ) - { - return statement.readOperations().relationshipExplicitIndexesGetAll(); - } + + return transactionBridge.get().indexRead().relationshipExplicitIndexesGetAll(); } @Override public Map getConfiguration( Index index ) { - try ( Statement statement = transactionBridge.get() ) + try { + Transaction transaction = transactionBridge.get(); if ( index.getEntityType().equals( Node.class ) ) { - return statement.readOperations().nodeExplicitIndexGetConfiguration( index.getName() ); + return transaction.indexRead().nodeExplicitIndexGetConfiguration( index.getName() ); } if ( index.getEntityType().equals( Relationship.class ) ) { - return statement.readOperations().relationshipExplicitIndexGetConfiguration( index.getName() ); + return transaction.indexRead().relationshipExplicitIndexGetConfiguration( index.getName() ); } throw new IllegalArgumentException( "Unknown entity type " + index.getEntityType().getSimpleName() ); } @@ -166,15 +163,16 @@ public String setConfiguration( Index index, String { // Configuration changes should be done transactionally. However this // has always been done non-transactionally, so it's not a regression. - try ( Statement statement = transactionBridge.get() ) + try { + Transaction transaction = transactionBridge.get(); if ( index.getEntityType().equals( Node.class ) ) { - return statement.dataWriteOperations().nodeExplicitIndexSetConfiguration( index.getName(), key, value ); + return transaction.indexWrite().nodeExplicitIndexSetConfiguration( index.getName(), key, value ); } if ( index.getEntityType().equals( Relationship.class ) ) { - return statement.dataWriteOperations().relationshipExplicitIndexSetConfiguration( + return transaction.indexWrite().relationshipExplicitIndexSetConfiguration( index.getName(), key, value ); } throw new IllegalArgumentException( "Unknown entity type " + index.getEntityType().getSimpleName() ); @@ -194,15 +192,17 @@ public String removeConfiguration( Index index, Str { // Configuration changes should be done transactionally. However this // has always been done non-transactionally, so it's not a regression. - try ( Statement statement = transactionBridge.get() ) + + try { + Transaction transaction = transactionBridge.get(); if ( index.getEntityType().equals( Node.class ) ) { - return statement.dataWriteOperations().nodeExplicitIndexRemoveConfiguration( index.getName(), key ); + return transaction.indexWrite().nodeExplicitIndexRemoveConfiguration( index.getName(), key ); } if ( index.getEntityType().equals( Relationship.class ) ) { - return statement.dataWriteOperations().relationshipExplicitIndexRemoveConfiguration( + return transaction.indexWrite().relationshipExplicitIndexRemoveConfiguration( index.getName(), key ); } throw new IllegalArgumentException( "Unknown entity type " + index.getEntityType().getSimpleName() ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java index a37b5e22d63ab..fbd4cf7eee42e 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java @@ -226,7 +226,8 @@ public void init( EditionModule editionModule, SPI spi, Guard guard, ThreadToSta idxProvider.getOrCreateRelationshipIndex( RELATIONSHIP_AUTO_INDEX, null ) ), spi.autoIndexing().relationships() ); - return new IndexManagerImpl( txBridge, idxProvider, nodeAutoIndexer, relAutoIndexer ); + return new IndexManagerImpl( () -> txBridge.getKernelTransactionBoundToThisThread( true ), idxProvider, + nodeAutoIndexer, relAutoIndexer ); } ); this.contextFactory = Neo4jTransactionalContextFactory.create( spi, guard, txBridge, locker ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/ExplicitIndexStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/ExplicitIndexStore.java index cc61e2589bb10..886e3d5a27abf 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/ExplicitIndexStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/ExplicitIndexStore.java @@ -35,7 +35,6 @@ import org.neo4j.internal.kernel.api.exceptions.explicitindex.ExplicitIndexNotFoundKernelException; import org.neo4j.kernel.api.InwardKernel; import org.neo4j.kernel.api.KernelTransaction; -import org.neo4j.kernel.api.Statement; import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.spi.explicitindex.IndexImplementation; @@ -175,17 +174,16 @@ private Map getOrCreateIndexConfig( // We were the first one here, let's create this config try ( KernelTransaction transaction = - kernel.get().newTransaction( KernelTransaction.Type.implicit, AUTH_DISABLED ); - Statement statement = transaction.acquireStatement() ) + kernel.get().newTransaction( KernelTransaction.Type.implicit, AUTH_DISABLED ) ) { switch ( entityType ) { case Node: - statement.dataWriteOperations().nodeExplicitIndexCreate( indexName, config ); + transaction.indexWrite().nodeExplicitIndexCreate( indexName, config ); break; case Relationship: - statement.dataWriteOperations().relationshipExplicitIndexCreate( indexName, config ); + transaction.indexWrite().relationshipExplicitIndexCreate( indexName, config ); break; default: diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java index 911cd1383aabf..69f7780cc9c45 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java @@ -288,6 +288,14 @@ public boolean nodeExplicitIndexExists( String indexName, Map cus return explicitIndexTxState().checkIndexExistence( IndexEntityType.Node, indexName, customConfiguration ); } + @Override + public Map nodeExplicitIndexGetConfiguration( String indexName ) + throws ExplicitIndexNotFoundKernelException + { + ktx.assertOpen(); + return explicitIndexStore.getNodeIndexConfiguration( indexName ); + } + @Override public String[] relationshipExplicitIndexesGetAll() { @@ -302,6 +310,14 @@ public boolean relationshipExplicitIndexExists( String indexName, Map relationshipExplicitIndexGetConfiguration( String indexName ) + throws ExplicitIndexNotFoundKernelException + { + ktx.assertOpen(); + return explicitIndexStore.getRelationshipIndexConfiguration( indexName ); + } + @Override public CapableIndexReference index( int label, int... properties ) { @@ -1019,6 +1035,11 @@ public void schemaStateFlush() schemaState.clear(); } + ExplicitIndexStore explicitIndexStore() + { + return explicitIndexStore; + } + private RawIterator callProcedure( int id, Object[] input, final AccessMode override ) throws ProcedureException 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 c696a9818d952..986fa0e82ac4a 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 @@ -642,6 +642,22 @@ public void nodeExplicitIndexDrop( String indexName ) throws ExplicitIndexNotFou txState.deleteIndex( IndexEntityType.Node, indexName ); } + @Override + public String nodeExplicitIndexSetConfiguration( String indexName, String key, String value ) + throws ExplicitIndexNotFoundKernelException + { + ktx.assertOpen(); + return allStoreHolder.explicitIndexStore().setNodeIndexConfiguration( indexName, key, value ); + } + + @Override + public String nodeExplicitIndexRemoveConfiguration( String indexName, String key ) + throws ExplicitIndexNotFoundKernelException + { + ktx.assertOpen(); + return allStoreHolder.explicitIndexStore().removeNodeIndexConfiguration( indexName, key ); + } + @Override public void relationshipAddToExplicitIndex( String indexName, long relationship, String key, Object value ) throws ExplicitIndexNotFoundKernelException, EntityNotFoundException @@ -966,6 +982,22 @@ public ConstraintDescriptor relationshipPropertyExistenceConstraintCreate( Relat } + @Override + public String relationshipExplicitIndexSetConfiguration( String indexName, String key, String value ) + throws ExplicitIndexNotFoundKernelException + { + ktx.assertOpen(); + return allStoreHolder.explicitIndexStore().setRelationshipIndexConfiguration( indexName, key, value ); + } + + @Override + public String relationshipExplicitIndexRemoveConfiguration( String indexName, String key ) + throws ExplicitIndexNotFoundKernelException + { + ktx.assertOpen(); + return allStoreHolder.explicitIndexStore().removeRelationshipIndexConfiguration( indexName, key ); + } + @Override public void constraintDrop( ConstraintDescriptor descriptor ) throws SchemaKernelException { diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexStatisticsTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexStatisticsTest.java index 07134ade858c1..df53c9da0fb18 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexStatisticsTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexStatisticsTest.java @@ -372,10 +372,8 @@ private void deleteNode( long nodeId ) throws KernelException { try ( Transaction tx = db.beginTx() ) { - try ( Statement statement = bridge.get() ) - { - statement.dataWriteOperations().nodeDelete( nodeId ); - } + + bridge.getKernelTransactionBoundToThisThread( true ).dataWrite().nodeDelete( nodeId ); tx.success(); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/MockStore.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/MockStore.java index 0be80df8f23e7..2d83d9cc2bcd9 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/MockStore.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/MockStore.java @@ -36,6 +36,7 @@ import org.neo4j.internal.kernel.api.InternalIndexState; import org.neo4j.internal.kernel.api.exceptions.KernelException; import org.neo4j.internal.kernel.api.exceptions.ProcedureException; +import org.neo4j.internal.kernel.api.exceptions.explicitindex.ExplicitIndexNotFoundKernelException; import org.neo4j.internal.kernel.api.exceptions.schema.SchemaKernelException; import org.neo4j.internal.kernel.api.procs.ProcedureHandle; import org.neo4j.internal.kernel.api.procs.ProcedureSignature; @@ -416,6 +417,13 @@ public boolean nodeExplicitIndexExists( String indexName, Map cus throw new UnsupportedOperationException(); } + @Override + public Map nodeExplicitIndexGetConfiguration( String indexName ) + throws ExplicitIndexNotFoundKernelException + { + throw new UnsupportedOperationException(); + } + @Override public boolean relationshipExplicitIndexExists( String indexName, Map customConfiguration ) { @@ -434,6 +442,13 @@ public String[] relationshipExplicitIndexesGetAll() throw new UnsupportedOperationException(); } + @Override + public Map relationshipExplicitIndexGetConfiguration( String indexName ) + throws ExplicitIndexNotFoundKernelException + { + throw new UnsupportedOperationException(); + } + private abstract static class Record { abstract void initialize( R record ); diff --git a/community/lucene-index/src/test/java/org/neo4j/concurrencytest/ConstraintIndexConcurrencyTest.java b/community/lucene-index/src/test/java/org/neo4j/concurrencytest/ConstraintIndexConcurrencyTest.java index 04f368c6c77cf..7430a02358ed4 100644 --- a/community/lucene-index/src/test/java/org/neo4j/concurrencytest/ConstraintIndexConcurrencyTest.java +++ b/community/lucene-index/src/test/java/org/neo4j/concurrencytest/ConstraintIndexConcurrencyTest.java @@ -27,13 +27,17 @@ import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Transaction; import org.neo4j.helpers.collection.Iterators; -import org.neo4j.kernel.api.Statement; +import org.neo4j.internal.kernel.api.IndexOrder; +import org.neo4j.internal.kernel.api.IndexQuery; +import org.neo4j.internal.kernel.api.NodeValueIndexCursor; +import org.neo4j.internal.kernel.api.Read; +import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException; import org.neo4j.kernel.api.exceptions.schema.UniquePropertyValueValidationException; -import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.kernel.api.schema.constaints.ConstraintDescriptorFactory; import org.neo4j.kernel.api.schema.index.SchemaIndexDescriptor; import org.neo4j.kernel.api.schema.index.SchemaIndexDescriptorFactory; +import org.neo4j.kernel.impl.api.store.DefaultIndexReference; import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge; import org.neo4j.kernel.internal.GraphDatabaseAPI; import org.neo4j.test.rule.DatabaseRule; @@ -58,8 +62,8 @@ public void shouldNotAllowConcurrentViolationOfConstraint() throws Exception // Given GraphDatabaseAPI graphDb = db.getGraphDatabaseAPI(); - Supplier statementSupplier = graphDb.getDependencyResolver() - .resolveDependency( ThreadToStatementContextBridge.class ); + Supplier ktxSupplier = () -> graphDb.getDependencyResolver() + .resolveDependency( ThreadToStatementContextBridge.class ).getKernelTransactionBoundToThisThread( true ); Label label = label( "Foo" ); String propertyKey = "bar"; @@ -73,15 +77,20 @@ public void shouldNotAllowConcurrentViolationOfConstraint() throws Exception } // When - try ( Transaction tx = graphDb.beginTx(); - Statement statement = statementSupplier.get() ) + try ( Transaction tx = graphDb.beginTx() ) { - int labelId = statement.readOperations().labelGetForName( label.name() ); - int propertyKeyId = statement.readOperations().propertyKeyGetForName( propertyKey ); + KernelTransaction ktx = ktxSupplier.get(); + int labelId = ktx.tokenRead().nodeLabel( label.name() ); + int propertyKeyId = ktx.tokenRead().propertyKey( propertyKey ); SchemaIndexDescriptor index = SchemaIndexDescriptorFactory.uniqueForLabel( labelId, propertyKeyId ); - statement.readOperations().indexQuery( index, IndexQuery.exact( index.schema().getPropertyId(), - "The value is irrelevant, we just want to perform some sort of lookup against this index" ) ); - + Read read = ktx.dataRead(); + try ( NodeValueIndexCursor cursor = ktx.cursors().allocateNodeValueIndexCursor() ) + { + read.nodeIndexSeek( DefaultIndexReference.unique( labelId, propertyKeyId ), cursor, IndexOrder.NONE, + IndexQuery.exact( index.schema().getPropertyId(), + "The value is irrelevant, we just want to perform some sort of lookup against this " + + "index" ) ); + } // then let another thread come in and create a node threads.execute( db -> { @@ -95,11 +104,11 @@ public void shouldNotAllowConcurrentViolationOfConstraint() throws Exception // before we create a node with the same property ourselves - using the same statement that we have // already used for lookup against that very same index - long node = statement.dataWriteOperations().nodeCreate(); - statement.dataWriteOperations().nodeAddLabel( node, labelId ); + long node = ktx.dataWrite().nodeCreate(); + ktx.dataWrite().nodeAddLabel( node, labelId ); try { - statement.dataWriteOperations().nodeSetProperty( node, propertyKeyId, Values.of( conflictingValue ) ); + ktx.dataWrite().nodeSetProperty( node, propertyKeyId, Values.of( conflictingValue ) ); fail( "exception expected" ); }