Add additional tests for id components constructed during edition cre…

…ation. Update dependency resolver to include only one id generation factory.
neo4j · Jul 4, 2017 · 7d0b6d9 · 7d0b6d9
1 parent da8f3fb
commit 7d0b6d9
Show file tree

Hide file tree

Showing 15 changed files with 250 additions and 9 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/CommunityEditionModule.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/CommunityEditionModule.java
@@ -109,6 +109,8 @@ public CommunityEditionModule( PlatformModule platformModule )
         eligibleForIdReuse = IdReuseEligibility.ALWAYS;
 
         createIdComponents( platformModule, dependencies, createIdGeneratorFactory( fileSystem, idTypeConfigurationProvider ) );
+        dependencies.satisfyDependency( idGeneratorFactory );
+        dependencies.satisfyDependency( idController );
 
         propertyKeyTokenHolder = life.add( dependencies.satisfyDependency( new DelegatingPropertyKeyTokenHolder(
                 createPropertyKeyCreator( config, dataSourceManager, idGeneratorFactory ) ) ) );

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/EditionModule.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/EditionModule.java
@@ -253,7 +253,6 @@ protected void createIdComponents( PlatformModule platformModule, Dependencies d
             idController = createDefaultIdController();
         }
         this.idGeneratorFactory = factory;
-        dependencies.satisfyDependency( factory );
     }
 
     private BufferedIdController createBufferedIdController( BufferingIdGeneratorFactory idGeneratorFactory,

diff --git a/...main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java b/...main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java
@@ -387,7 +387,6 @@ public void satisfyDependencies( DependencySatisfier satisfier )
         // providing TransactionIdStore, LogVersionRepository
         satisfier.satisfyDependency( neoStores.getMetaDataStore() );
         satisfier.satisfyDependency( indexStoreView );
-        satisfier.satisfyDependency( idController );
     }
 
     @Override

diff --git a/...l/factory/CommunityEditionModuleTest.java → ...ommunityEditionModuleIntegrationTest.java b/...l/factory/CommunityEditionModuleTest.java → ...ommunityEditionModuleIntegrationTest.java
@@ -19,21 +19,55 @@
  */
 package org.neo4j.kernel.impl.factory;
 
+import org.junit.Rule;
 import org.junit.Test;
 
 import java.util.function.Predicate;
 
+import org.neo4j.graphdb.DependencyResolver;
+import org.neo4j.graphdb.factory.GraphDatabaseFactory;
 import org.neo4j.kernel.impl.index.IndexConfigStore;
+import org.neo4j.kernel.impl.storageengine.impl.recordstorage.id.BufferedIdController;
+import org.neo4j.kernel.impl.storageengine.impl.recordstorage.id.IdController;
 import org.neo4j.kernel.impl.store.MetaDataStore;
 import org.neo4j.kernel.impl.store.StoreFile;
+import org.neo4j.kernel.impl.store.id.BufferingIdGeneratorFactory;
+import org.neo4j.kernel.impl.store.id.IdGeneratorFactory;
 import org.neo4j.kernel.impl.storemigration.StoreFileType;
 import org.neo4j.kernel.impl.transaction.log.PhysicalLogFile;
+import org.neo4j.kernel.internal.GraphDatabaseAPI;
+import org.neo4j.test.rule.TestDirectory;
 
