From cc73c11c3623756a82319e379ffb8023926003be Mon Sep 17 00:00:00 2001 From: MishaDemianenko Date: Wed, 13 Jun 2018 16:11:21 +0200 Subject: [PATCH] NeoStoreDatasource dependencies lifecycle. Simplify possible possible parent-child relationships in dependency resolver. Now its not possible to setup parent dependency resolver over supplier. Use platform dependency resolver as parent of neo store datasource resolver. --- .../org/neo4j/kernel/NeoStoreDataSource.java | 68 +++++++++--------- .../impl/api/StatementOperationParts.java | 29 +++----- .../transaction/state/DataSourceManager.java | 22 ------ .../neo4j/kernel/impl/util/Dependencies.java | 51 ++++---------- .../state/DependencyResolverSupplierTest.java | 69 ------------------- .../graphdb/facade/spi/ClassicCoreSPI.java | 2 +- .../factory/module/PlatformModule.java | 6 +- .../org/neo4j/server/AbstractNeoServer.java | 16 ++--- 8 files changed, 61 insertions(+), 202 deletions(-) delete mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/DependencyResolverSupplierTest.java 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 465079a0d939..213a68f39ede 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java @@ -154,6 +154,7 @@ import org.neo4j.storageengine.api.StoreFileMetadata; import org.neo4j.time.SystemNanoClock; import org.neo4j.util.FeatureToggles; +import org.neo4j.util.VisibleForTesting; import static org.neo4j.helpers.Exceptions.throwIfUnchecked; @@ -255,7 +256,7 @@ boolean applicable( DiagnosticsPhase phase ) private final StoreCopyCheckPointMutex storeCopyCheckPointMutex; private final CollectionsFactorySupplier collectionsFactorySupplier; - private Dependencies dependencies; + private Dependencies dataSourceDependencies; private LifeSupport life; private IndexProviderMap indexProviderMap; private File storeDir; @@ -362,7 +363,7 @@ public void init() @Override public void start() throws IOException { - dependencies = new Dependencies(); + dataSourceDependencies = new Dependencies( dependencyResolver ); life = new LifeSupport(); life.add( recoveryCleanupWorkCollector ); @@ -375,11 +376,11 @@ public void start() throws IOException indexProviderMap = new DefaultIndexProviderMap( defaultIndexProvider, indexProviderSelection.lowerPrioritizedCandidates() ); - dependencies.satisfyDependency( indexProviderMap ); + dataSourceDependencies.satisfyDependency( indexProviderMap ); IndexConfigStore indexConfigStore = new IndexConfigStore( storeDir, fs ); - dependencies.satisfyDependency( lockService ); - dependencies.satisfyDependency( indexConfigStore ); + dataSourceDependencies.satisfyDependency( lockService ); + dataSourceDependencies.satisfyDependency( indexConfigStore ); life.add( indexConfigStore ); life.add( Lifecycles.multiple( indexProviders.values() ) ); @@ -391,7 +392,7 @@ public void start() throws IOException .withLogEntryReader( logEntryReader ) .withLogFileMonitor( physicalLogMonitor ) .withConfig( config ) - .withDependencies( dependencies ).build(); + .withDependencies( dataSourceDependencies ).build(); LogTailScanner tailScanner = new LogTailScanner( logFiles, logEntryReader, monitors, failOnCorruptedLogFiles ); monitors.addMonitorListener( @@ -410,11 +411,9 @@ public void start() throws IOException { DatabaseSchemaState databaseSchemaState = new DatabaseSchemaState( logProvider ); - SynchronizedArrayIdOrderingQueue explicitIndexTransactionOrdering = - new SynchronizedArrayIdOrderingQueue( 20 ); + SynchronizedArrayIdOrderingQueue explicitIndexTransactionOrdering = new SynchronizedArrayIdOrderingQueue( 20 ); - Supplier transactionsSnapshotSupplier = - () -> kernelModule.kernelTransactions().get(); + Supplier transactionsSnapshotSupplier = () -> kernelModule.kernelTransactions().get(); idController.initialize( transactionsSnapshotSupplier ); storageEngine = buildStorageEngine( @@ -423,14 +422,14 @@ public void start() throws IOException versionContextSupplier ); life.add( logFiles ); - TransactionIdStore transactionIdStore = dependencies.resolveDependency( TransactionIdStore.class ); + TransactionIdStore transactionIdStore = dataSourceDependencies.resolveDependency( TransactionIdStore.class ); versionContextSupplier.init( transactionIdStore::getLastClosedTransactionId ); - LogVersionRepository logVersionRepository = dependencies.resolveDependency( LogVersionRepository.class ); + LogVersionRepository logVersionRepository = dataSourceDependencies.resolveDependency( LogVersionRepository.class ); NeoStoreTransactionLogModule transactionLogModule = buildTransactionLogs( logFiles, config, logProvider, scheduler, storageEngine, logEntryReader, explicitIndexTransactionOrdering, transactionIdStore ); - transactionLogModule.satisfyDependencies( dependencies ); + transactionLogModule.satisfyDependencies( dataSourceDependencies ); buildRecovery( fs, transactionIdStore, @@ -442,32 +441,32 @@ public void start() throws IOException ); // At the time of writing this comes from the storage engine (IndexStoreView) - NodePropertyAccessor nodePropertyAccessor = dependencies.resolveDependency( NodePropertyAccessor.class ); + NodePropertyAccessor nodePropertyAccessor = dataSourceDependencies.resolveDependency( NodePropertyAccessor.class ); final NeoStoreKernelModule kernelModule = buildKernel( logFiles, transactionLogModule.transactionAppender(), - dependencies.resolveDependency( IndexingService.class ), + dataSourceDependencies.resolveDependency( IndexingService.class ), databaseSchemaState, - dependencies.resolveDependency( LabelScanStore.class ), + dataSourceDependencies.resolveDependency( LabelScanStore.class ), storageEngine, indexConfigStore, transactionIdStore, availabilityGuard, clock, nodePropertyAccessor ); - kernelModule.satisfyDependencies( dependencies ); + kernelModule.satisfyDependencies( dataSourceDependencies ); // Do these assignments last so that we can ensure no cyclical dependencies exist this.storageEngine = storageEngine; this.transactionLogModule = transactionLogModule; this.kernelModule = kernelModule; - dependencies.satisfyDependency( this ); - dependencies.satisfyDependency( databaseSchemaState ); - dependencies.satisfyDependency( logEntryReader ); - dependencies.satisfyDependency( storageEngine ); - dependencies.satisfyDependency( explicitIndexProviderLookup ); + dataSourceDependencies.satisfyDependency( this ); + dataSourceDependencies.satisfyDependency( databaseSchemaState ); + dataSourceDependencies.satisfyDependency( logEntryReader ); + dataSourceDependencies.satisfyDependency( storageEngine ); + dataSourceDependencies.satisfyDependency( explicitIndexProviderLookup ); } catch ( Throwable e ) { @@ -562,7 +561,7 @@ private StorageEngine buildStorageEngine( // We pretend that the storage engine abstract hides all details within it. Whereas that's mostly // true it's not entirely true for the time being. As long as we need this call below, which // makes available one or more internal things to the outside world, there are leaks to plug. - storageEngine.satisfyDependencies( dependencies ); + storageEngine.satisfyDependencies( dataSourceDependencies ); return life.add( storageEngine ); } @@ -649,10 +648,8 @@ private NeoStoreKernelModule buildKernel( LogFiles logFiles, TransactionAppender ExplicitIndexStore explicitIndexStore = new ExplicitIndexStore( config, indexConfigStore, kernelProvider, explicitIndexProviderLookup ); - StatementOperationParts statementOperationParts = dependencies.satisfyDependency( - buildStatementOperations( - cpuClockRef, - heapAllocationRef ) ); + StatementOperationParts statementOperationParts = dataSourceDependencies.satisfyDependency( + buildStatementOperations( cpuClockRef, heapAllocationRef ) ); TransactionHooks hooks = new TransactionHooks(); @@ -674,7 +671,7 @@ private NeoStoreKernelModule buildKernel( LogFiles logFiles, TransactionAppender final NeoStoreFileListing fileListing = new NeoStoreFileListing( storeDir, logFiles, labelScanStore, indexingService, explicitIndexProviderLookup, storageEngine ); - dependencies.satisfyDependency( fileListing ); + dataSourceDependencies.satisfyDependency( fileListing ); return new NeoStoreKernelModule( transactionCommitProcess, kernel, kernelTransactions, fileListing ); } @@ -721,7 +718,7 @@ private void buildTransactionMonitor( KernelTransactions kernelTransactions, Clo { KernelTransactionTimeoutMonitor kernelTransactionTimeoutMonitor = new KernelTransactionTimeoutMonitor( kernelTransactions, clock, logService ); - dependencies.satisfyDependency( kernelTransactionTimeoutMonitor ); + dataSourceDependencies.satisfyDependency( kernelTransactionTimeoutMonitor ); KernelTransactionMonitorScheduler transactionMonitorScheduler = new KernelTransactionMonitorScheduler( kernelTransactionTimeoutMonitor, scheduler, config.get( GraphDatabaseSettings.transaction_monitor_check_interval ).toMillis() ); @@ -803,14 +800,12 @@ public InwardKernel getKernel() public ResourceIterator listStoreFiles( boolean includeLogs ) throws IOException { - if ( includeLogs ) + NeoStoreFileListing.StoreFileListingBuilder fileListingBuilder = getNeoStoreFileListing().builder(); + if ( !includeLogs ) { - return getNeoStoreFileListing().builder().build(); - } - else - { - return getNeoStoreFileListing().builder().excludeLogFiles().build(); + fileListingBuilder.excludeLogFiles(); } + return fileListingBuilder.build(); } public NeoStoreFileListing getNeoStoreFileListing() @@ -826,7 +821,7 @@ public void registerDiagnosticsWith( DiagnosticsManager manager ) public DependencyResolver getDependencyResolver() { - return dependencies; + return dataSourceDependencies; } private StatementOperationParts buildStatementOperations( AtomicReference cpuClockRef, @@ -888,6 +883,7 @@ public StoreCopyCheckPointMutex getStoreCopyCheckPointMutex() } // For test purposes only, not thread safe + @VisibleForTesting public LifeSupport getLife() { return life; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StatementOperationParts.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StatementOperationParts.java index 3f54d386b12f..cfb52134ef35 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StatementOperationParts.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StatementOperationParts.java @@ -19,41 +19,28 @@ */ package org.neo4j.kernel.impl.api; +import java.util.Objects; + import org.neo4j.kernel.impl.api.operations.QueryRegistrationOperations; +import static org.apache.commons.lang3.ObjectUtils.firstNonNull; + public class StatementOperationParts { private final QueryRegistrationOperations queryRegistrationOperations; - public StatementOperationParts( - QueryRegistrationOperations queryRegistrationOperations ) + public StatementOperationParts( QueryRegistrationOperations queryRegistrationOperations ) { this.queryRegistrationOperations = queryRegistrationOperations; } - public QueryRegistrationOperations queryRegistrationOperations() + QueryRegistrationOperations queryRegistrationOperations() { - return checkNotNull( queryRegistrationOperations, QueryRegistrationOperations.class ); + return Objects.requireNonNull( queryRegistrationOperations, "No part of type " + QueryRegistrationOperations.class.getSimpleName() + " assigned" ); } public StatementOperationParts override( QueryRegistrationOperations queryRegistrationOperations ) { - return new StatementOperationParts( - eitherOr( queryRegistrationOperations, this.queryRegistrationOperations, QueryRegistrationOperations.class ) ); - } - - private T checkNotNull( T object, Class cls ) - { - if ( object == null ) - { - throw new IllegalStateException( "No part of type " + cls.getSimpleName() + " assigned" ); - } - return object; - } - - private T eitherOr( T first, T other, - @SuppressWarnings( "UnusedParameters"/*used as type flag*/ ) Class cls ) - { - return first != null ? first : other; + return new StatementOperationParts( firstNonNull( queryRegistrationOperations, this.queryRegistrationOperations ) ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/DataSourceManager.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/DataSourceManager.java index 07e7aebb9ff1..2caa05a612fe 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/DataSourceManager.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/DataSourceManager.java @@ -21,7 +21,6 @@ import java.util.function.Supplier; -import org.neo4j.graphdb.DependencyResolver; import org.neo4j.helpers.Listeners; import org.neo4j.internal.kernel.api.Kernel; import org.neo4j.kernel.NeoStoreDataSource; @@ -132,25 +131,4 @@ public Kernel get() { return dataSource.getKernel(); } - - public static class DependencyResolverSupplier implements Supplier - { - private DataSourceManager dataSourceManager; - - public DependencyResolverSupplier( DataSourceManager dataSourceManager ) - { - this.dataSourceManager = dataSourceManager; - } - - @Override - public DependencyResolver get() - { - NeoStoreDataSource dataSource = dataSourceManager.getDataSource(); - if ( dataSource == null ) - { - return null; - } - return dataSource.getDependencyResolver(); - } - } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/Dependencies.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/Dependencies.java index f49c86bbcd5c..257ba5c22a28 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/Dependencies.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/Dependencies.java @@ -19,10 +19,11 @@ */ package org.neo4j.kernel.impl.util; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import org.eclipse.collections.api.RichIterable; +import org.eclipse.collections.api.multimap.list.MutableListMultimap; +import org.eclipse.collections.impl.factory.Multimaps; + +import java.util.Objects; import java.util.function.Supplier; import org.neo4j.graphdb.DependencyResolver; @@ -30,8 +31,8 @@ @SuppressWarnings( "unchecked" ) public class Dependencies extends DependencyResolver.Adapter implements DependencySatisfier { - private final Supplier parent; - private final Map, List> typeDependencies = new HashMap<>(); + private final DependencyResolver parent; + private final MutableListMultimap typeDependencies = Multimaps.mutable.list.empty(); public Dependencies() { @@ -40,33 +41,23 @@ public Dependencies() public Dependencies( final DependencyResolver parent ) { - this.parent = () -> parent; - } - - public Dependencies( Supplier parent ) - { + Objects.requireNonNull( parent ); this.parent = parent; } @Override public T resolveDependency( Class type, SelectionStrategy selector ) { - List options = typeDependencies.get( type ); - - if ( options != null ) + RichIterable options = typeDependencies.get( type ); + if ( options.notEmpty() ) { - return selector.select( type, (Iterable) options); + return selector.select( type, (Iterable) options ); } // Try parent if ( parent != null ) { - DependencyResolver dependencyResolver = parent.get(); - - if ( dependencyResolver != null ) - { - return dependencyResolver.resolveDependency( type, selector ); - } + return parent.resolveDependency( type, selector ); } // Out of options @@ -92,13 +83,7 @@ public T satisfyDependency( T dependency ) Class type = dependency.getClass(); do { - List deps = (List) typeDependencies.get( type ); - if ( deps == null ) - { - deps = new ArrayList<>( ); - typeDependencies.put(type, deps); - } - deps.add( dependency ); + typeDependencies.put( type, dependency ); // Add as all interfaces Class[] interfaces = type.getInterfaces(); @@ -123,15 +108,7 @@ private void addInterfaces( Class[] interfaces, T dependency ) { for ( Class type : interfaces ) { - List deps = (List) typeDependencies.get( type ); - if ( deps == null ) - { - deps = new ArrayList<>( ); - typeDependencies.put(type, deps); - } - deps.add( dependency ); - - // Add as all sub-interfaces + typeDependencies.put( type, dependency ); addInterfaces(type.getInterfaces(), dependency); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/DependencyResolverSupplierTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/DependencyResolverSupplierTest.java deleted file mode 100644 index df9726669583..000000000000 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/DependencyResolverSupplierTest.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright (c) 2002-2018 "Neo4j," - * Neo4j Sweden AB [http://neo4j.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.kernel.impl.transaction.state; - - -import org.junit.Test; - -import org.neo4j.graphdb.DependencyResolver; -import org.neo4j.kernel.NeoStoreDataSource; -import org.neo4j.kernel.impl.transaction.state.DataSourceManager.DependencyResolverSupplier; - -import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class DependencyResolverSupplierTest -{ - @Test - public void shouldReturnTheDependencyResolveFromTheRegisteredDatasource() - { - // given - DataSourceManager dataSourceManager = new DataSourceManager(); - DependencyResolverSupplier supplier = new DependencyResolverSupplier( dataSourceManager ); - NeoStoreDataSource neoStoreDataSource = mock( NeoStoreDataSource.class ); - DependencyResolver dependencyResolver = mock( DependencyResolver.class ); - when( neoStoreDataSource.getDependencyResolver() ).thenReturn( dependencyResolver ); - - // when - dataSourceManager.register( neoStoreDataSource ); - - // then - assertEquals( dependencyResolver, supplier.get() ); - } - - @Test - public void shouldReturnNullIfDataSourceHasBeenUnregistered() - { - // given - DataSourceManager dataSourceManager = new DataSourceManager(); - DependencyResolverSupplier supplier = new DependencyResolverSupplier( dataSourceManager ); - NeoStoreDataSource neoStoreDataSource = mock( NeoStoreDataSource.class ); - DependencyResolver dependencyResolver = mock( DependencyResolver.class ); - when( neoStoreDataSource.getDependencyResolver() ).thenReturn( dependencyResolver ); - dataSourceManager.register( neoStoreDataSource ); - - // when - dataSourceManager.unregister( neoStoreDataSource ); - - // then - assertEquals( null, supplier.get() ); - } -} diff --git a/community/neo4j/src/main/java/org/neo4j/graphdb/facade/spi/ClassicCoreSPI.java b/community/neo4j/src/main/java/org/neo4j/graphdb/facade/spi/ClassicCoreSPI.java index f96df1a0f549..2f655ae88933 100644 --- a/community/neo4j/src/main/java/org/neo4j/graphdb/facade/spi/ClassicCoreSPI.java +++ b/community/neo4j/src/main/java/org/neo4j/graphdb/facade/spi/ClassicCoreSPI.java @@ -95,7 +95,7 @@ public AutoIndexing autoIndexing() @Override public DependencyResolver resolver() { - return platform.dependencies; + return dataSource.neoStoreDataSource.getDependencyResolver(); } @Override diff --git a/community/neo4j/src/main/java/org/neo4j/graphdb/factory/module/PlatformModule.java b/community/neo4j/src/main/java/org/neo4j/graphdb/factory/module/PlatformModule.java index 8a4df36ad672..43adb2b59a73 100644 --- a/community/neo4j/src/main/java/org/neo4j/graphdb/factory/module/PlatformModule.java +++ b/community/neo4j/src/main/java/org/neo4j/graphdb/factory/module/PlatformModule.java @@ -136,8 +136,7 @@ public PlatformModule( File providedStoreDir, Config config, DatabaseInfo databa { this.databaseInfo = databaseInfo; this.dataSourceManager = new DataSourceManager(); - dependencies = new org.neo4j.kernel.impl.util.Dependencies( - new DataSourceManager.DependencyResolverSupplier( dataSourceManager ) ); + dependencies = new org.neo4j.kernel.impl.util.Dependencies(); dependencies.satisfyDependency( databaseInfo ); clock = dependencies.satisfyDependency( createClock() ); @@ -192,8 +191,7 @@ public PlatformModule( File providedStoreDir, Config config, DatabaseInfo databa versionContextSupplier = createCursorContextSupplier( config ); collectionsFactorySupplier = createCollectionsFactorySupplier( config ); dependencies.satisfyDependency( versionContextSupplier ); - pageCache = dependencies.satisfyDependency( createPageCache( fileSystem, config, logging, tracers, - versionContextSupplier ) ); + pageCache = dependencies.satisfyDependency( createPageCache( fileSystem, config, logging, tracers, versionContextSupplier ) ); life.add( new PageCacheLifecycle( pageCache ) ); diff --git a/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java b/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java index 7e6525b2cdef..e45ce1fb1444 100644 --- a/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java +++ b/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java @@ -128,6 +128,7 @@ public abstract class AbstractNeoServer implements NeoServer private Optional httpsAdvertisedAddress; protected Database database; + private Dependencies dependencyResolver; protected CypherExecutor cypherExecutor; protected WebServer webServer; protected Supplier authManagerSupplier; @@ -182,8 +183,9 @@ public void init() return; } - this.database = - life.add( dependencyResolver.satisfyDependency( dbFactory.newDatabase( config, dependencies ) ) ); + this.database = dbFactory.newDatabase( config, dependencies ); + this.dependencyResolver = new Dependencies( database.getGraph().getDependencyResolver() ); + life.add( database ); this.authManagerSupplier = dependencyResolver.provideDependency( AuthManager.class ); this.userManagerSupplier = dependencyResolver.provideDependency( UserManagerSupplier.class ); @@ -490,16 +492,6 @@ protected T resolveDependency( Class type ) return dependencyResolver.resolveDependency( type ); } - private final Dependencies dependencyResolver = new Dependencies( new Supplier() - { - @Override - public DependencyResolver get() - { - Database db = dependencyResolver.resolveDependency( Database.class ); - return db.getGraph().getDependencyResolver(); - } - } ); - private class ServerComponentsLifecycleAdapter extends LifecycleAdapter { @Override