Skip to content

Commit

Permalink
Backup temporary directory rotation fix
Browse files Browse the repository at this point in the history 
FileMoveActions on non page cache files are copied. This is evident when
backups are made to a temporary directory and then renamed when
successful.

This change fixes file move actions to always move and not copy.

Renamed FileMoveAction helpers from `copy` to `move`
  • Loading branch information
@phughk
phughk committed Sep 25, 2018
1 parent b0bb221 commit d24d349
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 12 deletions.
2 changes: 1 addition & 1 deletion community/kernel/src/test/java/org/neo4j/test/rule/EmbeddedDatabaseRule.java
View file
Expand Up @@ -71,7 +71,7 @@ protected GraphDatabaseFactory newFactory()
@Override
protected GraphDatabaseBuilder newBuilder( GraphDatabaseFactory factory )
{
return factory.newEmbeddedDatabaseBuilder( testDirectory.graphDbDir() );
return factory.newEmbeddedDatabaseBuilder( getStoreDir() );
}

@Override
Expand Down
1 change: 1 addition & 0 deletions enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupCopyService.java
View file
Expand Up @@ -70,6 +70,7 @@ public void moveBackupLocation( Path oldLocation, Path newLocation ) throws IOEx
{
moves.next().move( target );
}
oldLocation.toFile().delete();
}
catch ( IOException e )
{
Expand Down
10 changes: 4 additions & 6 deletions enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupStrategyWrapper.java
View file
Expand Up @@ -74,8 +74,7 @@ Fallible<BackupStrategyOutcome> doBackup( OnlineBackupContext onlineBackupContex
return state;
}

private Fallible<BackupStrategyOutcome> performBackupWithoutLifecycle(
OnlineBackupContext onlineBackupContext )
private Fallible<BackupStrategyOutcome> performBackupWithoutLifecycle( OnlineBackupContext onlineBackupContext )
{
Path backupLocation = onlineBackupContext.getResolvedLocationFromName();
Path userSpecifiedBackupLocation = onlineBackupContext.getResolvedLocationFromName();
Expand Down Expand Up @@ -136,8 +135,7 @@ private void clearIdFiles( Path backupLocation )
* @param onlineBackupContext command line arguments, config etc.
* @return outcome of full backup
*/
private Fallible<BackupStageOutcome> fullBackupWithTemporaryFolderResolutions(
OnlineBackupContext onlineBackupContext )
private Fallible<BackupStageOutcome> fullBackupWithTemporaryFolderResolutions( OnlineBackupContext onlineBackupContext )
{
Path userSpecifiedBackupLocation = onlineBackupContext.getResolvedLocationFromName();
Path temporaryFullBackupLocation = backupCopyService.findAnAvailableLocationForNewFullBackup( userSpecifiedBackupLocation );
Expand All @@ -146,12 +144,12 @@ private Fallible<BackupStageOutcome> fullBackupWithTemporaryFolderResolutions(
Fallible<BackupStageOutcome> state = backupStrategy.performFullBackup( temporaryFullBackupLocation, config, address );

// NOTE temporaryFullBackupLocation can be equal to desired
boolean aBackupAlreadyExisted = userSpecifiedBackupLocation.equals( temporaryFullBackupLocation );
boolean backupWasMadeToATemporaryLocation = !userSpecifiedBackupLocation.equals( temporaryFullBackupLocation );

if ( BackupStageOutcome.SUCCESS.equals( state.getState() ) )
{
backupRecoveryService.recoverWithDatabase( temporaryFullBackupLocation, pageCache, config );
if ( !aBackupAlreadyExisted )
if ( backupWasMadeToATemporaryLocation )
{
try
{
Expand Down
54 changes: 54 additions & 0 deletions enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandCcIT.java
View file
Expand Up @@ -71,9 +71,11 @@
import org.neo4j.util.TestHelpers;

import static java.lang.String.format;
import static java.util.Collections.emptyMap;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeFalse;

Expand Down Expand Up @@ -317,6 +319,45 @@ public void ipv6Enabled() throws Exception
}
}

@Test
public void backupRenamesWork() throws Exception
{
// given a prexisting backup from a different store
String backupName = "preexistingBackup_" + recordFormat;
Cluster cluster = startCluster( recordFormat );
String firstBackupAddress = CausalClusteringTestHelpers.transactionAddress( clusterLeader( cluster ).database() );

assertEquals( 0, runBackupToolFromOtherJvmToGetExitCode(
"--from", firstBackupAddress,
"--cc-report-dir=" + backupDir,
"--backup-dir=" + backupDir,
"--name=" + backupName ) );
DbRepresentation firstDatabaseRepresentation = DbRepresentation.of( clusterLeader( cluster ).database() );

// and a different database
Cluster cluster2 = startCluster2( recordFormat );
DbRepresentation secondDatabaseRepresentation = DbRepresentation.of( clusterLeader( cluster2 ).database() );
assertNotEquals( firstDatabaseRepresentation, secondDatabaseRepresentation );
String secondBackupAddress = CausalClusteringTestHelpers.transactionAddress( clusterLeader( cluster2 ).database() );

// when backup is performed
assertEquals( 0, runBackupToolFromOtherJvmToGetExitCode(
"--from", secondBackupAddress,
"--cc-report-dir=" + backupDir,
"--backup-dir=" + backupDir,
"--name=" + backupName ) );
cluster2.shutdown();

// then the new backup has the correct name
assertEquals( secondDatabaseRepresentation, getBackupDbRepresentation( backupName, backupDir ) );

// and the old backup is in a renamed location
assertEquals( firstDatabaseRepresentation, getBackupDbRepresentation( backupName + ".err.0", backupDir ) );

// and the data isn't equal (sanity check)
assertNotEquals( firstDatabaseRepresentation, secondDatabaseRepresentation );
}

static PrintStream wrapWithNormalOutput( PrintStream normalOutput, PrintStream nullAbleOutput )
{
if ( nullAbleOutput == null )
Expand Down Expand Up @@ -409,6 +450,19 @@ private Cluster startIpv6Cluster() throws ExecutionException, InterruptedExcepti
return cluster;
}

private Cluster startCluster2( String recordFormat ) throws ExecutionException, InterruptedException
{
Map<String,String> sharedParams = new HashMap<>( );
sharedParams.put( GraphDatabaseSettings.record_format.name(), recordFormat );
Cluster cluster =
new Cluster( testDirectory.directory( "cluster-b_" + recordFormat ), 3, 0, new SharedDiscoveryServiceFactory(), sharedParams, emptyMap(),
sharedParams, emptyMap(),
recordFormat, IpFamily.IPV4, false );
cluster.start();
createSomeData( cluster );
return cluster;
}

public static DbRepresentation createSomeData( Cluster cluster )
{
try
Expand Down
89 changes: 85 additions & 4 deletions enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandHaIT.java
View file
Expand Up @@ -48,6 +48,8 @@
import org.neo4j.graphdb.Node;
import org.neo4j.graphdb.RelationshipType;
import org.neo4j.graphdb.Transaction;
import org.neo4j.graphdb.factory.GraphDatabaseBuilder;
import org.neo4j.graphdb.factory.GraphDatabaseFactory;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.io.fs.DefaultFileSystemAbstraction;
import org.neo4j.kernel.configuration.Config;
Expand All @@ -66,6 +68,7 @@
import static org.hamcrest.Matchers.greaterThan;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeFalse;
Expand Down Expand Up @@ -110,6 +113,19 @@ private static void createSomeData( GraphDatabaseService db )
}
}

private static void createSpecificNodePair( GraphDatabaseService db, String name )
{
try ( Transaction tx = db.beginTx() )
{
Node left = db.createNode();
left.setProperty( "name", name + "Left" );
Node right = db.createNode();
right.setProperty( "name", name + "Right" );
right.createRelationshipTo( left, RelationshipType.withName( "KNOWS" ) );
tx.success();
}
}

@Before
public void resetTasks()
{
Expand Down Expand Up @@ -143,15 +159,15 @@ public void makeSureBackupCanBePerformedWithCustomPort() throws Exception
"--cc-report-dir=" + backupDir,
"--backup-dir=" + backupDir,
"--name=" + backupName ) );
assertEquals( getDbRepresentation(), getBackupDbRepresentation( backupName ) );
assertEquals( DbRepresentation.of( db ), getBackupDbRepresentation( backupName ) );
createSomeData( db );
assertEquals(
0,
runBackupTool( testDirectory.absolutePath(), "--from", "127.0.0.1:" + backupPort,
"--cc-report-dir=" + backupDir,
"--backup-dir=" + backupDir,
"--name=" + backupName ) );
assertEquals( getDbRepresentation(), getBackupDbRepresentation( backupName ) );
assertEquals( DbRepresentation.of( db ), getBackupDbRepresentation( backupName ) );
}

@Test
Expand Down Expand Up @@ -289,6 +305,48 @@ public void reportsProgress() throws Exception
assertFalse( output.contains( "Finished receiving index snapshots" ) );
}

@Test
public void backupRenamesWork() throws Exception
{
// given a prexisting backup from a different store
String backupName = "preexistingBackup_" + recordFormat;
int firstBackupPort = PortAuthority.allocatePort();
startDb( firstBackupPort );
createSpecificNodePair( db, "first" );

assertEquals( 0, runSameJvm( backupDir, backupName,
"--from", "127.0.0.1:" + firstBackupPort,
"--cc-report-dir=" + backupDir,
"--protocol=common",
"--backup-dir=" + backupDir,
"--name=" + backupName ) );
DbRepresentation firstDatabaseRepresentation = DbRepresentation.of( db );

// and a different database
int secondBackupPort = PortAuthority.allocatePort();
GraphDatabaseService db2 = createDb2( secondBackupPort );
createSpecificNodePair( db2, "second" );
DbRepresentation secondDatabaseRepresentation = DbRepresentation.of( db2 );

// when backup is performed
assertEquals( 0, runSameJvm(backupDir, backupName,
"--from", "127.0.0.1:" + secondBackupPort,
"--cc-report-dir=" + backupDir,
"--backup-dir=" + backupDir,
"--protocol=common",
"--name=" + backupName ) );

// then the new backup has the correct name
assertEquals( secondDatabaseRepresentation, getBackupDbRepresentation( backupName ) );

// and the old backup is in a renamed location
assertEquals( firstDatabaseRepresentation, getBackupDbRepresentation( backupName + ".err.0" ) );

// and the data isn't equal (sanity check)
assertNotEquals( firstDatabaseRepresentation, secondDatabaseRepresentation );
db2.shutdown();
}

private void repeatedlyPopulateDatabase( GraphDatabaseService db, AtomicBoolean continueFlagReference )
{
while ( continueFlagReference.get() )
Expand All @@ -313,7 +371,21 @@ private void createIndexes( GraphDatabaseService db )
}
}

