Addressed PR comments

community/common/src/test/java/org/neo4j/test/rule/RepeatRule.java

@@ -28,8 +28,6 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
-import static java.lang.Integer.max;
-
 /**
  * Set a test to loop a number of times. If you find yourself using this in a production test, you are probably doing
  * something wrong.
@@ -95,9 +93,13 @@ public void evaluate() throws Throwable
     public Statement apply( Statement base, Description description )
     {
         Repeat repeat = description.getAnnotation( Repeat.class );
-        if ( repeat != null || defaultTimes > 1 )
+        if ( repeat != null )
+        {
+            return new RepeatStatement( repeat.times(), base, description );
+        }
+        if ( defaultTimes > 1 )
         {
-            return new RepeatStatement( max( repeat != null ? repeat.times() : 1, defaultTimes ), base, description );
+            return new RepeatStatement( defaultTimes, base, description );
         }
         return base;
     }

community/kernel/src/main/java/org/neo4j/kernel/api/index/SchemaIndexProvider.java

@@ -135,18 +135,18 @@ public String getPopulationFailure( long indexId ) throws IllegalStateException
     protected final int priority;
     private final Descriptor providerDescriptor;
 
+    protected SchemaIndexProvider( SchemaIndexProvider copySource )
+    {
+        this( copySource.providerDescriptor, copySource.priority );
+    }
+
     protected SchemaIndexProvider( Descriptor descriptor, int priority )
     {
         assert descriptor != null;
         this.priority = priority;
         this.providerDescriptor = descriptor;
     }
 
-    public int priority()
-    {
-        return this.priority;
-    }
-
     /**
      * Used for initially populating a created index, using batch insertion.
      */

community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java

@@ -125,6 +125,8 @@
 import org.neo4j.unsafe.impl.internal.dragons.FeatureToggles;
 
 import static org.neo4j.kernel.impl.locking.LockService.NO_LOCK_SERVICE;
