Addressed Martin's and Andrew's comments

enterprise/backup/src/main/java/org/neo4j/backup/AbstractBackupSupportingClassesFactory.java

@@ -73,7 +73,7 @@ BackupSupportingClasses createSupportingClassesForBackupStrategies( Config confi
     {
         PageCache pageCache = createPageCache( fileSystemAbstraction, config );
         return new BackupSupportingClasses(
-                backupDelegatorFormConfig( pageCache, config ),
+                backupDelegatorFromConfig( pageCache, config ),
                 haFromConfig( pageCache ),
                 pageCache );
     }
@@ -83,7 +83,7 @@ private BackupProtocolService haFromConfig( PageCache pageCache )
         return new BackupProtocolService( () -> fileSystemAbstraction, logProvider, logDestination, monitors, pageCache );
     }
 
-    private BackupDelegator backupDelegatorFormConfig( PageCache pageCache, Config config )
+    private BackupDelegator backupDelegatorFromConfig( PageCache pageCache, Config config )
     {
         PipelineHandlerAppenderFactory pipelineHandlerAppenderFactory = getPipelineHandlerAppenderFactory();
         PipelineHandlerAppender pipelineHandlerAppender = pipelineHandlerAppenderFactory.create( config, getDependencies(), logProvider );

enterprise/backup/src/main/java/org/neo4j/backup/BackupCopyService.java

@@ -21,8 +21,12 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
 

enterprise/backup/src/main/java/org/neo4j/backup/BackupFlow.java

@@ -22,21 +22,17 @@
 import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 import java.util.function.Supplier;
 
 import org.neo4j.commandline.admin.CommandFailed;
 import org.neo4j.commandline.admin.OutsideWorld;
 import org.neo4j.consistency.ConsistencyCheckService;
 import org.neo4j.consistency.checking.full.ConsistencyFlags;
 import org.neo4j.helpers.progress.ProgressMonitorFactory;
-import org.neo4j.io.pagecache.PageCache;
 import org.neo4j.kernel.configuration.Config;
-import org.neo4j.kernel.internal.GraphDatabaseAPI;
 import org.neo4j.logging.LogProvider;
 
 import static java.lang.String.format;
-import static org.neo4j.backup.BackupProtocolService.startTemporaryDb;
 
 /**
  * Controls the outcome of the backup tool.

enterprise/backup/src/main/java/org/neo4j/backup/BackupProtocolService.java

@@ -80,6 +80,7 @@
 import static org.neo4j.com.storecopy.TransactionCommittingResponseUnpacker.DEFAULT_BATCH_SIZE;
 import static org.neo4j.graphdb.factory.GraphDatabaseSettings.logs_directory;
 import static org.neo4j.graphdb.factory.GraphDatabaseSettings.store_internal_log_path;
+import static org.neo4j.helpers.Exceptions.launderedException;
 import static org.neo4j.helpers.Exceptions.rootCause;
 import static org.neo4j.kernel.impl.pagecache.ConfigurableStandalonePageCacheFactory.createPageCache;
 
@@ -93,6 +94,8 @@ class BackupProtocolService
             " perform a full backup at this point. You can modify this time interval by setting the '" +
             GraphDatabaseSettings.keep_logical_logs.name() + "' configuration on the database to a higher value.";
 
+    static final String DIFFERENT_STORE_MESSAGE = "Target directory contains full backup of a logically different store.";
+
     private final Supplier<FileSystemAbstraction> fileSystemSupplier;
     private final LogProvider logProvider;
     private final Log log;
@@ -368,7 +371,7 @@ private long incrementalWithContext( String sourceHostNameOrIp, int sourcePort,
         }
         catch ( MismatchingStoreIdException e )
         {
-            throw new ExistingBackupWithDifferentStoreException( e );
+            throw new RuntimeException( DIFFERENT_STORE_MESSAGE, e );
         }
         catch ( RuntimeException | IOException e )
         {

enterprise/backup/src/main/java/org/neo4j/backup/BackupStrategyWrapper.java

@@ -70,15 +70,17 @@ PotentiallyErroneousState<BackupStrategyOutcome> doBackup( OnlineBackupContext o
     private PotentiallyErroneousState<BackupStrategyOutcome> performBackupWithoutLifecycle( OnlineBackupContext onlineBackupContext )
     {
         File backupLocation = onlineBackupContext.getResolvedLocationFromName();
-        PotentiallyErroneousState<BackupStageOutcome> state;
         final File userSpecifiedBackupLocation = onlineBackupContext.getResolvedLocationFromName();
         final OptionalHostnamePort userSpecifiedAddress = onlineBackupContext.getRequiredArguments().getAddress();
         final Config config = onlineBackupContext.getConfig();
+
         if ( backupCopyService.backupExists( backupLocation ) )
         {
-            state = backupStrategy.performIncrementalBackup( userSpecifiedBackupLocation, config, userSpecifiedAddress );
+            PotentiallyErroneousState<BackupStageOutcome> state =
+                    backupStrategy.performIncrementalBackup( userSpecifiedBackupLocation, config, userSpecifiedAddress );
             boolean fullBackupWontWork = BackupStageOutcome.WRONG_PROTOCOL.equals( state.getState() );
             boolean incrementalWasSuccessful = BackupStageOutcome.SUCCESS.equals( state.getState() );
+
             if ( fullBackupWontWork || incrementalWasSuccessful )
             {
                 return describeOutcome( state );
@@ -95,28 +97,37 @@ private PotentiallyErroneousState<BackupStageOutcome> fullBackupWithTemporaryFol
     {
         final File userSpecifiedBackupLocation = onlineBackupContext.getResolvedLocationFromName();
         File temporaryFullBackupLocation = backupCopyService.findAnAvailableLocationForNewFullBackup( userSpecifiedBackupLocation );
-        PotentiallyErroneousState<BackupStageOutcome> state =
-                backupStrategy.performFullBackup( temporaryFullBackupLocation, onlineBackupContext.getConfig(),
-                        onlineBackupContext.getRequiredArguments().getAddress() );
+        PotentiallyErroneousState<BackupStageOutcome> state = backupStrategy.performFullBackup( temporaryFullBackupLocation, onlineBackupContext.getConfig(),
+                onlineBackupContext.getRequiredArguments().getAddress() );
 
-        boolean aBackupAlreadyExisted = userSpecifiedBackupLocation.equals( temporaryFullBackupLocation );
-        if ( BackupStageOutcome.SUCCESS.equals( state.getState() ) && !aBackupAlreadyExisted )
+        boolean aBackupAlreadyExisted =
+                userSpecifiedBackupLocation.equals( temporaryFullBackupLocation ); // NOTE temporaryFullBackupLocation can be equal to desired
+
+        if ( BackupStageOutcome.SUCCESS.equals( state.getState() ) )
         {
-            File newBackupLocationForPreExistingBackup = backupCopyService.findNewBackupLocationForBrokenExisting( userSpecifiedBackupLocation );
-            try
-            {
-                backupCopyService.moveBackupLocation( userSpecifiedBackupLocation, newBackupLocationForPreExistingBackup );
-                backupCopyService.moveBackupLocation( temporaryFullBackupLocation, userSpecifiedBackupLocation );
-            }
-            catch ( CommandFailed commandFailed )
+            backupRecoveryService.recoverWithDatabase( temporaryFullBackupLocation, pageCache, config );
+            if ( !aBackupAlreadyExisted )
             {
-                return new PotentiallyErroneousState<>( BackupStageOutcome.UNRECOVERABLE_FAILURE, commandFailed );
+                try
+                {
+                    renameTemporaryBackupToExpected( temporaryFullBackupLocation, userSpecifiedBackupLocation );
+                }
+                catch ( CommandFailed commandFailed )
+                {
+                    return new PotentiallyErroneousState<>( BackupStageOutcome.UNRECOVERABLE_FAILURE, commandFailed );
+                }
             }
         }
-        backupRecoveryService.recoverWithDatabase( userSpecifiedBackupLocation, pageCache, config );
         return state;
     }
 
+    private void renameTemporaryBackupToExpected( File temporaryFullBackupLocation, File userSpecifiedBackupLocation ) throws CommandFailed
+    {
+        File newBackupLocationForPreExistingBackup = backupCopyService.findNewBackupLocationForBrokenExisting( userSpecifiedBackupLocation );
+        backupCopyService.moveBackupLocation( userSpecifiedBackupLocation, newBackupLocationForPreExistingBackup );
+        backupCopyService.moveBackupLocation( temporaryFullBackupLocation, userSpecifiedBackupLocation );
+    }
+
     private PotentiallyErroneousState<BackupStrategyOutcome> describeOutcome( PotentiallyErroneousState<BackupStageOutcome> strategyStageOutcome )
     {
         BackupStageOutcome stageOutcome = strategyStageOutcome.getState();

enterprise/backup/src/main/java/org/neo4j/backup/HaBackupStrategy.java

@@ -25,6 +25,7 @@
 import org.neo4j.helpers.HostnamePort;
 import org.neo4j.helpers.OptionalHostnamePort;
 import org.neo4j.kernel.configuration.Config;
+import org.neo4j.kernel.impl.store.MismatchingStoreIdException;
 import org.neo4j.kernel.lifecycle.LifecycleAdapter;
 
 public class HaBackupStrategy extends LifecycleAdapter implements BackupStrategy
@@ -50,10 +51,9 @@ public PotentiallyErroneousState<BackupStageOutcome> performIncrementalBackup( F
                     config );
             return new PotentiallyErroneousState<>( BackupStageOutcome.SUCCESS, null );
         }
-        catch ( ExistingBackupWithDifferentStoreException e )
+        catch ( MismatchingStoreIdException e )
         {
-            ExistingBackupWithDifferentStoreException exceptionWithFilename = new ExistingBackupWithDifferentStoreException( e, backupDestination );
-            return new PotentiallyErroneousState<>( BackupStageOutcome.UNRECOVERABLE_FAILURE, exceptionWithFilename );
+            return new PotentiallyErroneousState<>( BackupStageOutcome.UNRECOVERABLE_FAILURE, e );
         }
         catch ( IncrementalBackupNotPossibleException e )
         {

enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackup.java

@@ -99,8 +99,8 @@ public OnlineBackup backup( String targetDirectory )
      */
     public OnlineBackup backup( File targetDirectory )
     {
-        outcome = new BackupProtocolService( out ).doIncrementalBackupOrFallbackToFull( hostNameOrIp, port, targetDirectory,
-                getConsistencyCheck( true ), defaultConfig(), timeoutMillis, forensics );
+        outcome = new BackupProtocolService( out ).doIncrementalBackupOrFallbackToFull( hostNameOrIp, port, targetDirectory, getConsistencyCheck( true ),
+                defaultConfig(), timeoutMillis, forensics );
         return this;
     }
 
@@ -133,8 +133,9 @@ public OnlineBackup backup( String targetDirectory, boolean verification )
      */
     public OnlineBackup backup( File targetDirectory, boolean verification )
     {
-        outcome = new BackupProtocolService( out ).doIncrementalBackupOrFallbackToFull( hostNameOrIp, port, targetDirectory,
-                getConsistencyCheck( verification ), defaultConfig(), timeoutMillis, forensics );
+        outcome =
+                new BackupProtocolService( out ).doIncrementalBackupOrFallbackToFull( hostNameOrIp, port, targetDirectory, getConsistencyCheck( verification ),
+                        defaultConfig(), timeoutMillis, forensics );
         return this;
     }
 
@@ -166,8 +167,8 @@ public OnlineBackup backup( String targetDirectory, Config tuningConfiguration )
      */
     public OnlineBackup backup( File targetDirectory, Config tuningConfiguration )
     {
-        outcome = new BackupProtocolService( out ).doIncrementalBackupOrFallbackToFull( hostNameOrIp, port, targetDirectory,
-                getConsistencyCheck( true ), tuningConfiguration, timeoutMillis, forensics );
+        outcome = new BackupProtocolService( out ).doIncrementalBackupOrFallbackToFull( hostNameOrIp, port, targetDirectory, getConsistencyCheck( true ),
+                tuningConfiguration, timeoutMillis, forensics );
         return this;
     }
 
@@ -202,8 +203,9 @@ public OnlineBackup backup( String targetDirectory, Config tuningConfiguration,
      */
     public OnlineBackup backup( File targetDirectory, Config tuningConfiguration, boolean verification )
     {
-        outcome = new BackupProtocolService( out ).doIncrementalBackupOrFallbackToFull( hostNameOrIp, port, targetDirectory,
-                getConsistencyCheck( verification ), tuningConfiguration, timeoutMillis, forensics );
+        outcome =
+                new BackupProtocolService( out ).doIncrementalBackupOrFallbackToFull( hostNameOrIp, port, targetDirectory, getConsistencyCheck( verification ),
+                        tuningConfiguration, timeoutMillis, forensics );
         return this;
     }
 
@@ -308,8 +310,8 @@ public OnlineBackup full( String targetDirectory, boolean verification, Config t
     @Deprecated
     public OnlineBackup incremental( String targetDirectory )
     {
-        outcome = new BackupProtocolService( out ).doIncrementalBackup( hostNameOrIp, port, new File( targetDirectory ),
-                getConsistencyCheck( false ), timeoutMillis, defaultConfig() );
+        outcome = new BackupProtocolService( out ).doIncrementalBackup( hostNameOrIp, port, new File( targetDirectory ), getConsistencyCheck( false ),
+                timeoutMillis, defaultConfig() );
         return this;
     }
 
@@ -330,8 +332,8 @@ public OnlineBackup incremental( String targetDirectory )
     @Deprecated
     public OnlineBackup incremental( String targetDirectory, boolean verification )
     {
-        outcome = new BackupProtocolService( out ).doIncrementalBackup( hostNameOrIp, port, new File( targetDirectory ),
-                getConsistencyCheck( verification ), timeoutMillis, defaultConfig() );
+        outcome = new BackupProtocolService( out ).doIncrementalBackup( hostNameOrIp, port, new File( targetDirectory ), getConsistencyCheck( verification ),
+                timeoutMillis, defaultConfig() );
         return this;
     }
 

enterprise/backup/src/test/java/org/neo4j/backup/BackupProtocolServiceIT.java

@@ -55,13 +55,11 @@
 import org.neo4j.io.fs.FileUtils;
 import org.neo4j.io.pagecache.IOLimiter;
 import org.neo4j.io.pagecache.PageCache;
-import org.neo4j.io.pagecache.impl.muninn.MuninnPageCache;
 import org.neo4j.kernel.configuration.Config;
 import org.neo4j.kernel.configuration.Settings;
 import org.neo4j.kernel.impl.enterprise.configuration.OnlineBackupSettings;
 import org.neo4j.kernel.impl.factory.DatabaseInfo;
 import org.neo4j.kernel.impl.logging.LogService;
-import org.neo4j.kernel.impl.pagecache.ConfigurableStandalonePageCacheFactory;
 import org.neo4j.kernel.impl.spi.SimpleKernelContext;
 import org.neo4j.kernel.impl.store.MetaDataStore;
 import org.neo4j.kernel.impl.store.MetaDataStore.Position;
@@ -1011,7 +1009,7 @@ public void incrementalBackupShouldFailWhenTargetDirContainsDifferentStore() thr
         catch ( RuntimeException e )
         {
             // Then
-            assertThat( e.getMessage(), equalTo( ExistingBackupWithDifferentStoreException.DIFFERENT_STORE_MESSAGE ) );
+            assertThat( e.getMessage(), equalTo( BackupProtocolService.DIFFERENT_STORE_MESSAGE ) );
             assertThat( e.getCause(), instanceOf( MismatchingStoreIdException.class ) );
         }
     }