Skip to content

Commit

Permalink
Remove possibility to recover database directly from consistency chec…
Browse files Browse the repository at this point in the history 
…ker, remove -recovery option.

 Previously consistency checker tried to recover database in case if non clean store was detected.
 We are not able to do it correctly at the moment, also it's not a CC responsibility to do so.

 From now on we will require cleanly shutdown store.
  • Loading branch information
@MishaDemianenko
MishaDemianenko committed Mar 31, 2016
1 parent 44e70c5 commit e73cf34
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 105 deletions.
54 changes: 19 additions & 35 deletions community/consistency-check/src/main/java/org/neo4j/consistency/ConsistencyCheckTool.java
View file
Expand Up @@ -26,7 +26,6 @@
import java.util.Map; import java.util.Map;


import org.neo4j.consistency.checking.full.ConsistencyCheckIncompleteException; import org.neo4j.consistency.checking.full.ConsistencyCheckIncompleteException;
import org.neo4j.graphdb.factory.GraphDatabaseFactory;
import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.helpers.Args; import org.neo4j.helpers.Args;
import org.neo4j.helpers.Strings; import org.neo4j.helpers.Strings;
Expand All @@ -47,17 +46,13 @@


public class ConsistencyCheckTool public class ConsistencyCheckTool
{ {
private static final String RECOVERY = "recovery";
private static final String CONFIG = "config"; private static final String CONFIG = "config";
private static final String PROP_OWNER = "propowner"; private static final String PROP_OWNER = "propowner";
private static final String VERBOSE = "v"; private static final String VERBOSE = "v";


// Use the ExitHandle from consistency-check-legacy so that ConsistencyCheckToolTest can properly
// test everything with -experimental and w/o it.

public static void main( String[] args ) throws IOException public static void main( String[] args ) throws IOException
{ {
ConsistencyCheckTool tool = new ConsistencyCheckTool( new ConsistencyCheckService(), new GraphDatabaseFactory(), ConsistencyCheckTool tool = new ConsistencyCheckTool( new ConsistencyCheckService(),
new DefaultFileSystemAbstraction(), System.err ); new DefaultFileSystemAbstraction(), System.err );
try try
{ {
Expand All @@ -70,28 +65,26 @@ public static void main( String[] args ) throws IOException
} }


private final ConsistencyCheckService consistencyCheckService; private final ConsistencyCheckService consistencyCheckService;
private final GraphDatabaseFactory dbFactory;
private final PrintStream systemError; private final PrintStream systemError;
private final FileSystemAbstraction fs; private final FileSystemAbstraction fs;


ConsistencyCheckTool( ConsistencyCheckService consistencyCheckService, ConsistencyCheckTool( ConsistencyCheckService consistencyCheckService, FileSystemAbstraction fs,
GraphDatabaseFactory dbFactory, FileSystemAbstraction fs, PrintStream systemError ) PrintStream systemError )
{ {
this.consistencyCheckService = consistencyCheckService; this.consistencyCheckService = consistencyCheckService;
this.dbFactory = dbFactory;
this.fs = fs; this.fs = fs;
this.systemError = systemError; this.systemError = systemError;
} }


void run( String... args ) throws ToolFailureException, IOException void run( String... args ) throws ToolFailureException, IOException
{ {
Args arguments = Args.withFlags( RECOVERY, PROP_OWNER, VERBOSE ).parse( args ); Args arguments = Args.withFlags( PROP_OWNER, VERBOSE ).parse( args );


File storeDir = determineStoreDirectory( arguments ); File storeDir = determineStoreDirectory( arguments );
Config tuningConfiguration = readConfiguration( arguments ); Config tuningConfiguration = readConfiguration( arguments );
boolean verbose = isVerbose( arguments ); boolean verbose = isVerbose( arguments );


attemptRecoveryOrCheckStateOfLogicalLogs( arguments, storeDir, tuningConfiguration ); checkDbState( storeDir, tuningConfiguration );


LogProvider logProvider = FormattedLogProvider.toOutputStream( System.out ); LogProvider logProvider = FormattedLogProvider.toOutputStream( System.out );
try try
Expand All @@ -110,32 +103,24 @@ private boolean isVerbose( Args arguments )
return arguments.getBoolean( VERBOSE, false, true ); return arguments.getBoolean( VERBOSE, false, true );
} }


private void attemptRecoveryOrCheckStateOfLogicalLogs( Args arguments, File storeDir, Config tuningConfiguration ) private void checkDbState( File storeDir, Config tuningConfiguration )
{ {
if ( arguments.getBoolean( RECOVERY, false, true ) ) try ( PageCache pageCache = StandalonePageCacheFactory.createPageCache( fs, tuningConfiguration ) )
{
dbFactory.newEmbeddedDatabase( storeDir ).shutdown();
}
else
{ {
try ( PageCache pageCache = StandalonePageCacheFactory.createPageCache( fs, tuningConfiguration ) ) if ( new RecoveryRequiredChecker( fs, pageCache ).isRecoveryRequiredAt( storeDir ) )
{ {
if ( new RecoveryRequiredChecker( fs, pageCache ).isRecoveryRequiredAt( storeDir ) ) systemError.print( Strings.joinAsLines(
{ "Active logical log detected, this might be a source of inconsistencies.",
systemError.print( Strings.joinAsLines( "Please recover database before running the consistency check.",
"Active logical log detected, this might be a source of inconsistencies.", "To perform recovery please start database and perform clean shutdown." ) );
"Consider allowing the database to recover before running the consistency check.",
"Consistency checking will continue, abort if you wish to perform recovery first.", exit();
"To perform recovery before checking consistency, use the '--recovery' flag." ) );

exit();
}
}
catch ( IOException e )
{
systemError.printf( "Failure when checking for recovery state: '%s', continuing as normal.%n", e );
} }
} }
catch ( IOException e )
{
systemError.printf( "Failure when checking for recovery state: '%s', continuing as normal.%n", e );
}
} }


private File determineStoreDirectory( Args arguments ) throws ToolFailureException private File determineStoreDirectory( Args arguments ) throws ToolFailureException
Expand Down Expand Up @@ -178,9 +163,8 @@ private Config readConfiguration( Args arguments ) throws ToolFailureException
private String usage() private String usage()
{ {
return joinAsLines( return joinAsLines(
jarUsage( getClass(), "[-propowner] [-recovery] [-config <neo4j.conf>] [-v] <storedir>" ), jarUsage( getClass(), "[-propowner] [-config <neo4j.conf>] [-v] <storedir>" ),
"WHERE: -propowner also check property owner consistency (more time consuming)", "WHERE: -propowner also check property owner consistency (more time consuming)",
" -recovery to perform recovery on the store before checking",
" -config <filename> is the location of an optional properties file", " -config <filename> is the location of an optional properties file",
" containing tuning parameters for the consistency check", " containing tuning parameters for the consistency check",
" -v produce execution output", " -v produce execution output",
Expand Down
76 changes: 6 additions & 70 deletions ...unity/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckToolTest.java
View file
Expand Up @@ -33,16 +33,10 @@
import org.neo4j.consistency.ConsistencyCheckTool.ToolFailureException; import org.neo4j.consistency.ConsistencyCheckTool.ToolFailureException;
import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.GraphDatabaseService;
import org.neo4j.graphdb.Transaction; import org.neo4j.graphdb.Transaction;
import org.neo4j.graphdb.factory.GraphDatabaseFactory;
import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction;
import org.neo4j.helpers.progress.ProgressMonitorFactory; import org.neo4j.helpers.progress.ProgressMonitorFactory;
import org.neo4j.io.fs.DefaultFileSystemAbstraction; import org.neo4j.io.fs.DefaultFileSystemAbstraction;
import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.impl.transaction.log.LogPosition;
import org.neo4j.kernel.impl.transaction.log.PhysicalLogFile;
import org.neo4j.kernel.monitoring.Monitors;
import org.neo4j.kernel.recovery.Recovery;
import org.neo4j.logging.LogProvider; import org.neo4j.logging.LogProvider;
import org.neo4j.test.EphemeralFileSystemRule; import org.neo4j.test.EphemeralFileSystemRule;
import org.neo4j.test.SystemExitRule; import org.neo4j.test.SystemExitRule;
Expand Down Expand Up @@ -183,58 +177,12 @@ public void exitWithFailureIfConfigSpecifiedButConfigFileDoesNotExist() throws E
} }


@Test @Test
public void shouldExecuteRecoveryWhenStoreWasNonCleanlyShutdown() throws Exception public void failWhenStoreWasNonCleanlyShutdown() throws Exception
{ {
// Given
createGraphDbAndKillIt();

Monitors monitors = new Monitors();
Recovery.Monitor listener = mock( Recovery.Monitor.class );
monitors.addMonitorListener( listener );

// When
runConsistencyCheckToolWith( monitors, fs.get(),
"-recovery", storeDirectory.graphDbDir().getAbsolutePath() );

// Then
verify( listener ).recoveryRequired( any( LogPosition.class ) );
verify( listener ).recoveryCompleted();
}

@Test
public void shouldExitWhenRecoveryNeededButRecoveryFalseOptionSpecified() throws Exception
{
// Given
systemExitRule.expectExit( 1 ); systemExitRule.expectExit( 1 );
File storeDir = storeDirectory.graphDbDir(); createGraphDbAndKillIt();
EphemeralFileSystemAbstraction fileSystem = createDataBaseWithStateThatNeedsRecovery( storeDir );

Monitors monitors = new Monitors();
PhysicalLogFile.Monitor listener = mock( PhysicalLogFile.Monitor.class );
monitors.addMonitorListener( listener );

// When
runConsistencyCheckToolWith( monitors, fileSystem, "-recovery=false", storeDir.getAbsolutePath() );

// Then
verifyZeroInteractions( listener );
}

private EphemeralFileSystemAbstraction createDataBaseWithStateThatNeedsRecovery( File storeDir )
{
EphemeralFileSystemAbstraction fileSystem = fs.get();
final GraphDatabaseService db =
new TestGraphDatabaseFactory().setFileSystem( fileSystem ).newImpermanentDatabase( storeDir );

try ( Transaction tx = db.beginTx() )
{
db.createNode();
tx.success();
}


EphemeralFileSystemAbstraction snapshot = fileSystem.snapshot(); runConsistencyCheckToolWith( fs.get(), storeDirectory.graphDbDir().getAbsolutePath() );
db.shutdown();
return snapshot;
} }


private void createGraphDbAndKillIt() private void createGraphDbAndKillIt()
Expand All @@ -254,27 +202,15 @@ private void createGraphDbAndKillIt()
fs.snapshot( shutdownDbAction( db ) ); fs.snapshot( shutdownDbAction( db ) );
} }


private void runConsistencyCheckToolWith( Monitors monitors, FileSystemAbstraction fileSystem, String... args ) private void runConsistencyCheckToolWith( FileSystemAbstraction fileSystem, String... args )
throws IOException, ToolFailureException throws IOException, ToolFailureException
{ {
GraphDatabaseFactory graphDbFactory = new TestGraphDatabaseFactory() new ConsistencyCheckTool( mock( ConsistencyCheckService.class ), fileSystem, mock( PrintStream.class ) ).run( args );
{
@Override
public GraphDatabaseService newEmbeddedDatabase( File storeDir )
{
return newImpermanentDatabase( storeDir );
}
}.setFileSystem( fileSystem ).setMonitors( monitors );

new ConsistencyCheckTool( mock( ConsistencyCheckService.class ),
graphDbFactory, fileSystem, mock( PrintStream.class ) ).run( args );
} }


private void runConsistencyCheckToolWith( ConsistencyCheckService private void runConsistencyCheckToolWith( ConsistencyCheckService
consistencyCheckService, PrintStream systemError, String... args ) throws ToolFailureException, IOException consistencyCheckService, PrintStream systemError, String... args ) throws ToolFailureException, IOException
{ {
new ConsistencyCheckTool( consistencyCheckService, new GraphDatabaseFactory(), new ConsistencyCheckTool( consistencyCheckService, new DefaultFileSystemAbstraction(), systemError ).run( args );
new DefaultFileSystemAbstraction(), systemError )
.run( args );
} }
} }

0 comments on commit e73cf34

Please sign in to comment.