From c4b7a4dac11ddfd4679d9075150e0df4f3e4d11b Mon Sep 17 00:00:00 2001 From: Mattias Persson Date: Mon, 20 Feb 2017 10:54:29 +0100 Subject: [PATCH] Adds configuration for releasing schema lock in constraint index building --- .../factory/GraphDatabaseSettings.java | 5 +++ .../org/neo4j/kernel/NeoStoreDataSource.java | 5 ++- .../api/state/ConstraintIndexCreator.java | 27 +++++++++++--- .../ConstraintIndexCreatorTest.java | 35 +++++++++++++++++-- .../neo4j/kernel/impl/api/index/IndexIT.java | 2 +- .../ha/MasterTransactionCommitProcess.java | 10 ++++-- .../modeswitch/CommitProcessSwitcher.java | 8 +++-- .../factory/HighlyAvailableEditionModule.java | 3 +- 8 files changed, 80 insertions(+), 15 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java b/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java index 1d0d7ca6d7ce4..cc49c9f68dacb 100644 --- a/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java +++ b/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java @@ -511,6 +511,11 @@ private static String defaultPageCacheMemory() public static final Setting auth_store = pathSetting( "unsupported.dbms.security.auth_store.location", NO_DEFAULT ); + @Description( "Whether or not to release the exclusive schema lock is while building uniqueness constraints index" ) + @Internal + public static final Setting release_schema_lock_while_building_constraint = setting( + "unsupported.dbms.schema.release_lock_while_building_constraint", BOOLEAN, FALSE ); + // Bolt Settings @Group("dbms.connector") diff --git a/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java b/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java index 10f87c6a7b647..4aae6d6bfa418 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java @@ -779,8 +779,11 @@ private KernelModule buildKernel( TransactionAppender appender, */ Supplier kernelProvider = () -> kernelModule.kernelAPI(); + boolean releaseSchemaLockWhenBuildingConstratinIndexes = + config.get( GraphDatabaseSettings.release_schema_lock_while_building_constraint ); ConstraintIndexCreator constraintIndexCreator = - new ConstraintIndexCreator( kernelProvider, indexingService, propertyAccessor ); + new ConstraintIndexCreator( kernelProvider, indexingService, propertyAccessor, + releaseSchemaLockWhenBuildingConstratinIndexes ); LegacyIndexStore legacyIndexStore = new LegacyIndexStore( config, indexConfigStore, kernelProvider, legacyIndexProviderLookup ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/ConstraintIndexCreator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/ConstraintIndexCreator.java index 01e1225000309..a51ee97fb04e6 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/ConstraintIndexCreator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/ConstraintIndexCreator.java @@ -42,7 +42,6 @@ import org.neo4j.kernel.impl.api.index.IndexingService; import org.neo4j.kernel.impl.api.operations.SchemaReadOperations; import org.neo4j.kernel.impl.locking.Locks.Client; - import static java.util.Collections.singleton; import static org.neo4j.kernel.impl.locking.ResourceTypes.SCHEMA; @@ -54,13 +53,15 @@ public class ConstraintIndexCreator private final IndexingService indexingService; private final Supplier kernelSupplier; private final PropertyAccessor propertyAccessor; + private final boolean releaseSchemaLockWhenCreatingConstraint; public ConstraintIndexCreator( Supplier kernelSupplier, IndexingService indexingService, - PropertyAccessor propertyAccessor ) + PropertyAccessor propertyAccessor, boolean releaseSchemaLockWhenCreatingConstraint ) { this.kernelSupplier = kernelSupplier; this.indexingService = indexingService; this.propertyAccessor = propertyAccessor; + this.releaseSchemaLockWhenCreatingConstraint = releaseSchemaLockWhenCreatingConstraint; } /** @@ -101,7 +102,7 @@ public long createUniquenessConstraintIndex( KernelStatement state, SchemaReadOp // At this point the integrity of the constraint to be created was checked // while holding the lock and the index rule backing the soon-to-be-created constraint // has been created. Now it's just the population left, which can take a long time - locks.releaseExclusive( SCHEMA, schemaResource() ); + releaseSchemaLock( locks ); awaitIndexPopulation( constraint, indexId ); @@ -109,7 +110,7 @@ public long createUniquenessConstraintIndex( KernelStatement state, SchemaReadOp // Acquire SCHEMA WRITE lock and verify the constraints here in this user transaction // and if everything checks out then it will be held until after the constraint has been // created and activated. - locks.acquireExclusive( SCHEMA, schemaResource() ); + acquireSchemaLock( locks ); reacquiredSchemaLock = true; indexingService.getIndexProxy( indexId ).verifyDeferredConstraints( propertyAccessor ); @@ -135,13 +136,29 @@ public long createUniquenessConstraintIndex( KernelStatement state, SchemaReadOp { if ( !reacquiredSchemaLock ) { - locks.acquireExclusive( SCHEMA, schemaResource() ); + acquireSchemaLock( locks ); } dropUniquenessConstraintIndex( descriptor ); } } } + private void acquireSchemaLock( Client locks ) + { + if ( releaseSchemaLockWhenCreatingConstraint ) + { + locks.acquireExclusive( SCHEMA, schemaResource() ); + } + } + + private void releaseSchemaLock( Client locks ) + { + if ( releaseSchemaLockWhenCreatingConstraint ) + { + locks.releaseExclusive( SCHEMA, schemaResource() ); + } + } + /** * You MUST hold a schema write lock before you call this method. */ diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintIndexCreatorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintIndexCreatorTest.java index 211b90699638b..c91abeadf0759 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintIndexCreatorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintIndexCreatorTest.java @@ -43,9 +43,11 @@ import org.neo4j.kernel.impl.api.index.IndexProxy; import org.neo4j.kernel.impl.api.index.IndexingService; import org.neo4j.kernel.impl.api.state.ConstraintIndexCreator; +import org.neo4j.kernel.impl.locking.ResourceTypes; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -77,7 +79,7 @@ public void shouldCreateIndexInAnotherTransaction() throws Exception when( indexingService.getIndexProxy( 2468L ) ).thenReturn( indexProxy ); PropertyAccessor propertyAccessor = mock( PropertyAccessor.class ); - ConstraintIndexCreator creator = new ConstraintIndexCreator( () -> kernel, indexingService, propertyAccessor ); + ConstraintIndexCreator creator = new ConstraintIndexCreator( () -> kernel, indexingService, propertyAccessor, false ); // when long indexId = creator.createUniquenessConstraintIndex( state, constraintCreationContext.schemaReadOperations(), 123, 456 ); @@ -113,7 +115,7 @@ public void shouldDropIndexIfPopulationFails() throws Exception .when(indexProxy).awaitStoreScanCompleted(); PropertyAccessor propertyAccessor = mock( PropertyAccessor.class ); - ConstraintIndexCreator creator = new ConstraintIndexCreator( () -> kernel, indexingService, propertyAccessor ); + ConstraintIndexCreator creator = new ConstraintIndexCreator( () -> kernel, indexingService, propertyAccessor, false ); // when try @@ -149,7 +151,7 @@ public void shouldDropIndexInAnotherTransaction() throws Exception IndexDescriptor descriptor = new IndexDescriptor( 123, 456 ); PropertyAccessor propertyAccessor = mock( PropertyAccessor.class ); - ConstraintIndexCreator creator = new ConstraintIndexCreator( () -> kernel, indexingService, propertyAccessor ); + ConstraintIndexCreator creator = new ConstraintIndexCreator( () -> kernel, indexingService, propertyAccessor, false ); // when creator.dropUniquenessConstraintIndex( descriptor ); @@ -160,6 +162,33 @@ public void shouldDropIndexInAnotherTransaction() throws Exception verifyZeroInteractions( indexingService ); } + @Test + public void shouldReleaseSchemaLockWhileAwaitingIndexPopulation() throws Exception + { + // given + StubKernel kernel = new StubKernel(); + IndexingService indexingService = mock( IndexingService.class ); + StatementOperationParts constraintCreationContext = mockedParts(); + IndexDescriptor descriptor = new IndexDescriptor( 123, 456 ); + PropertyAccessor propertyAccessor = mock( PropertyAccessor.class ); + + KernelStatement state = mockedState(); + + when( constraintCreationContext.schemaReadOperations().indexGetCommittedId( state, descriptor, CONSTRAINT ) ) + .thenReturn( 2468L ); + IndexProxy indexProxy = mock( IndexProxy.class ); + when( indexingService.getIndexProxy( anyLong() ) ).thenReturn( indexProxy ); + + ConstraintIndexCreator creator = new ConstraintIndexCreator( () -> kernel, indexingService, propertyAccessor, true ); + + // when + creator.createUniquenessConstraintIndex( state, constraintCreationContext.schemaReadOperations(), 123, 456 ); + + // then + verify( state.locks().pessimistic() ).releaseExclusive( ResourceTypes.SCHEMA, ResourceTypes.schemaResource() ); + verify( state.locks().pessimistic() ).acquireExclusive( ResourceTypes.SCHEMA, ResourceTypes.schemaResource() ); + } + public class StubKernel implements KernelAPI { private final List statements = new ArrayList<>(); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexIT.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexIT.java index bb9ea6295b07e..ac7718a93765b 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexIT.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexIT.java @@ -136,7 +136,7 @@ public void shouldRemoveAConstraintIndexWithoutOwnerInRecovery() throws Exceptio { // given PropertyAccessor propertyAccessor = mock( PropertyAccessor.class ); - ConstraintIndexCreator creator = new ConstraintIndexCreator( () -> kernel, indexingService, propertyAccessor ); + ConstraintIndexCreator creator = new ConstraintIndexCreator( () -> kernel, indexingService, propertyAccessor, false ); creator.createConstraintIndex( labelId, propertyKeyId ); // when diff --git a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/MasterTransactionCommitProcess.java b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/MasterTransactionCommitProcess.java index 1b3885d3f150a..aaa258b51d712 100644 --- a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/MasterTransactionCommitProcess.java +++ b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/MasterTransactionCommitProcess.java @@ -57,6 +57,7 @@ command instanceof NodeCommand && mayResultInIndexUpdates( (NodeCommand) command private final IntegrityValidator validator; private final Monitor monitor; private final Locks locks; + private final boolean reacquireSharedSchemaLockOnIncomingTransactions; public interface Monitor { @@ -64,13 +65,15 @@ public interface Monitor } public MasterTransactionCommitProcess( TransactionCommitProcess commitProcess, TransactionPropagator txPropagator, - IntegrityValidator validator, Monitor monitor, Locks locks ) + IntegrityValidator validator, Monitor monitor, Locks locks, + boolean reacquireSharedSchemaLockOnIncomingTransactions ) { this.inner = commitProcess; this.txPropagator = txPropagator; this.validator = validator; this.monitor = monitor; this.locks = locks; + this.reacquireSharedSchemaLockOnIncomingTransactions = reacquireSharedSchemaLockOnIncomingTransactions; } @Override @@ -102,7 +105,10 @@ private Locks.Client validate( TransactionToApply batch ) throws TransactionFail { while ( batch != null ) { - locks = acquireSharedSchemaLockIfTransactionResultsInIndexUpdates( batch, locks ); + if ( reacquireSharedSchemaLockOnIncomingTransactions ) + { + locks = acquireSharedSchemaLockIfTransactionResultsInIndexUpdates( batch, locks ); + } validator.validateTransactionStartKnowledge( batch.transactionRepresentation().getLatestCommittedTxWhenStarted() ); batch = batch.next(); diff --git a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/cluster/modeswitch/CommitProcessSwitcher.java b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/cluster/modeswitch/CommitProcessSwitcher.java index cb2b029cfe54f..03a784ae94583 100644 --- a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/cluster/modeswitch/CommitProcessSwitcher.java +++ b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/cluster/modeswitch/CommitProcessSwitcher.java @@ -42,11 +42,13 @@ public class CommitProcessSwitcher extends AbstractComponentSwitcher delegate, RequestContextFactory requestContextFactory, Locks locks, - Monitors monitors, DependencyResolver dependencyResolver ) + Monitors monitors, DependencyResolver dependencyResolver, + boolean reacquireSharedSchemaLockOnIncomingTransactions ) { super( delegate ); this.txPropagator = txPropagator; @@ -54,6 +56,7 @@ public CommitProcessSwitcher( TransactionPropagator txPropagator, Master master, this.requestContextFactory = requestContextFactory; this.locks = locks; this.dependencyResolver = dependencyResolver; + this.reacquireSharedSchemaLockOnIncomingTransactions = reacquireSharedSchemaLockOnIncomingTransactions; this.monitor = monitors.newMonitor( MasterTransactionCommitProcess.Monitor.class ); } @@ -71,6 +74,7 @@ protected TransactionCommitProcess getMasterImpl() dependencyResolver.resolveDependency( StorageEngine.class ) ); IntegrityValidator validator = dependencyResolver.resolveDependency( IntegrityValidator.class ); - return new MasterTransactionCommitProcess( commitProcess, txPropagator, validator, monitor, locks ); + return new MasterTransactionCommitProcess( commitProcess, txPropagator, validator, monitor, locks, + reacquireSharedSchemaLockOnIncomingTransactions ); } } diff --git a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/factory/HighlyAvailableEditionModule.java b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/factory/HighlyAvailableEditionModule.java index 4b36cddea31b2..9da1072717eca 100644 --- a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/factory/HighlyAvailableEditionModule.java +++ b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/factory/HighlyAvailableEditionModule.java @@ -606,7 +606,8 @@ private CommitProcessFactory createCommitProcessFactory( Dependencies dependenci TransactionCommitProcess.class ); CommitProcessSwitcher commitProcessSwitcher = new CommitProcessSwitcher( transactionPropagator, - master, commitProcessDelegate, requestContextFactory, lockManager, monitors, dependencies ); + master, commitProcessDelegate, requestContextFactory, lockManager, monitors, dependencies, + config.get( GraphDatabaseSettings.release_schema_lock_while_building_constraint ) ); componentSwitcherContainer.add( commitProcessSwitcher ); return new HighlyAvailableCommitProcessFactory( commitProcessDelegate );