From e73cf340ce64dcbc98d36c9b72e7bf0949262961 Mon Sep 17 00:00:00 2001 From: Mikhaylo Demianenko Date: Thu, 31 Mar 2016 15:03:00 +0200 Subject: [PATCH] Remove possibility to recover database directly from consistency checker, 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. --- .../consistency/ConsistencyCheckTool.java | 54 +++++-------- .../consistency/ConsistencyCheckToolTest.java | 76 ++----------------- 2 files changed, 25 insertions(+), 105 deletions(-) diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/ConsistencyCheckTool.java b/community/consistency-check/src/main/java/org/neo4j/consistency/ConsistencyCheckTool.java index 75f081f95945a..ccd6248f2a0d9 100644 --- a/community/consistency-check/src/main/java/org/neo4j/consistency/ConsistencyCheckTool.java +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/ConsistencyCheckTool.java @@ -26,7 +26,6 @@ import java.util.Map; import org.neo4j.consistency.checking.full.ConsistencyCheckIncompleteException; -import org.neo4j.graphdb.factory.GraphDatabaseFactory; import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.helpers.Args; import org.neo4j.helpers.Strings; @@ -47,17 +46,13 @@ public class ConsistencyCheckTool { - private static final String RECOVERY = "recovery"; private static final String CONFIG = "config"; private static final String PROP_OWNER = "propowner"; 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 { - ConsistencyCheckTool tool = new ConsistencyCheckTool( new ConsistencyCheckService(), new GraphDatabaseFactory(), + ConsistencyCheckTool tool = new ConsistencyCheckTool( new ConsistencyCheckService(), new DefaultFileSystemAbstraction(), System.err ); try { @@ -70,28 +65,26 @@ public static void main( String[] args ) throws IOException } private final ConsistencyCheckService consistencyCheckService; - private final GraphDatabaseFactory dbFactory; private final PrintStream systemError; private final FileSystemAbstraction fs; - ConsistencyCheckTool( ConsistencyCheckService consistencyCheckService, - GraphDatabaseFactory dbFactory, FileSystemAbstraction fs, PrintStream systemError ) + ConsistencyCheckTool( ConsistencyCheckService consistencyCheckService, FileSystemAbstraction fs, + PrintStream systemError ) { this.consistencyCheckService = consistencyCheckService; - this.dbFactory = dbFactory; this.fs = fs; this.systemError = systemError; } 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 ); Config tuningConfiguration = readConfiguration( arguments ); boolean verbose = isVerbose( arguments ); - attemptRecoveryOrCheckStateOfLogicalLogs( arguments, storeDir, tuningConfiguration ); + checkDbState( storeDir, tuningConfiguration ); LogProvider logProvider = FormattedLogProvider.toOutputStream( System.out ); try @@ -110,32 +103,24 @@ private boolean isVerbose( Args arguments ) 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 ) ) - { - dbFactory.newEmbeddedDatabase( storeDir ).shutdown(); - } - else + try ( PageCache pageCache = StandalonePageCacheFactory.createPageCache( fs, tuningConfiguration ) ) { - 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.", - "Consider allowing the database to recover before running the consistency check.", - "Consistency checking will continue, abort if you wish to perform recovery first.", - "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 ); + systemError.print( Strings.joinAsLines( + "Active logical log detected, this might be a source of inconsistencies.", + "Please recover database before running the consistency check.", + "To perform recovery please start database and perform clean shutdown." ) ); + + exit(); } } + catch ( IOException e ) + { + systemError.printf( "Failure when checking for recovery state: '%s', continuing as normal.%n", e ); + } } private File determineStoreDirectory( Args arguments ) throws ToolFailureException @@ -178,9 +163,8 @@ private Config readConfiguration( Args arguments ) throws ToolFailureException private String usage() { return joinAsLines( - jarUsage( getClass(), "[-propowner] [-recovery] [-config ] [-v] " ), + jarUsage( getClass(), "[-propowner] [-config ] [-v] " ), "WHERE: -propowner also check property owner consistency (more time consuming)", - " -recovery to perform recovery on the store before checking", " -config is the location of an optional properties file", " containing tuning parameters for the consistency check", " -v produce execution output", diff --git a/community/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckToolTest.java b/community/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckToolTest.java index 5a25d8c285b9d..70d761e759bb2 100644 --- a/community/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckToolTest.java +++ b/community/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckToolTest.java @@ -33,16 +33,10 @@ import org.neo4j.consistency.ConsistencyCheckTool.ToolFailureException; import org.neo4j.graphdb.GraphDatabaseService; 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.io.fs.DefaultFileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction; 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.test.EphemeralFileSystemRule; import org.neo4j.test.SystemExitRule; @@ -183,58 +177,12 @@ public void exitWithFailureIfConfigSpecifiedButConfigFileDoesNotExist() throws E } @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 ); - File storeDir = storeDirectory.graphDbDir(); - 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(); - } + createGraphDbAndKillIt(); - EphemeralFileSystemAbstraction snapshot = fileSystem.snapshot(); - db.shutdown(); - return snapshot; + runConsistencyCheckToolWith( fs.get(), storeDirectory.graphDbDir().getAbsolutePath() ); } private void createGraphDbAndKillIt() @@ -254,27 +202,15 @@ private void createGraphDbAndKillIt() fs.snapshot( shutdownDbAction( db ) ); } - private void runConsistencyCheckToolWith( Monitors monitors, FileSystemAbstraction fileSystem, String... args ) + private void runConsistencyCheckToolWith( FileSystemAbstraction fileSystem, String... args ) throws IOException, ToolFailureException { - GraphDatabaseFactory graphDbFactory = new TestGraphDatabaseFactory() - { - @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 ); + new ConsistencyCheckTool( mock( ConsistencyCheckService.class ), fileSystem, mock( PrintStream.class ) ).run( args ); } private void runConsistencyCheckToolWith( ConsistencyCheckService consistencyCheckService, PrintStream systemError, String... args ) throws ToolFailureException, IOException { - new ConsistencyCheckTool( consistencyCheckService, new GraphDatabaseFactory(), - new DefaultFileSystemAbstraction(), systemError ) - .run( args ); + new ConsistencyCheckTool( consistencyCheckService, new DefaultFileSystemAbstraction(), systemError ).run( args ); } }