private GraphDatabaseService createDb2( Integer backupPort )
{
File storeDir = testDirectory.directory("graph-db-2");
GraphDatabaseFactory factory = new GraphDatabaseFactory();
GraphDatabaseBuilder builder = factory.newEmbeddedDatabaseBuilder( storeDir );
builder.setConfig( OnlineBackupSettings.online_backup_server, "0.0.0.0:" + backupPort );
return builder.newGraphDatabase();
}

private void startDb( Integer backupPort )
{
startDb( db, backupPort );
}

private void startDb( EmbeddedDatabaseRule db, Integer backupPort )
{
db.setConfig( GraphDatabaseSettings.record_format, recordFormat );
db.setConfig( OnlineBackupSettings.online_backup_enabled, Settings.TRUE );
Expand All @@ -333,9 +405,18 @@ private static int runBackupTool( File neo4jHome, String... args )
return runBackupToolFromOtherJvmToGetExitCode( neo4jHome, args );
}

private DbRepresentation getDbRepresentation()
private static int runSameJvm( File home, String name, String... args )
{
return DbRepresentation.of( db );
try
{
new OnlineBackupCommandBuilder().withRawArgs( args ).backup( home, name );
return 0;
}
catch ( Exception e )
{
e.printStackTrace();
return 1;
}
}

private DbRepresentation getBackupDbRepresentation( String name )
Expand Down
1 change: 0 additions & 1 deletion enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java
View file
Expand Up @@ -107,6 +107,5 @@ public static String backupAddressHa( GraphDatabaseAPI graphDatabase )

return hostnamePort.toString();
}

}

0 comments on commit d24d349

Please sign in to comment.