3.4 - Resolved Consistency check errors in backup

Before applying this patch, the neo4j-admin backup tool can fail
performing a backup. This is due to the omission of the step to recover
a store once it has been copied. The solution is starting a temporary
embedded database after every full backup.

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

@@ -32,7 +32,6 @@
 import org.neo4j.io.fs.FileSystemAbstraction;
 import org.neo4j.io.pagecache.PageCache;
 import org.neo4j.kernel.configuration.Config;
-import org.neo4j.kernel.impl.factory.PlatformModule;
 import org.neo4j.kernel.impl.pagecache.ConfigurableStandalonePageCacheFactory;
 import org.neo4j.kernel.impl.util.Dependencies;
 import org.neo4j.kernel.monitoring.Monitors;
@@ -72,8 +71,11 @@ protected AbstractBackupSupportingClassesFactory( BackupModuleResolveAtRuntime b
      */
     BackupSupportingClasses createSupportingClassesForBackupStrategies( Config config )
     {
-        PageCache pageCache = ConfigurableStandalonePageCacheFactory.createPageCache( fileSystemAbstraction, config );
-        return new BackupSupportingClasses( backupDelegatorFormConfig( pageCache, config ), haFromConfig( pageCache ) );
+        PageCache pageCache = createPageCache( fileSystemAbstraction, config );
+        return new BackupSupportingClasses(
+                backupDelegatorFormConfig( pageCache, config ),
+                haFromConfig( pageCache ),
+                pageCache );
     }
 
     private BackupProtocolService haFromConfig( PageCache pageCache )
@@ -100,6 +102,11 @@ private static BackupDelegator backupDelegator( RemoteStore remoteStore, CatchUp
         return new BackupDelegator( remoteStore, catchUpClient, storeCopyClient, new ClearIdService( new IdGeneratorWrapper() ) );
     }
 
+    private static PageCache createPageCache( FileSystemAbstraction fileSystemAbstraction, Config config )
+    {
+        return ConfigurableStandalonePageCacheFactory.createPageCache( fileSystemAbstraction, config );
+    }
+
     private Dependencies getDependencies()
     {
         return new Dependencies();

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

@@ -22,17 +22,21 @@
 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/BackupFlowFactory.java

@@ -26,6 +26,7 @@
 import org.neo4j.commandline.admin.OutsideWorld;
 import org.neo4j.consistency.ConsistencyCheckService;
 import org.neo4j.helpers.progress.ProgressMonitorFactory;
+import org.neo4j.io.pagecache.PageCache;
 import org.neo4j.logging.LogProvider;
 
 /*
@@ -50,15 +51,17 @@ class BackupFlowFactory
         this.addressResolutionHelper = new AddressResolutionHelper();
     }
 
-    BackupFlow backupFlow( OnlineBackupContext onlineBackupContext, BackupProtocolService backupProtocolService, BackupDelegator backupDelegator )
+    BackupFlow backupFlow( OnlineBackupContext onlineBackupContext, BackupProtocolService backupProtocolService, BackupDelegator backupDelegator,
+            PageCache pageCache )
     {
         ProgressMonitorFactory progressMonitorFactory = ProgressMonitorFactory.textual( outsideWorld.errorStream() );
 
         List<BackupStrategyWrapper> strategies = Stream
             .of(
                 new CausalClusteringBackupStrategy( backupDelegator, addressResolutionHelper ),
                 new HaBackupStrategy( backupProtocolService, addressResolutionHelper, onlineBackupContext.getRequiredArguments().getTimeout() ) )
-            .map( strategy -> new BackupStrategyWrapper( strategy, backupCopyService ) )
+            .map( strategy -> new BackupStrategyWrapper( strategy, backupCopyService, pageCache, onlineBackupContext.getConfig(),
+                    new BackupRecoveryService() ) )
             .collect( Collectors.toList() );
 
         return new BackupFlow( consistencyCheckService, outsideWorld, logProvider, progressMonitorFactory, strategies );

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

@@ -93,14 +93,12 @@ 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;
     private final OutputStream logDestination;
     private final Monitors monitors;
-    private final Optional<PageCache> pageCacheOptional;
+    private final PageCache pageCache;
 
     BackupProtocolService()
     {
@@ -109,8 +107,8 @@ class BackupProtocolService
 
     BackupProtocolService( OutputStream logDestination )
     {
-        this( DefaultFileSystemAbstraction::new, FormattedLogProvider.toOutputStream( logDestination ), logDestination,
-                new Monitors(), null );
+        this( DefaultFileSystemAbstraction::new, FormattedLogProvider.toOutputStream( logDestination ), logDestination, new Monitors(),
+                createPageCache( new DefaultFileSystemAbstraction() ) );
     }
 
     BackupProtocolService( Supplier<FileSystemAbstraction> fileSystemSupplier, LogProvider logProvider, OutputStream logDestination, Monitors monitors,
@@ -121,7 +119,7 @@ class BackupProtocolService
         this.log = logProvider.getLog( getClass() );
         this.logDestination = logDestination;
         this.monitors = monitors;
-        this.pageCacheOptional = Optional.ofNullable( pageCache );
+        this.pageCache = pageCache;
         monitors.addMonitorListener( new StoreCopyClientLoggingMonitor( log ), getClass().getName() );
     }
 
@@ -148,7 +146,7 @@ private BackupOutcome fullBackup( FileSystemAbstraction fileSystem, String sourc
         }
         long timestamp = System.currentTimeMillis();
         long lastCommittedTx = -1;
-        try ( PageCache pageCache = this.pageCacheOptional.orElse( createPageCache( fileSystem, tuningConfiguration ) ) )
+        try
         {
             StoreCopyClient storeCopier = new StoreCopyClient( targetDirectory, tuningConfiguration,
                     loadKernelExtensions(), logProvider, fileSystem, pageCache,
@@ -201,7 +199,7 @@ private BackupOutcome incrementalBackup( FileSystemAbstraction fileSystem, Strin
 
         Map<String,String> configParams = config.getRaw();
 
-        try ( PageCache pageCache = this.pageCacheOptional.orElse( createPageCache( fileSystem, config ) ) )
+        try
         {
             GraphDatabaseAPI targetDb = startTemporaryDb( targetDirectory, pageCache, configParams );
             long backupStartTime = System.currentTimeMillis();
@@ -328,7 +326,7 @@ private boolean directoryIsEmpty( FileSystemAbstraction fileSystem, File targetD
         return !fileSystem.isDirectory( targetDirectory ) || 0 == fileSystem.listFiles( targetDirectory ).length;
     }
 
-    private static GraphDatabaseAPI startTemporaryDb( File targetDirectory, PageCache pageCache,
+    static GraphDatabaseAPI startTemporaryDb( File targetDirectory, PageCache pageCache,
             Map<String,String> config )
     {
         GraphDatabaseFactory factory = ExternallyManagedPageCache.graphDatabaseFactoryWithPageCache( pageCache );
@@ -370,7 +368,7 @@ private long incrementalWithContext( String sourceHostNameOrIp, int sourcePort,
         }
         catch ( MismatchingStoreIdException e )
         {
-            throw new RuntimeException( DIFFERENT_STORE_MESSAGE, e );
+            throw new ExistingBackupWithDifferentStoreException( e );
         }
         catch ( RuntimeException | IOException e )
         {

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

@@ -0,0 +1,39 @@
+/*
+ * 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.backup;
+
+import java.io.File;
+import java.util.Map;
+
+import org.neo4j.io.pagecache.PageCache;
+import org.neo4j.kernel.configuration.Config;
+import org.neo4j.kernel.internal.GraphDatabaseAPI;
+
+import static org.neo4j.backup.BackupProtocolService.startTemporaryDb;
+
+public class BackupRecoveryService
+{
+    public void recoverWithDatabase( File targetDirectory, PageCache pageCache, Config config )
+    {
+        Map<String,String> configParams = config.getRaw();
+        GraphDatabaseAPI targetDb = startTemporaryDb( targetDirectory, pageCache, configParams );
+        targetDb.shutdown();
+    }
+}

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

@@ -20,21 +20,34 @@
 package org.neo4j.backup;
 
 import java.io.File;
+import java.util.Map;
 
 import org.neo4j.commandline.admin.CommandFailed;
 import org.neo4j.helpers.OptionalHostnamePort;
+import org.neo4j.io.pagecache.PageCache;
 import org.neo4j.kernel.configuration.Config;
+import org.neo4j.kernel.internal.GraphDatabaseAPI;
 import org.neo4j.kernel.lifecycle.LifeSupport;
 
+import static org.neo4j.backup.BackupProtocolService.startTemporaryDb;
+
 class BackupStrategyWrapper
 {
     private final BackupStrategy backupStrategy;
     private final BackupCopyService backupCopyService;
+    private final BackupRecoveryService backupRecoveryService;
+
+    private final PageCache pageCache;
+    private final Config config;
 
-    BackupStrategyWrapper( BackupStrategy backupStrategy, BackupCopyService backupCopyService )
+    BackupStrategyWrapper( BackupStrategy backupStrategy, BackupCopyService backupCopyService, PageCache pageCache, Config config,
+            BackupRecoveryService backupRecoveryService )
     {
         this.backupStrategy = backupStrategy;
         this.backupCopyService = backupCopyService;
+        this.pageCache = pageCache;
+        this.config = config;
+        this.backupRecoveryService = backupRecoveryService;
     }
 
     /**
@@ -100,6 +113,7 @@ private PotentiallyErroneousState<BackupStageOutcome> fullBackupWithTemporaryFol
                 return new PotentiallyErroneousState<>( BackupStageOutcome.UNRECOVERABLE_FAILURE, commandFailed );
             }
         }
+        backupRecoveryService.recoverWithDatabase( userSpecifiedBackupLocation, pageCache, config );
         return state;
     }
 

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

@@ -19,15 +19,22 @@
  */
 package org.neo4j.backup;
 
+import org.neo4j.io.pagecache.PageCache;
+
 public class BackupSupportingClasses
 {
+    // Strategies
     private final BackupDelegator backupDelegator;
     private final BackupProtocolService backupProtocolService;
 
-    public BackupSupportingClasses( BackupDelegator backupDelegator, BackupProtocolService backupProtocolService )
+    // Dependency Helpers
+    private final PageCache pageCache;
+
+    public BackupSupportingClasses( BackupDelegator backupDelegator, BackupProtocolService backupProtocolService, PageCache pageCache )
     {
         this.backupDelegator = backupDelegator;
         this.backupProtocolService = backupProtocolService;
+        this.pageCache = pageCache;
     }
 
     public BackupDelegator getBackupDelegator()
@@ -39,4 +46,9 @@ public BackupProtocolService getBackupProtocolService()
     {
         return backupProtocolService;
     }
+
+    public PageCache getPageCache()
+    {
+        return pageCache;
+    }
 }

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

@@ -0,0 +1,40 @@
+/*
+ * 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.backup;
+
+import java.io.File;
+
+import org.neo4j.kernel.impl.store.MismatchingStoreIdException;
+
+class ExistingBackupWithDifferentStoreException extends RuntimeException
+{
+    static final String DIFFERENT_STORE_MESSAGE = "Target directory contains full backup of a logically different store.";
+    private static final String BACKUP_LOCATION_MESSAGE = "Directory `%s` contains full backup of a logically different store.";
+
+    ExistingBackupWithDifferentStoreException( MismatchingStoreIdException mismatchingStoreIdException )
+    {
+        super( DIFFERENT_STORE_MESSAGE, mismatchingStoreIdException );
+    }
+
+    ExistingBackupWithDifferentStoreException( ExistingBackupWithDifferentStoreException cause, File backupLocation )
+    {
+        super( String.format( BACKUP_LOCATION_MESSAGE, backupLocation ), cause );
+    }
+}

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

@@ -50,6 +50,11 @@ public PotentiallyErroneousState<BackupStageOutcome> performIncrementalBackup( F
                     config );
             return new PotentiallyErroneousState<>( BackupStageOutcome.SUCCESS, null );
         }
+        catch ( ExistingBackupWithDifferentStoreException e )
+        {
+            ExistingBackupWithDifferentStoreException exceptionWithFilename = new ExistingBackupWithDifferentStoreException( e, backupDestination );
+            return new PotentiallyErroneousState<>( BackupStageOutcome.UNRECOVERABLE_FAILURE, exceptionWithFilename );
+        }
         catch ( IncrementalBackupNotPossibleException e )
         {
             return new PotentiallyErroneousState<>( BackupStageOutcome.FAILURE, e );

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

@@ -25,6 +25,7 @@
 import org.neo4j.commandline.admin.CommandFailed;
 import org.neo4j.commandline.admin.IncorrectUsage;
 import org.neo4j.commandline.admin.OutsideWorld;
+import org.neo4j.io.pagecache.PageCache;
 
 public class OnlineBackupCommand implements AdminCommand
 {
@@ -62,7 +63,7 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed
         checkDestination( onlineBackupContext.getRequiredArguments().getReportDir() );
 
         BackupFlow backupFlow = backupFlowFactory.backupFlow( onlineBackupContext, backupSupportingClasses.getBackupProtocolService(),
-                backupSupportingClasses.getBackupDelegator() );
+                backupSupportingClasses.getBackupDelegator(), backupSupportingClasses.getPageCache() );
 
         backupFlow.performBackup( onlineBackupContext );
         outsideWorld.stdOutLine( "Backup complete." );

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

@@ -55,11 +55,13 @@
 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;
@@ -164,13 +166,13 @@ public void setup()
     private BackupProtocolService backupService()
     {
         return new BackupProtocolService( () -> new UncloseableDelegatingFileSystemAbstraction( fileSystemRule.get() ),
-                FormattedLogProvider.toOutputStream( NULL_OUTPUT ), NULL_OUTPUT, new Monitors(), null );
+                FormattedLogProvider.toOutputStream( NULL_OUTPUT ), NULL_OUTPUT, new Monitors(), pageCacheRule.getPageCache( fileSystemRule.get() ) );
     }
 
     private BackupProtocolService backupService( LogProvider logProvider )
     {
-        return new BackupProtocolService( () -> new UncloseableDelegatingFileSystemAbstraction( fileSystemRule.get() ),
-                logProvider, NULL_OUTPUT, new Monitors(), null );
+        return new BackupProtocolService( () -> new UncloseableDelegatingFileSystemAbstraction( fileSystemRule.get() ), logProvider, NULL_OUTPUT,
+                new Monitors(), pageCacheRule.getPageCache( fileSystemRule.get() ) );
     }
 
     @Test
@@ -1009,7 +1011,7 @@ public void incrementalBackupShouldFailWhenTargetDirContainsDifferentStore() thr
         catch ( RuntimeException e )
         {
             // Then
-            assertThat( e.getMessage(), equalTo( BackupProtocolService.DIFFERENT_STORE_MESSAGE ) );
+            assertThat( e.getMessage(), equalTo( ExistingBackupWithDifferentStoreException.DIFFERENT_STORE_MESSAGE ) );
             assertThat( e.getCause(), instanceOf( MismatchingStoreIdException.class ) );
         }
     }