From efb93f3460249d5da2160eb4f68db4e569130fc3 Mon Sep 17 00:00:00 2001 From: Przemek Hugh Kaznowski Date: Mon, 9 Oct 2017 09:13:52 +0100 Subject: [PATCH] 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. --- ...bstractBackupSupportingClassesFactory.java | 13 ++- .../java/org/neo4j/backup/BackupFlow.java | 4 + .../org/neo4j/backup/BackupFlowFactory.java | 7 +- .../neo4j/backup/BackupProtocolService.java | 18 ++-- .../neo4j/backup/BackupRecoveryService.java | 39 ++++++++ .../neo4j/backup/BackupStrategyWrapper.java | 16 +++- .../neo4j/backup/BackupSupportingClasses.java | 14 ++- ...tingBackupWithDifferentStoreException.java | 40 ++++++++ .../org/neo4j/backup/HaBackupStrategy.java | 5 + .../org/neo4j/backup/OnlineBackupCommand.java | 3 +- .../neo4j/backup/BackupProtocolServiceIT.java | 10 +- .../backup/BackupStrategyWrapperTest.java | 32 ++++++- .../neo4j/backup/OnlineBackupCommandCcIT.java | 62 +++++++++++- .../neo4j/backup/OnlineBackupCommandHaIT.java | 94 ++++++++++++++++++- .../neo4j/backup/OnlineBackupCommandTest.java | 2 +- .../test/java/org/neo4j/util/TestHelpers.java | 14 ++- 16 files changed, 341 insertions(+), 32 deletions(-) create mode 100644 enterprise/backup/src/main/java/org/neo4j/backup/BackupRecoveryService.java create mode 100644 enterprise/backup/src/main/java/org/neo4j/backup/ExistingBackupWithDifferentStoreException.java diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/AbstractBackupSupportingClassesFactory.java b/enterprise/backup/src/main/java/org/neo4j/backup/AbstractBackupSupportingClassesFactory.java index e49af34aca126..1ed56b04126da 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/AbstractBackupSupportingClassesFactory.java +++ b/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(); diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/BackupFlow.java b/enterprise/backup/src/main/java/org/neo4j/backup/BackupFlow.java index 2cbee4a547428..b6b601f4cccd7 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/BackupFlow.java +++ b/enterprise/backup/src/main/java/org/neo4j/backup/BackupFlow.java @@ -22,6 +22,7 @@ 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; @@ -29,10 +30,13 @@ 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. diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/BackupFlowFactory.java b/enterprise/backup/src/main/java/org/neo4j/backup/BackupFlowFactory.java index 7cfa2298d64b9..9119fadf82d89 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/BackupFlowFactory.java +++ b/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,7 +51,8 @@ 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() ); @@ -58,7 +60,8 @@ BackupFlow backupFlow( OnlineBackupContext onlineBackupContext, BackupProtocolSe .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 ); diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/BackupProtocolService.java b/enterprise/backup/src/main/java/org/neo4j/backup/BackupProtocolService.java index b58cf42a1957a..fb242c889b434 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/BackupProtocolService.java +++ b/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 fileSystemSupplier; private final LogProvider logProvider; private final Log log; private final OutputStream logDestination; private final Monitors monitors; - private final Optional 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 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 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 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 ) { diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/BackupRecoveryService.java b/enterprise/backup/src/main/java/org/neo4j/backup/BackupRecoveryService.java new file mode 100644 index 0000000000000..5c20af3748f59 --- /dev/null +++ b/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 . + */ +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 configParams = config.getRaw(); + GraphDatabaseAPI targetDb = startTemporaryDb( targetDirectory, pageCache, configParams ); + targetDb.shutdown(); + } +} diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/BackupStrategyWrapper.java b/enterprise/backup/src/main/java/org/neo4j/backup/BackupStrategyWrapper.java index f1443fd2b48fd..e1811cfbc9f45 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/BackupStrategyWrapper.java +++ b/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 fullBackupWithTemporaryFol return new PotentiallyErroneousState<>( BackupStageOutcome.UNRECOVERABLE_FAILURE, commandFailed ); } } + backupRecoveryService.recoverWithDatabase( userSpecifiedBackupLocation, pageCache, config ); return state; } diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/BackupSupportingClasses.java b/enterprise/backup/src/main/java/org/neo4j/backup/BackupSupportingClasses.java index d6876cb1d291d..887ac81a99bee 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/BackupSupportingClasses.java +++ b/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; + } } diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/ExistingBackupWithDifferentStoreException.java b/enterprise/backup/src/main/java/org/neo4j/backup/ExistingBackupWithDifferentStoreException.java new file mode 100644 index 0000000000000..d37bc36daa92d --- /dev/null +++ b/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 . + */ +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 ); + } +} diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/HaBackupStrategy.java b/enterprise/backup/src/main/java/org/neo4j/backup/HaBackupStrategy.java index 1aa319146599d..697b2ebac0c9a 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/HaBackupStrategy.java +++ b/enterprise/backup/src/main/java/org/neo4j/backup/HaBackupStrategy.java @@ -50,6 +50,11 @@ public PotentiallyErroneousState 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 ); diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java b/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java index f76a7c064b839..d6d1794ad4613 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java +++ b/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." ); diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/BackupProtocolServiceIT.java b/enterprise/backup/src/test/java/org/neo4j/backup/BackupProtocolServiceIT.java index d3ec110c77045..b27e203ec98c7 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/BackupProtocolServiceIT.java +++ b/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 ) ); } } diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/BackupStrategyWrapperTest.java b/enterprise/backup/src/test/java/org/neo4j/backup/BackupStrategyWrapperTest.java index f2437732c8d78..3518f7b737cff 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/BackupStrategyWrapperTest.java +++ b/enterprise/backup/src/test/java/org/neo4j/backup/BackupStrategyWrapperTest.java @@ -28,6 +28,7 @@ import org.neo4j.commandline.admin.OutsideWorld; import org.neo4j.helpers.OptionalHostnamePort; import org.neo4j.io.fs.FileSystemAbstraction; +import org.neo4j.io.pagecache.PageCache; import org.neo4j.kernel.configuration.Config; import static org.junit.Assert.assertEquals; @@ -54,6 +55,8 @@ public class BackupStrategyWrapperTest private Config config = mock( Config.class ); private OptionalHostnamePort userProvidedAddress = new OptionalHostnamePort( (String) null, null, null ); private PotentiallyErroneousState SUCCESS = new PotentiallyErroneousState<>( BackupStageOutcome.SUCCESS, null ); + private PageCache pageCache = mock( PageCache.class ); + private BackupRecoveryService backupRecoveryService = mock( BackupRecoveryService.class ); @Before public void setup() @@ -62,7 +65,7 @@ public void setup() when( onlineBackupContext.getResolvedLocationFromName() ).thenReturn( backupLocation ); when( onlineBackupContext.getRequiredArguments() ).thenReturn( requiredArguments ); when( backupStrategyImplementation.performFullBackup( any(), any(), any() ) ).thenReturn( SUCCESS ); - subject = new BackupStrategyWrapper( backupStrategyImplementation, backupCopyService ); + subject = new BackupStrategyWrapper( backupStrategyImplementation, backupCopyService, pageCache, config, backupRecoveryService ); } @Test @@ -234,4 +237,31 @@ public void failureDuringMoveCausesAbsoluteFailure() throws CommandFailed // and full backup was definitely performed verify( backupStrategyImplementation ).performFullBackup( any(), any(), any() ); } + + @Test + public void performingFullBackupInvokesRecovery() + { + // when + subject.doBackup( onlineBackupContext ); + + // then + verify( backupRecoveryService ).recoverWithDatabase( any(), any(), any() ); + } + + @Test + public void performingIncrementalBackupDoesNotInvokeRecovery() + { + // given backup exists + when( backupCopyService.backupExists( any() ) ).thenReturn( true ); + when( requiredArguments.isFallbackToFull() ).thenReturn( true ); + + // and incremental backups are successful + when( backupStrategyImplementation.performIncrementalBackup( any(), any(), any() ) ).thenReturn( SUCCESS ); + + // when + subject.doBackup( onlineBackupContext ); + + // then + verify( backupRecoveryService, never() ).recoverWithDatabase( any(), any(), any() ); + } } diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandCcIT.java b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandCcIT.java index a4f535d06cf18..2f4fce8c34863 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandCcIT.java +++ b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandCcIT.java @@ -20,6 +20,7 @@ package org.neo4j.backup; import org.apache.commons.lang3.SystemUtils; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -30,8 +31,10 @@ import org.junit.runners.Parameterized.Parameters; import java.io.File; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import org.neo4j.causalclustering.core.CausalClusteringSettings; import org.neo4j.causalclustering.core.CoreGraphDatabase; @@ -40,6 +43,7 @@ import org.neo4j.causalclustering.discovery.CoreClusterMember; import org.neo4j.causalclustering.helpers.CausalClusteringTestHelpers; import org.neo4j.graphdb.GraphDatabaseService; +import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.RelationshipType; import org.neo4j.graphdb.Transaction; @@ -75,6 +79,11 @@ public class OnlineBackupCommandCcIT private File backupDir; + private List oneOffShutdownTasks; + private static final Label label = Label.label( "any_label" ); + private static final String PROP_NAME = "name"; + private static final String PROP_RANDOM = "random"; + @Parameter public String recordFormat; @@ -87,9 +96,16 @@ public static List recordFormats() @Before public void initialiseBackupDirectory() { + oneOffShutdownTasks = new ArrayList<>(); backupDir = testDirectory.directory( "backups" ); } + @After + public void performShutdownTasks() + { + oneOffShutdownTasks.forEach( Runnable::run ); + } + @Test public void backupCanBePerformedOverCcWithCustomPort() throws Exception { @@ -126,7 +142,7 @@ public void backupCanNotBePerformedOverBackupProtocol() throws Exception assumeFalse( SystemUtils.IS_OS_WINDOWS ); Cluster cluster = startCluster( recordFormat ); - String ip = TestHelpers.backupAddress( clusterLeader( cluster ).database() ); + String ip = TestHelpers.backupAddressHa( clusterLeader( cluster ).database() ); assertEquals( 1, runBackupToolFromOtherJvmToGetExitCode( "--from", ip, @@ -135,6 +151,44 @@ public void backupCanNotBePerformedOverBackupProtocol() throws Exception "--name=defaultport" ) ); } + @Test + public void dataIsInAUsableStateAfterBackup() throws Exception + { + // given database exists + Cluster cluster = startCluster( recordFormat ); + + // and the database has indexes + createIndexes( cluster.getDbWithAnyRole( Role.LEADER ).database() ); + + // and the database is being populated + AtomicBoolean continueFlagReference = new AtomicBoolean( false ); + new Thread( () -> repeatedlyPopulateDatabase( clusterLeader( cluster ).database(), continueFlagReference ) ) + .start(); // populate db with number properties etc. + oneOffShutdownTasks.add( () -> continueFlagReference.set( false ) ); // kill thread + + // then backup is successful + String ip = TestHelpers.backupAddressCc( clusterLeader( cluster ).database() ); + assertEquals( 0, + runBackupToolFromOtherJvmToGetExitCode( "--from", ip, "--cc-report-dir=" + backupDir, "--backup-dir=" + backupDir, "--name=defaultport" ) ); + } + + private void repeatedlyPopulateDatabase( GraphDatabaseService db, AtomicBoolean continueFlagReference ) + { + while ( continueFlagReference.get() ) + { + createSomeData( db ); + } + } + + private void createIndexes( CoreGraphDatabase db ) + { + try ( Transaction tx = db.beginTx() ) + { + db.schema().indexFor( label ).on( PROP_NAME ).on( PROP_RANDOM ).create(); + tx.success(); + } + } + private static CoreGraphDatabase clusterDatabase( Cluster cluster ) { return clusterLeader( cluster ).database(); @@ -155,9 +209,9 @@ public static DbRepresentation createSomeData( GraphDatabaseService db ) { try ( Transaction tx = db.beginTx() ) { - Node node = db.createNode(); - node.setProperty( "name", "Neo" ); - node.setProperty( "random", Math.random() * 10000 ); + Node node = db.createNode( label ); + node.setProperty( PROP_NAME, "Neo" ); + node.setProperty( PROP_RANDOM, Math.random() * 10000 ); db.createNode().createRelationshipTo( node, RelationshipType.withName( "KNOWS" ) ); tx.success(); } diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandHaIT.java b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandHaIT.java index 12d69c47241fa..98d5d009d6537 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandHaIT.java +++ b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandHaIT.java @@ -20,6 +20,8 @@ package org.neo4j.backup; import org.apache.commons.lang3.SystemUtils; +import org.junit.After; +import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -30,10 +32,18 @@ import org.junit.runners.Parameterized.Parameters; import java.io.File; +import java.nio.file.Path; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import org.neo4j.commandline.admin.AdminCommand; +import org.neo4j.commandline.admin.CommandFailed; +import org.neo4j.commandline.admin.IncorrectUsage; +import org.neo4j.commandline.admin.RealOutsideWorld; import org.neo4j.graphdb.GraphDatabaseService; +import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.RelationshipType; import org.neo4j.graphdb.Transaction; @@ -75,6 +85,11 @@ public static List recordFormats() return Arrays.asList( Standard.LATEST_NAME, HighLimit.NAME ); } + private List oneOffShutdownTasks; + private static final Label label = Label.label( "any_label" ); + private static final String PROP_NAME = "name"; + private static final String PROP_RANDOM = "random"; + private static DbRepresentation createSomeData( GraphDatabaseService db ) { try ( Transaction tx = db.beginTx() ) @@ -87,6 +102,18 @@ private static DbRepresentation createSomeData( GraphDatabaseService db ) return DbRepresentation.of( db ); } + @Before + public void resetTasks() + { + oneOffShutdownTasks = new ArrayList<>(); + } + + @After + public void shutdownTasks() + { + oneOffShutdownTasks.forEach( Runnable::run ); + } + @Test public void makeSureBackupCanBePerformedWithCustomPort() throws Exception { @@ -97,13 +124,13 @@ public void makeSureBackupCanBePerformedWithCustomPort() throws Exception startDb( backupPort ); assertEquals( "should not be able to do backup when noone is listening", 1, - runBackupTool( "--from", "127.0.0.1:" + PortAuthority.allocatePort(), + runBackupToolInSameJvm( "--from", "127.0.0.1:" + PortAuthority.allocatePort(), "--cc-report-dir=" + backupDir, "--backup-dir=" + backupDir, "--name=" + backupName ) ); assertEquals( 0, - runBackupTool( "--from", "127.0.0.1:" + backupPort, + runBackupToolInSameJvm( "--from", "127.0.0.1:" + backupPort, "--cc-report-dir=" + backupDir, "--backup-dir=" + backupDir, "--name=" + backupName ) ); @@ -111,13 +138,51 @@ public void makeSureBackupCanBePerformedWithCustomPort() throws Exception createSomeData( db ); assertEquals( 0, - runBackupTool( "--from", "127.0.0.1:" + backupPort, + runBackupToolInSameJvm( "--from", "127.0.0.1:" + backupPort, "--cc-report-dir=" + backupDir, "--backup-dir=" + backupDir, "--name=" + backupName ) ); assertEquals( getDbRepresentation(), getBackupDbRepresentation( backupName ) ); } + @Test + public void dataIsInAUsableStateAfterBackup() throws Exception + { + // given database exists + int backupPort = PortAuthority.allocatePort(); + startDb( backupPort ); + + // and the database has indexes + createIndexes( db ); + + // and the database is being populated + AtomicBoolean continueFlagReference = new AtomicBoolean( true ); + new Thread( () -> repeatedlyPopulateDatabase( db, continueFlagReference ) ).start(); // populate db with number properties etc. + oneOffShutdownTasks.add( () -> continueFlagReference.set( false ) ); // kill thread + + // then backup is successful + String ip = ":" + backupPort; + assertEquals( 0, + runBackupTool( "--from", ip, "--cc-report-dir=" + backupDir, "--backup-dir=" + backupDir, "--name=defaultport" ) ); + } + + private void repeatedlyPopulateDatabase( GraphDatabaseService db, AtomicBoolean continueFlagReference ) + { + while ( continueFlagReference.get() ) + { + createSomeData( db ); + } + } + + private void createIndexes( GraphDatabaseService db ) + { + try ( Transaction tx = db.beginTx() ) + { + db.schema().indexFor( label ).on( PROP_NAME ).on( PROP_RANDOM ).create(); + tx.success(); + } + } + private void startDb( Integer backupPort ) { db.setConfig( GraphDatabaseSettings.record_format, recordFormat ); @@ -127,6 +192,29 @@ private void startDb( Integer backupPort ) createSomeData( db ); } + private static int runBackupToolInSameJvm( String... args ) + { + OnlineBackupCommandProvider onlineBackupCommandProvider = new OnlineBackupCommandProvider(); + Path homeDir = testDirectory.absolutePath().toPath(); + Path configDir = homeDir.resolve( "config" ); + AdminCommand backupCommand = onlineBackupCommandProvider.create( homeDir, configDir, new RealOutsideWorld() ); + try + { + backupCommand.execute( args ); + return 0; + } + catch ( IncorrectUsage incorrectUsage ) + { + incorrectUsage.printStackTrace(); + return 1; + } + catch ( CommandFailed commandFailed ) + { + commandFailed.printStackTrace(); + return commandFailed.code(); + } + } + private static int runBackupTool( String... args ) throws Exception { diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java index 7dce55bfe4f5b..88218217c1dff 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java +++ b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java @@ -83,7 +83,7 @@ public void setup() throws Exception when( requiredArguments.getFolder() ).thenReturn( backupDirectory ); when( requiredArguments.getReportDir() ).thenReturn( reportDirectory ); when( requiredArguments.getName() ).thenReturn( "backup name" ); - when( backupFlowFactory.backupFlow( any(), any(), any() ) ).thenReturn( backupFlow ); + when( backupFlowFactory.backupFlow( any(), any(), any(), any() ) ).thenReturn( backupFlow ); subject = new OnlineBackupCommand( outsideWorld, onlineBackupContextLoader, backupSupportingClassesFactory, backupFlowFactory ); } diff --git a/enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java b/enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java index b3d3e8232850b..18705a3143dcd 100644 --- a/enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java +++ b/enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java @@ -27,8 +27,10 @@ import org.neo4j.commandline.admin.AdminTool; import org.neo4j.helpers.AdvertisedSocketAddress; import org.neo4j.helpers.HostnamePort; +import org.neo4j.helpers.ListenSocketAddress; import org.neo4j.io.proc.ProcessUtil; 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.GraphDatabaseFacade; import org.neo4j.kernel.internal.GraphDatabaseAPI; @@ -37,6 +39,8 @@ import org.neo4j.test.rule.EmbeddedDatabaseRule; import static java.lang.String.format; +import static org.neo4j.kernel.configuration.Settings.listenAddress; +import static org.neo4j.kernel.configuration.Settings.setting; public class TestHelpers { @@ -76,7 +80,15 @@ public static int runBackupToolFromOtherJvmToGetExitCode( File neo4jHome, String return new ProcessStreamHandler( process, false ).waitForResult(); } - public static String backupAddress( GraphDatabaseAPI graphDatabase ) + public static String backupAddressCc( GraphDatabaseAPI graphDatabase ) + { + ListenSocketAddress hostnamePort = graphDatabase.getDependencyResolver().resolveDependency( Config.class ).get( + listenAddress( "causal_clustering.transaction_listen_address", 6000 ) ); + + return hostnamePort.toString(); + } + + public static String backupAddressHa( GraphDatabaseAPI graphDatabase ) { HostnamePort hostnamePort = graphDatabase .getDependencyResolver()