+import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
-public class CommunityEditionModuleTest
+public class CommunityEditionModuleIntegrationTest
 {
+    @Rule
+    public TestDirectory testDirectory = TestDirectory.testDirectory();
+
+    @Test
+    public void createBufferedIdComponentsByDefault()
+    {
+        GraphDatabaseAPI database =
+                (GraphDatabaseAPI) new GraphDatabaseFactory().newEmbeddedDatabase( testDirectory.graphDbDir() );
+        try
+        {
+            DependencyResolver dependencyResolver = database.getDependencyResolver();
+            IdController idController = dependencyResolver.resolveDependency( IdController.class );
+            IdGeneratorFactory idGeneratorFactory = dependencyResolver.resolveDependency( IdGeneratorFactory.class );
+
+            assertThat( idController, instanceOf( BufferedIdController.class ) );
+            assertThat( idGeneratorFactory, instanceOf( BufferingIdGeneratorFactory.class ) );
+        }
+        finally
+        {
+            database.shutdown();
+        }
+    }
+
     @Test
     public void fileWatcherFileNameFilter()
     {

diff --git a/...clustering/src/main/java/org/neo4j/causalclustering/core/EnterpriseCoreEditionModule.java b/...clustering/src/main/java/org/neo4j/causalclustering/core/EnterpriseCoreEditionModule.java
@@ -231,6 +231,8 @@ public void registerEditionSpecificProcedures( Procedures procedures ) throws Ke
         this.idTypeConfigurationProvider = coreStateMachinesModule.idTypeConfigurationProvider;
 
         createIdComponents( platformModule, dependencies, coreStateMachinesModule.idGeneratorFactory );
+        dependencies.satisfyDependency( idGeneratorFactory );
+        dependencies.satisfyDependency( idController );
 
         this.labelTokenHolder = coreStateMachinesModule.labelTokenHolder;
         this.propertyKeyTokenHolder = coreStateMachinesModule.propertyKeyTokenHolder;

diff --git a/...src/main/java/org/neo4j/causalclustering/core/state/machines/CoreStateMachinesModule.java b/...src/main/java/org/neo4j/causalclustering/core/state/machines/CoreStateMachinesModule.java
@@ -139,9 +139,8 @@ public CoreStateMachinesModule( MemberId myself, PlatformModule platformModule,
         idTypeConfigurationProvider = new EnterpriseIdTypeConfigurationProvider( config );
         CommandIndexTracker commandIndexTracker = new CommandIndexTracker();
         freeIdCondition = new IdReusabilityCondition( commandIndexTracker, raftMachine, myself );
-        this.idGeneratorFactory = dependencies.satisfyDependency( createIdGeneratorFactory( fileSystem,
-                idRangeAcquirer, logProvider,
-                idTypeConfigurationProvider ) );
+        this.idGeneratorFactory =
+                createIdGeneratorFactory( fileSystem, idRangeAcquirer, logProvider, idTypeConfigurationProvider );
 
         dependencies.satisfyDependency( new IdBasedStoreEntityCounters( this.idGeneratorFactory ) );
 

diff --git a/...ain/java/org/neo4j/causalclustering/core/state/machines/id/FreeIdFilteredIdGenerator.java b/...ain/java/org/neo4j/causalclustering/core/state/machines/id/FreeIdFilteredIdGenerator.java
@@ -23,6 +23,10 @@
 
 import org.neo4j.kernel.impl.store.id.IdGenerator;
 
+/**
+ * Id generator that will perform filtering of ids to free using supplied condition.
+ * Id will be freed only if condition is true, otherwise it will be ignored.
+ */
 public class FreeIdFilteredIdGenerator extends IdGenerator.Delegate
 {
     private final BooleanSupplier freeIdCondition;

diff --git a/...a/org/neo4j/causalclustering/core/state/machines/id/FreeIdFilteredIdGeneratorFactory.java b/...a/org/neo4j/causalclustering/core/state/machines/id/FreeIdFilteredIdGeneratorFactory.java
@@ -28,7 +28,6 @@
 import org.neo4j.kernel.impl.store.id.IdGeneratorFactory;
 import org.neo4j.kernel.impl.store.id.IdType;
 
-
 public class FreeIdFilteredIdGeneratorFactory implements IdGeneratorFactory
 {
     private Map<IdType, IdGenerator> delegatedGenerator = new HashMap<>();

diff --git a/.../main/java/org/neo4j/causalclustering/readreplica/EnterpriseReadReplicaEditionModule.java b/.../main/java/org/neo4j/causalclustering/readreplica/EnterpriseReadReplicaEditionModule.java
@@ -145,6 +145,8 @@ public class EnterpriseReadReplicaEditionModule extends EditionModule
         idGeneratorFactory = dependencies
                 .satisfyDependency( new DefaultIdGeneratorFactory( fileSystem, idTypeConfigurationProvider ) );
         idController = createDefaultIdController();
+        dependencies.satisfyDependency( idGeneratorFactory );
+        dependencies.satisfyDependency( idController );
         dependencies.satisfyDependency( new IdBasedStoreEntityCounters( this.idGeneratorFactory ) );
 
         propertyKeyTokenHolder = life.add(

diff --git a/...core/EnterpriseCoreEditionModuleTest.java → ...riseCoreEditionModuleIntegrationTest.java b/...core/EnterpriseCoreEditionModuleTest.java → ...riseCoreEditionModuleIntegrationTest.java
@@ -19,22 +19,50 @@
  */
 package org.neo4j.causalclustering.core;
 
+import org.junit.Rule;
 import org.junit.Test;
 
 import java.util.function.Predicate;
 
+import org.neo4j.causalclustering.core.state.machines.id.FreeIdFilteredIdGeneratorFactory;
+import org.neo4j.causalclustering.discovery.Cluster;
+import org.neo4j.causalclustering.discovery.CoreClusterMember;
 import org.neo4j.com.storecopy.StoreUtil;
+import org.neo4j.graphdb.DependencyResolver;
 import org.neo4j.kernel.impl.index.IndexConfigStore;
+import org.neo4j.kernel.impl.storageengine.impl.recordstorage.id.BufferedIdController;
+import org.neo4j.kernel.impl.storageengine.impl.recordstorage.id.IdController;
 import org.neo4j.kernel.impl.store.MetaDataStore;
 import org.neo4j.kernel.impl.store.StoreFile;
+import org.neo4j.kernel.impl.store.id.IdGeneratorFactory;
 import org.neo4j.kernel.impl.storemigration.StoreFileType;
 import org.neo4j.kernel.impl.transaction.log.PhysicalLogFile;
+import org.neo4j.test.causalclustering.ClusterRule;
 
+import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
-public class EnterpriseCoreEditionModuleTest
+public class EnterpriseCoreEditionModuleIntegrationTest
 {
+    @Rule
+    public ClusterRule clusterRule = new ClusterRule( getClass() );
+
+    @Test
+    public void createBufferedIdComponentsByDefault() throws Exception
+    {
+        Cluster cluster = clusterRule.startCluster();
+        CoreClusterMember leader = cluster.awaitLeader();
+        DependencyResolver dependencyResolver = leader.database().getDependencyResolver();
+
+        IdController idController = dependencyResolver.resolveDependency( IdController.class );
+        IdGeneratorFactory idGeneratorFactory = dependencyResolver.resolveDependency( IdGeneratorFactory.class );
+
+        assertThat( idController, instanceOf( BufferedIdController.class ) );
+        assertThat( idGeneratorFactory, instanceOf( FreeIdFilteredIdGeneratorFactory.class ) );
+    }
+
     @Test
     public void fileWatcherFileNameFilter()
     {

diff --git a/...g/neo4j/causalclustering/core/state/machines/id/FreeIdFilteredIdGeneratorFactoryTest.java b/...g/neo4j/causalclustering/core/state/machines/id/FreeIdFilteredIdGeneratorFactoryTest.java
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2002-2017 "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 Affero 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 Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.causalclustering.core.state.machines.id;
+
+import org.junit.Test;
+
+import java.io.File;
+
+import org.neo4j.kernel.impl.store.id.IdGenerator;
+import org.neo4j.kernel.impl.store.id.IdGeneratorFactory;
+import org.neo4j.kernel.impl.store.id.IdType;
+
+import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+public class FreeIdFilteredIdGeneratorFactoryTest
+{
+    private IdGeneratorFactory idGeneratorFactory = mock( IdGeneratorFactory.class );
+    private File file = mock( File.class );
+
+    @Test
+    public void openFilteredGenerator() throws Exception
+    {
+        FreeIdFilteredIdGeneratorFactory filteredGenerator = createFilteredFactory();
+        IdType idType = IdType.NODE;
+        long highId = 1L;
+        long maxId = 10L;
+        IdGenerator idGenerator = filteredGenerator.open( file, idType, highId, maxId );
+
+        verify( idGeneratorFactory ).open( file, idType, highId, maxId );
+        assertThat( idGenerator, instanceOf( FreeIdFilteredIdGenerator.class ) );
+    }
+
+    @Test
+    public void openFilteredGeneratorWithGrabSize() throws Exception
+    {
+        FreeIdFilteredIdGeneratorFactory filteredGenerator = createFilteredFactory();
+        IdType idType = IdType.NODE;
+        long highId = 1L;
+        long maxId = 10L;
+        int grabSize = 5;
+        IdGenerator idGenerator = filteredGenerator.open( file, grabSize, idType, highId, maxId );
+
+        verify( idGeneratorFactory ).open( file, grabSize, idType, highId, maxId );
+        assertThat( idGenerator, instanceOf( FreeIdFilteredIdGenerator.class ) );
+    }
+
+    private FreeIdFilteredIdGeneratorFactory createFilteredFactory()
+    {
+        return new FreeIdFilteredIdGeneratorFactory( idGeneratorFactory, () -> true );
+    }
+}
diff --git a/...java/org/neo4j/causalclustering/core/state/machines/id/FreeIdFilteredIdGeneratorTest.java b/...java/org/neo4j/causalclustering/core/state/machines/id/FreeIdFilteredIdGeneratorTest.java
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2002-2017 "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 Affero 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 Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.causalclustering.core.state.machines.id;
+
+import org.apache.commons.lang3.mutable.MutableBoolean;
+import org.junit.Test;
+
+import java.util.function.BooleanSupplier;
+
+import org.neo4j.kernel.impl.store.id.IdGenerator;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+public class FreeIdFilteredIdGeneratorTest
+{
+
+    private IdGenerator idGenerator = mock( IdGenerator.class );
+
+    @Test
+    public void freeIdIfConditionSatisfied() throws Exception
+    {
+        FreeIdFilteredIdGenerator generator = createFilteredIdGenerator( idGenerator, () -> true );
+        generator.freeId( 1 );
+
+        verify( idGenerator ).freeId( 1 );
+    }
+
+    @Test
+    public void skipFreeIdIfConditionIsNotSatisfied()
+    {
+        FreeIdFilteredIdGenerator generator = createFilteredIdGenerator( idGenerator, () -> false );
+        generator.freeId( 1 );
+
+        verifyZeroInteractions( idGenerator );
+    }
+
+    @Test
+    public void freeIdOnlyWhenConditionSatisfied()
+    {
+        MutableBoolean condition = new MutableBoolean();
+        FreeIdFilteredIdGenerator generator = createFilteredIdGenerator( idGenerator, condition::booleanValue );
+        generator.freeId( 1 );
+        condition.setTrue();
+        generator.freeId( 2 );
+
+        verify( idGenerator ).freeId( 2 );
+    }
+
+    private FreeIdFilteredIdGenerator createFilteredIdGenerator( IdGenerator idGenerator,
+            BooleanSupplier booleanSupplier )
+    {
+        return new FreeIdFilteredIdGenerator( idGenerator, booleanSupplier );
+    }
+}
diff --git a/...ausal-clustering/src/test/java/org/neo4j/causalclustering/scenarios/ClusterIdReuseIT.java b/...ausal-clustering/src/test/java/org/neo4j/causalclustering/scenarios/ClusterIdReuseIT.java
@@ -144,6 +144,7 @@ public void reusePreviouslyFreedIds() throws Exception
 
         assumeTrue( creationLeader != null && creationLeader.equals( deletionLeader ) );
         IdGeneratorFactory idGeneratorFactory = resolveDependency( creationLeader, IdGeneratorFactory.class );
+        idMaintenanceOnLeader( creationLeader );
         IdGenerator creationLeaderIdGenerator = idGeneratorFactory.get( IdType.NODE );
         assertEquals( 2, creationLeaderIdGenerator.getDefragCount() );
 

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
@@ -367,6 +367,8 @@ public void elected( String role, InstanceId instanceId, URI electedMember )
                 logging.getInternalLogProvider(), requestContextFactory, fs );
         eligibleForIdReuse = new HaIdReuseEligibility( members, platformModule.clock, idReuseSafeZone );
         createIdComponents( platformModule, dependencies, editionIdGeneratorFactory );
+        dependencies.satisfyDependency( idGeneratorFactory );
+        dependencies.satisfyDependency( idController );
         dependencies.satisfyDependency( new IdBasedStoreEntityCounters( this.idGeneratorFactory ) );
 
         // TODO There's a cyclical dependency here that should be fixed

diff --git a/...ory/HighlyAvailableEditionModuleTest.java → ...vailableEditionModuleIntegrationTest.java b/...ory/HighlyAvailableEditionModuleTest.java → ...vailableEditionModuleIntegrationTest.java
@@ -19,22 +19,48 @@
  */
 package org.neo4j.kernel.ha.factory;
 
+import org.junit.Rule;
 import org.junit.Test;
 
 import java.util.function.Predicate;
 
 import org.neo4j.com.storecopy.StoreUtil;
+import org.neo4j.graphdb.DependencyResolver;
+import org.neo4j.kernel.impl.ha.ClusterManager;
 import org.neo4j.kernel.impl.index.IndexConfigStore;
+import org.neo4j.kernel.impl.storageengine.impl.recordstorage.id.BufferedIdController;
+import org.neo4j.kernel.impl.storageengine.impl.recordstorage.id.IdController;
 import org.neo4j.kernel.impl.store.MetaDataStore;
 import org.neo4j.kernel.impl.store.StoreFile;
+import org.neo4j.kernel.impl.store.id.BufferingIdGeneratorFactory;
+import org.neo4j.kernel.impl.store.id.IdGeneratorFactory;
 import org.neo4j.kernel.impl.storemigration.StoreFileType;
 import org.neo4j.kernel.impl.transaction.log.PhysicalLogFile;
+import org.neo4j.test.ha.ClusterRule;
 
+import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
-public class HighlyAvailableEditionModuleTest
+public class HighlyAvailableEditionModuleIntegrationTest
 {
+    @Rule
+    public ClusterRule clusterRule = new ClusterRule( getClass() );
+
+    @Test
+    public void createBufferedIdComponentsByDefault() throws Exception
+    {
+        ClusterManager.ManagedCluster managedCluster = clusterRule.startCluster();
+        DependencyResolver dependencyResolver = managedCluster.getMaster().getDependencyResolver();
+
+        IdController idController = dependencyResolver.resolveDependency( IdController.class );
+        IdGeneratorFactory idGeneratorFactory = dependencyResolver.resolveDependency( IdGeneratorFactory.class );
+
+        assertThat( idController, instanceOf( BufferedIdController.class ) );
+        assertThat( idGeneratorFactory, instanceOf( BufferingIdGeneratorFactory.class ) );
+    }
+
     @Test
     public void fileWatcherFileNameFilter()
     {