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 3a87b9a79daae..7c213236da549 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java @@ -75,8 +75,8 @@ import org.neo4j.kernel.impl.core.PropertyKeyTokenHolder; import org.neo4j.kernel.impl.core.RelationshipTypeTokenHolder; import org.neo4j.kernel.impl.core.StartupStatisticsProvider; -import org.neo4j.kernel.impl.factory.GraphDatabaseFacadeFactory; import org.neo4j.kernel.impl.factory.AccessCapability; +import org.neo4j.kernel.impl.factory.GraphDatabaseFacadeFactory; import org.neo4j.kernel.impl.index.IndexConfigStore; import org.neo4j.kernel.impl.index.LegacyIndexStore; import org.neo4j.kernel.impl.locking.LockService; @@ -84,7 +84,6 @@ import org.neo4j.kernel.impl.locking.StatementLocksFactory; import org.neo4j.kernel.impl.logging.LogService; import org.neo4j.kernel.impl.proc.Procedures; -import org.neo4j.storageengine.api.StoreFileMetadata; import org.neo4j.kernel.impl.storageengine.impl.recordstorage.RecordStorageEngine; import org.neo4j.kernel.impl.store.MetaDataStore; import org.neo4j.kernel.impl.store.StoreId; @@ -159,6 +158,7 @@ import org.neo4j.logging.LogProvider; import org.neo4j.logging.Logger; import org.neo4j.storageengine.api.StorageEngine; +import org.neo4j.storageengine.api.StoreFileMetadata; import org.neo4j.storageengine.api.StoreReadLayer; import static org.neo4j.kernel.impl.transaction.log.pruning.LogPruneStrategyFactory.fromConfigValue; @@ -1057,6 +1057,12 @@ public void afterModeSwitch() clearTransactions(); } + // For test purposes only, not thread safe + public LifeSupport getLife() + { + return life; + } + @SuppressWarnings( "deprecation" ) public abstract static class Configuration { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java index 5ce4d0b3419b7..db94d41c80cd1 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java @@ -358,8 +358,10 @@ private void awaitOnline( IndexProxy proxy ) throws InterruptedException } } + // We need to stop indexing service on shutdown since we can have transactions that are ongoing/finishing + // after we start stopping components and those transactions should be able to finish successfully @Override - public void stop() + public void shutdown() { state = State.STOPPED; samplingController.stop(); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java index c781b7b056bfd..605ce62224a26 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java @@ -324,6 +324,8 @@ public void shouldLogIndexStateOnInit() throws Exception // given SchemaIndexProvider provider = mock( SchemaIndexProvider.class ); when( provider.getProviderDescriptor() ).thenReturn( PROVIDER_DESCRIPTOR ); + when( provider.getOnlineAccessor( anyLong(), any( IndexConfiguration.class ), any( IndexSamplingConfig.class ) ) ) + .thenReturn( mock( IndexAccessor.class ) ); SchemaIndexProviderMap providerMap = new DefaultSchemaIndexProviderMap( provider ); TokenNameLookup mockLookup = mock( TokenNameLookup.class ); @@ -534,13 +536,13 @@ public void shouldLogTriggerSamplingOnAnIndexes() throws Exception } @Test - public void applicationOfIndexUpdatesShouldThrowIfServiceIsStopped() + public void applicationOfIndexUpdatesShouldThrowIfServiceIsShutdown() throws IOException, IndexEntryConflictException { // Given IndexingService indexingService = newIndexingServiceWithMockedDependencies( populator, accessor, withData() ); life.start(); - life.stop(); + life.shutdown(); try { diff --git a/community/neo4j/src/test/java/org/neo4j/index/IndexFailureOnStartupTest.java b/community/neo4j/src/test/java/org/neo4j/index/IndexFailureOnStartupTest.java index f6e0002f1adb6..a1e600cb48089 100644 --- a/community/neo4j/src/test/java/org/neo4j/index/IndexFailureOnStartupTest.java +++ b/community/neo4j/src/test/java/org/neo4j/index/IndexFailureOnStartupTest.java @@ -26,6 +26,7 @@ import java.io.File; import java.io.IOException; import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Transaction; @@ -114,7 +115,7 @@ public void shouldArchiveFailedIndex() throws Exception assertThat( archiveFile(), nullValue() ); // when - db.restartDatabase( new DeleteIndexFile( "segments_1" ) ); + db.restartDatabase( new DeleteIndexFile( "segments_" ) ); // then indexStateShouldBe( equalTo( ONLINE ) ); @@ -176,22 +177,19 @@ private void createNamed( Label label, String name ) private static class DeleteIndexFile implements DatabaseRule.RestartAction { - private final String source; + private final String prefix; - DeleteIndexFile( String source ) + DeleteIndexFile( String prefix ) { - this.source = source; + this.prefix = prefix; } @Override public void run( FileSystemAbstraction fs, File base ) throws IOException { - File fileToDelete = new File( new File( soleIndexDir( fs, base ), "1" ), source ); - if ( !fs.fileExists( fileToDelete ) ) - { - throw new AssertionError( fileToDelete + " does not exist" ); - } - fs.deleteFile( fileToDelete ); + File indexRootDirectory = new File( soleIndexDir( fs, base ), "1" ); + File[] files = fs.listFiles( indexRootDirectory, ( dir, name ) -> name.startsWith( prefix ) ); + Stream.of(files).forEach( fs::deleteFile ); } } diff --git a/community/neo4j/src/test/java/org/neo4j/index/ShutdownOnIndexUpdateIT.java b/community/neo4j/src/test/java/org/neo4j/index/ShutdownOnIndexUpdateIT.java new file mode 100644 index 0000000000000..8f9b09bcfcf0e --- /dev/null +++ b/community/neo4j/src/test/java/org/neo4j/index/ShutdownOnIndexUpdateIT.java @@ -0,0 +1,145 @@ +/* + * Copyright (c) 2002-2016 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.index; + +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; + +import org.neo4j.graphdb.DependencyResolver; +import org.neo4j.graphdb.GraphDatabaseService; +import org.neo4j.graphdb.Label; +import org.neo4j.graphdb.Node; +import org.neo4j.graphdb.Transaction; +import org.neo4j.graphdb.factory.GraphDatabaseFactory; +import org.neo4j.graphdb.schema.Schema; +import org.neo4j.kernel.NeoStoreDataSource; +import org.neo4j.kernel.impl.storageengine.impl.recordstorage.RecordStorageEngine; +import org.neo4j.kernel.internal.GraphDatabaseAPI; +import org.neo4j.kernel.lifecycle.LifeSupport; +import org.neo4j.kernel.lifecycle.LifecycleListener; +import org.neo4j.kernel.lifecycle.LifecycleStatus; +import org.neo4j.test.rule.CleanupRule; +import org.neo4j.test.rule.TestDirectory; + +import static org.junit.Assert.assertTrue; + +public class ShutdownOnIndexUpdateIT +{ + private static final String UNIQUE_PROPERTY_NAME = "uniquePropertyName"; + private static final AtomicLong indexProvider = new AtomicLong(); + private static Label constraintIndexLabel = Label.label( "ConstraintIndexLabel" ); + + @Rule + public TestDirectory testDirectory = TestDirectory.testDirectory(); + @Rule + public CleanupRule cleanupRule = new CleanupRule(); + + private ExecutorService executors; + + @Before + public void setUp() + { + executors = Executors.newCachedThreadPool(); + } + + @After + public void tearDown() + { + executors.shutdown(); + } + + @Test + public void shutdownWhileFinishingTransactionWithIndexUpdates() throws Exception + { + GraphDatabaseService database = new GraphDatabaseFactory().newEmbeddedDatabase( testDirectory.graphDbDir() ); + cleanupRule.add( database ); + + createConstraint( database ); + waitIndexesOnline( database ); + + try ( Transaction transaction = database.beginTx() ) + { + Node node = database.createNode( constraintIndexLabel ); + node.setProperty( UNIQUE_PROPERTY_NAME, indexProvider.getAndIncrement() ); + + DependencyResolver dependencyResolver = ((GraphDatabaseAPI) database).getDependencyResolver(); + NeoStoreDataSource dataSource = dependencyResolver.resolveDependency( NeoStoreDataSource.class ); + LifeSupport dataSourceLife = dataSource.getLife(); + TransactionCloseListener closeListener = new TransactionCloseListener( transaction ); + dataSourceLife.addLifecycleListener( closeListener ); + dataSource.stop(); + + assertTrue( "Transaction should be closed and no exception should be thrown.", + closeListener.isTransactionClosed() ); + } + } + + private void waitIndexesOnline( GraphDatabaseService database ) + { + try ( Transaction ignored = database.beginTx() ) + { + database.schema().awaitIndexesOnline( 5, TimeUnit.MINUTES ); + } + } + + private void createConstraint( GraphDatabaseService database ) + { + try ( Transaction transaction = database.beginTx() ) + { + Schema schema = database.schema(); + schema.constraintFor( constraintIndexLabel ).assertPropertyIsUnique( UNIQUE_PROPERTY_NAME ).create(); + transaction.success(); + } + } + + private static class TransactionCloseListener implements LifecycleListener + { + private final Transaction transaction; + private boolean transactionClosed = false; + + TransactionCloseListener( Transaction transaction ) + { + this.transaction = transaction; + } + + @Override + public void notifyStatusChanged( Object instance, LifecycleStatus from, LifecycleStatus to ) + { + if ( (LifecycleStatus.STOPPED == to) && (instance instanceof RecordStorageEngine) ) + { + transaction.success(); + transaction.close(); + transactionClosed = true; + } + } + + boolean isTransactionClosed() + { + return transactionClosed; + } + } +}