+import static org.neo4j.storageengine.api.TransactionApplicationMode.RECOVERY;
+import static org.neo4j.storageengine.api.TransactionApplicationMode.REVERSE_RECOVERY;
 
 public class RecordStorageEngine implements StorageEngine, Lifecycle
 {
@@ -165,7 +167,6 @@ public class RecordStorageEngine implements StorageEngine, Lifecycle
     private final RelationshipDeleter relationshipDeleter;
     private final PropertyCreator propertyCreator;
     private final PropertyDeleter propertyDeleter;
-    private boolean started;
 
     public RecordStorageEngine(
             File storeDir,
@@ -358,7 +359,7 @@ protected BatchTransactionApplierFacade applier( TransactionApplicationMode mode
     {
         ArrayList<BatchTransactionApplier> appliers = new ArrayList<>();
         // Graph store application. The order of the decorated store appliers is irrelevant
-        appliers.add( new NeoStoreBatchTransactionApplier( mode.version(), neoStores, cacheAccess, lockService() ) );
+        appliers.add( new NeoStoreBatchTransactionApplier( mode.version(), neoStores, cacheAccess, lockService( mode ) ) );
         if ( mode.needsHighIdTracking() )
         {
             appliers.add( new HighIdBatchTransactionApplier( neoStores ) );
@@ -388,9 +389,9 @@ protected BatchTransactionApplierFacade applier( TransactionApplicationMode mode
                 appliers.toArray( new BatchTransactionApplier[appliers.size()] ) );
     }
 
-    private LockService lockService()
+    private LockService lockService( TransactionApplicationMode mode )
     {
-        return started ? lockService : NO_LOCK_SERVICE;
+        return mode == RECOVERY || mode == REVERSE_RECOVERY ? NO_LOCK_SERVICE : lockService;
     }
 
     public void satisfyDependencies( DependencySatisfier satisfier )
@@ -430,7 +431,6 @@ public void start() throws Throwable
         indexingService.start();
         labelScanStore.start();
         idController.start();
-        started = true;
     }
 
     @Override

community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplier.java

@@ -62,7 +62,6 @@ public class IndexBatchTransactionApplier extends BatchTransactionApplier.Adapte
 
     private List<NodeLabelUpdate> labelUpdates;
     private IndexUpdates indexUpdates;
-    private long txId;
 
     public IndexBatchTransactionApplier( IndexingService indexingService,
             WorkSync<Supplier<LabelScanWriter>,LabelUpdateWork> labelScanStoreSync,
@@ -80,7 +79,6 @@ public IndexBatchTransactionApplier( IndexingService indexingService,
     @Override
     public TransactionApplier startTx( CommandsToApply transaction )
     {
-        txId = transaction.transactionId();
         return transactionApplier;
     }
 

community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/NeoStoreTransactionApplier.java

@@ -26,10 +26,12 @@
 import org.neo4j.kernel.impl.locking.LockGroup;
 import org.neo4j.kernel.impl.locking.LockService;
 import org.neo4j.kernel.impl.store.NeoStores;
+import org.neo4j.kernel.impl.store.RecordStore;
 import org.neo4j.kernel.impl.store.SchemaStore;
+import org.neo4j.kernel.impl.store.record.AbstractBaseRecord;
 import org.neo4j.kernel.impl.store.record.ConstraintRule;
 import org.neo4j.kernel.impl.store.record.DynamicRecord;
-import org.neo4j.kernel.impl.store.record.PropertyRecord;
+import org.neo4j.kernel.impl.transaction.command.Command.BaseCommand;
 import org.neo4j.kernel.impl.transaction.command.Command.Version;
 
 /**
@@ -73,7 +75,7 @@ public boolean visitNodeCommand( Command.NodeCommand command ) throws IOExceptio
         lockGroup.add( lockService.acquireNodeLock( command.getKey(), LockService.LockType.WRITE_LOCK ) );
 
         // update store
-        neoStores.getNodeStore().updateRecord( version.select( command ) );
+        updateStore( neoStores.getNodeStore(), command );
         return false;
     }
 
@@ -82,7 +84,7 @@ public boolean visitRelationshipCommand( Command.RelationshipCommand command ) t
     {
         lockGroup.add( lockService.acquireRelationshipLock( command.getKey(), LockService.LockType.WRITE_LOCK ) );
 
-        neoStores.getRelationshipStore().updateRecord( version.select( command ) );
+        updateStore( neoStores.getRelationshipStore(), command );
         return false;
     }
 
@@ -99,36 +101,35 @@ else if ( command.getRelId() != -1 )
             lockGroup.add( lockService.acquireRelationshipLock( command.getRelId(), LockService.LockType.WRITE_LOCK ) );
         }
 
-        PropertyRecord select = version.select( command );
-        neoStores.getPropertyStore().updateRecord( select );
+        updateStore( neoStores.getPropertyStore(), command );
         return false;
     }
 
     @Override
     public boolean visitRelationshipGroupCommand( Command.RelationshipGroupCommand command ) throws IOException
     {
-        neoStores.getRelationshipGroupStore().updateRecord( version.select( command ) );
+        updateStore( neoStores.getRelationshipGroupStore(), command );
         return false;
     }
 
     @Override
     public boolean visitRelationshipTypeTokenCommand( Command.RelationshipTypeTokenCommand command ) throws IOException
     {
-        neoStores.getRelationshipTypeTokenStore().updateRecord( version.select( command ) );
+        updateStore( neoStores.getRelationshipTypeTokenStore(), command );
         return false;
     }
 
     @Override
     public boolean visitLabelTokenCommand( Command.LabelTokenCommand command ) throws IOException
     {
-        neoStores.getLabelTokenStore().updateRecord( version.select( command ) );
+        updateStore( neoStores.getLabelTokenStore(), command );
         return false;
     }
 
     @Override
     public boolean visitPropertyKeyTokenCommand( Command.PropertyKeyTokenCommand command ) throws IOException
     {
-        neoStores.getPropertyKeyTokenStore().updateRecord( version.select( command ) );
+        updateStore( neoStores.getPropertyKeyTokenStore(), command );
         return false;
     }
 
@@ -183,4 +184,9 @@ public boolean visitNeoStoreCommand( Command.NeoStoreCommand command ) throws IO
         neoStores.getMetaDataStore().setGraphNextProp( version.select( command ).getNextProp() );
         return false;
     }
+
+    private <RECORD extends AbstractBaseRecord> void updateStore( RecordStore<RECORD> store, BaseCommand<RECORD> command )
+    {
+        store.updateRecord( version.select( command ) );
+    }
 }

community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/log/ReversedSingleFileTransactionCursor.java

@@ -97,7 +97,7 @@ private long[] sketchOutTransactionStartOffsets() throws IOException
     @Override
     public boolean next() throws IOException
     {
-        while ( !exhausted() )
+        if ( !exhausted() )
         {
             if ( currentChunkExhausted() )
             {

community/kernel/src/main/java/org/neo4j/kernel/recovery/DefaultRecoverySPI.java

@@ -48,7 +48,6 @@ public class DefaultRecoverySPI implements Recovery.SPI
     private final StorageEngine storageEngine;
     private final TransactionIdStore transactionIdStore;
     private final LogicalTransactionStore logicalTransactionStore;
-    private TransactionQueue transactionsToApply;
 
     public DefaultRecoverySPI(
             StorageEngine storageEngine,
@@ -86,8 +85,7 @@ public void startRecovery()
     @Override
     public RecoveryApplier getRecoveryApplier( TransactionApplicationMode mode ) throws Exception
     {
-        transactionsToApply = new TransactionQueue( 100, ( first, last ) -> storageEngine.apply( first, mode ) );
-        return new RecoveryVisitor( transactionsToApply );
+        return new RecoveryVisitor( new TransactionQueue( 100, ( first, last ) -> storageEngine.apply( first, mode ) ) );
     }
 
     @Override

community/kernel/src/main/java/org/neo4j/kernel/recovery/Recovery.java

@@ -52,7 +52,7 @@ default void recoveryCompleted( int numberOfRecoveredTransactions )
         { // no-op by default
         }
 
-        default void reverseStoreRecoveryCompleted( long checkpointTxId )
+        default void reverseStoreRecoveryCompleted( long lowestRecoveredTxId )
         { // no-op by default
         }
     }
@@ -102,18 +102,18 @@ public void init() throws Throwable
         spi.startRecovery();
 
         // Backwards for neo store only
-        long checkpointTxId = TransactionIdStore.BASE_TX_ID;
+        long lowestRecoveredTxId = TransactionIdStore.BASE_TX_ID;
         try (   TransactionCursor transactionsToRecover = spi.getTransactionsInReverseOrder( recoveryFromPosition );
                 RecoveryApplier recoveryVisitor = spi.getRecoveryApplier( REVERSE_RECOVERY ); )
         {
             while ( transactionsToRecover.next() )
             {
                 recoveryVisitor.visit( transactionsToRecover.get() );
-                checkpointTxId = transactionsToRecover.get().getCommitEntry().getTxId();
+                lowestRecoveredTxId = transactionsToRecover.get().getCommitEntry().getTxId();
             }
         }
 
-        monitor.reverseStoreRecoveryCompleted( checkpointTxId );
+        monitor.reverseStoreRecoveryCompleted( lowestRecoveredTxId );
 
         // Forward with all appliers
         LogPosition recoveryToPosition;

community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/UpdateCapturingIndexProvider.java

@@ -44,7 +44,7 @@ public class UpdateCapturingIndexProvider extends SchemaIndexProvider
 
     public UpdateCapturingIndexProvider( SchemaIndexProvider actual, Map<Long,Collection<IndexEntryUpdate>> initialUpdates )
     {
-        super( actual.getProviderDescriptor(), actual.priority() );
+        super( actual );
         this.actual = actual;
         this.initialUpdates = initialUpdates;
     }

community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/log/GivenTransactionCursor.java

@@ -30,7 +30,7 @@ public class GivenTransactionCursor implements TransactionCursor
     private int index = -1;
     private final CommittedTransactionRepresentation[] transactions;
 
-    public GivenTransactionCursor( CommittedTransactionRepresentation... transactions )
+    private GivenTransactionCursor( CommittedTransactionRepresentation... transactions )
     {
         this.transactions = transactions;
     }

community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/log/ReversedMultiFileTransactionCursorTest.java

@@ -20,8 +20,6 @@
 package org.neo4j.kernel.impl.transaction.log;
 
 import org.junit.Test;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
 
 import java.io.IOException;
 import java.util.concurrent.atomic.AtomicLong;
@@ -135,25 +133,21 @@ private ThrowingFunction<LogPosition,TransactionCursor,IOException> log( int...
             logs[logVersion] = transactions( transactionCounts[logVersion], txId );
         }
 
-        when( result.apply( any( LogPosition.class ) ) ).thenAnswer( new Answer<TransactionCursor>()
+        when( result.apply( any( LogPosition.class ) ) ).thenAnswer( invocation ->
         {
-            @Override
-            public TransactionCursor answer( InvocationOnMock invocation ) throws Throwable
+            LogPosition position = invocation.getArgumentAt( 0, LogPosition.class );
+            if ( position == null )
             {
-                LogPosition position = invocation.getArgumentAt( 0, LogPosition.class );
-                if ( position == null )
-                {
-                    // A mockito issue when calling the "when" methods, I believe
-                    return null;
-                }
-
-                // For simplicity the offset means, in this test, the array offset
-                CommittedTransactionRepresentation[] transactions = logs[toIntExact( position.getLogVersion() )];
-                CommittedTransactionRepresentation[] subset =
-                        copyOfRange( transactions, toIntExact( position.getByteOffset() - baseOffset ), transactions.length );
-                ArrayUtil.reverse( subset );
-                return given( subset );
+                // A mockito issue when calling the "when" methods, I believe
+                return null;
             }
+
+            // For simplicity the offset means, in this test, the array offset
+            CommittedTransactionRepresentation[] transactions = logs[toIntExact( position.getLogVersion() )];
+            CommittedTransactionRepresentation[] subset =
+                    copyOfRange( transactions, toIntExact( position.getByteOffset() - baseOffset ), transactions.length );
+            ArrayUtil.reverse( subset );
+            return given( subset );
         } );
         return result;
     }

community/kernel/src/test/java/org/neo4j/test/TestGraphDatabaseFactory.java

@@ -237,12 +237,12 @@ public static class TestGraphDatabaseFacadeFactory extends GraphDatabaseFacadeFa
         private final TestGraphDatabaseFactoryState state;
         private final boolean impermanent;
 
-        public TestGraphDatabaseFacadeFactory( TestGraphDatabaseFactoryState state, boolean impermanent )
+        protected TestGraphDatabaseFacadeFactory( TestGraphDatabaseFactoryState state, boolean impermanent )
         {
             this( state, impermanent, DatabaseInfo.COMMUNITY, CommunityEditionModule::new );
         }
 
-        public TestGraphDatabaseFacadeFactory( TestGraphDatabaseFactoryState state, boolean impermanent,
+        protected TestGraphDatabaseFacadeFactory( TestGraphDatabaseFactoryState state, boolean impermanent,
                 DatabaseInfo databaseInfo, Function<PlatformModule,EditionModule> editionFactory )
         {
             super( databaseInfo, editionFactory );

community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/populator/NonUniqueDatabaseIndexPopulatingUpdaterTest.java

@@ -201,9 +201,9 @@ public void additionsDeliveredToIndexWriter() throws Exception
         LuceneIndexWriter writer = mock( LuceneIndexWriter.class );
         NonUniqueLuceneIndexPopulatingUpdater updater = newUpdater( writer );
 
-        String expectedString1 = documentRepresentingProperties( (long) 1, "foo" ).toString();
-        String expectedString2 = documentRepresentingProperties( (long) 2, "bar" ).toString();
-        String expectedString3 = documentRepresentingProperties( (long) 3, "qux" ).toString();
+        String expectedString1 = documentRepresentingProperties( 1, "foo" ).toString();
+        String expectedString2 = documentRepresentingProperties( 2, "bar" ).toString();
+        String expectedString3 = documentRepresentingProperties( 3, "qux" ).toString();
         String expectedString4 = documentRepresentingProperties( 4, "git", "bit" ).toString();
 
         updater.process( add( 1, SCHEMA_DESCRIPTOR, "foo" ) );
@@ -225,8 +225,8 @@ public void changesDeliveredToIndexWriter() throws Exception
         LuceneIndexWriter writer = mock( LuceneIndexWriter.class );
         NonUniqueLuceneIndexPopulatingUpdater updater = newUpdater( writer );
 
-        String expectedString1 = documentRepresentingProperties( (long) 1, "after1" ).toString();
-        String expectedString2 = documentRepresentingProperties( (long) 2, "after2" ).toString();
+        String expectedString1 = documentRepresentingProperties( 1, "after1" ).toString();
+        String expectedString2 = documentRepresentingProperties( 2, "after2" ).toString();
         String expectedString3 = documentRepresentingProperties( 3, "bit", "after2" ).toString();
 
         updater.process( change( 1, SCHEMA_DESCRIPTOR, "before1", "after1" ) );
@@ -274,11 +274,6 @@ private static void verifySamplingResult( NonUniqueIndexSampler sampler, long ex
         assertEquals( expectedSampleSize, sample.sampleSize() );
     }
 
-    private static NonUniqueLuceneIndexPopulatingUpdater newUpdater()
-    {
-        return newUpdater( newSampler() );
-    }
-
     private static NonUniqueLuceneIndexPopulatingUpdater newUpdater( NonUniqueIndexSampler sampler )
     {
         return newUpdater( mock( LuceneIndexWriter.class ), sampler );