From d9751535683b150481b04178677a02987ebfc257 Mon Sep 17 00:00:00 2001 From: jimwebber Date: Tue, 4 Oct 2016 20:32:33 -0700 Subject: [PATCH] Unbind an instance from the cluster. Using the neo4j config to determine the database location. Re-using existing validator code, and deleted proprietary version of same. --- .../commandline/admin/CommandFailed.java | 7 ++ .../dbms/CannotWriteException.java | 28 ++++++ .../neo4j/commandline/dbms/DumpCommand.java | 35 +++---- .../commandline/dbms/StoreLockChecker.java | 54 +++++++++++ .../commandline/dbms/DumpCommandTest.java | 5 +- .../dbms/UnbindFromClusterCommand.java | 94 +++++++++---------- .../dbms/UnbindFromClusterCommandTest.java | 44 ++++----- 7 files changed, 168 insertions(+), 99 deletions(-) create mode 100644 community/dbms/src/main/java/org/neo4j/commandline/dbms/CannotWriteException.java create mode 100644 community/dbms/src/main/java/org/neo4j/commandline/dbms/StoreLockChecker.java diff --git a/community/command-line/src/main/java/org/neo4j/commandline/admin/CommandFailed.java b/community/command-line/src/main/java/org/neo4j/commandline/admin/CommandFailed.java index 829322be7e9b9..9c7bc1095a9a2 100644 --- a/community/command-line/src/main/java/org/neo4j/commandline/admin/CommandFailed.java +++ b/community/command-line/src/main/java/org/neo4j/commandline/admin/CommandFailed.java @@ -19,6 +19,8 @@ */ package org.neo4j.commandline.admin; +import org.neo4j.kernel.StoreLockException; + public class CommandFailed extends Exception { public CommandFailed( String message, Exception cause ) @@ -26,6 +28,11 @@ public CommandFailed( String message, Exception cause ) super( message, cause ); } + public CommandFailed( StoreLockException exception ) + { + super( exception ); + } + public CommandFailed( String message ) { super( message ); diff --git a/community/dbms/src/main/java/org/neo4j/commandline/dbms/CannotWriteException.java b/community/dbms/src/main/java/org/neo4j/commandline/dbms/CannotWriteException.java new file mode 100644 index 0000000000000..155f60547dee9 --- /dev/null +++ b/community/dbms/src/main/java/org/neo4j/commandline/dbms/CannotWriteException.java @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2002-2016 "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 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.commandline.dbms; + +public class CannotWriteException extends Exception +{ + public CannotWriteException( String message ) + { + super(message); + } +} diff --git a/community/dbms/src/main/java/org/neo4j/commandline/dbms/DumpCommand.java b/community/dbms/src/main/java/org/neo4j/commandline/dbms/DumpCommand.java index 640d7f675e47d..3ab3979ff2170 100644 --- a/community/dbms/src/main/java/org/neo4j/commandline/dbms/DumpCommand.java +++ b/community/dbms/src/main/java/org/neo4j/commandline/dbms/DumpCommand.java @@ -38,8 +38,8 @@ import org.neo4j.dbms.archive.Dumper; import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.helpers.Args; -import org.neo4j.io.fs.DefaultFileSystemAbstraction; import org.neo4j.kernel.StoreLockException; +import org.neo4j.kernel.impl.transaction.command.Command; import org.neo4j.kernel.internal.StoreLocker; import org.neo4j.server.configuration.ConfigLoader; @@ -53,6 +53,9 @@ public class DumpCommand implements AdminCommand { + + private final StoreLockChecker storeLockChecker; + public static class Provider extends AdminCommand.Provider { public Provider() @@ -90,6 +93,7 @@ public DumpCommand( Path homeDir, Path configDir, Dumper dumper ) this.homeDir = homeDir; this.configDir = configDir; this.dumper = dumper; + this.storeLockChecker = new StoreLockChecker(); } @Override @@ -99,7 +103,7 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed Path archive = calculateArchive( database, parse( args, "to", Paths::get ) ); Path databaseDirectory = toDatabaseDirectory( database ); - try ( Closeable ignored = withLock( databaseDirectory ) ) + try ( Closeable ignored = storeLockChecker.withLock( databaseDirectory ) ) { dump( database, databaseDirectory, archive ); } @@ -111,6 +115,11 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed { wrapIOException( e ); } + catch ( CannotWriteException e ) + { + throw new CommandFailed( "you do not have permission to dump the database -- is Neo4j running as a " + + "different user?", e ); + } } private T parse( String[] args, String argument, Function converter ) throws IncorrectUsage @@ -141,28 +150,6 @@ private Path calculateArchive( String database, Path to ) return Files.isDirectory( to ) ? to.resolve( database + ".dump" ) : to; } - private Closeable withLock( Path databaseDirectory ) throws CommandFailed - { - Path lockFile = databaseDirectory.resolve( StoreLocker.STORE_LOCK_FILENAME ); - if ( Files.exists( lockFile ) ) - { - if ( Files.isWritable( lockFile ) ) - { - StoreLocker storeLocker = new StoreLocker( new DefaultFileSystemAbstraction() ); - storeLocker.checkLock( databaseDirectory.toFile() ); - return storeLocker::release; - } - else - { - throw new CommandFailed( "you do not have permission to dump the database -- is Neo4j running as a " + - "different user?" ); - } - } - return () -> - { - }; - } - private void dump( String database, Path databaseDirectory, Path archive ) throws CommandFailed { try diff --git a/community/dbms/src/main/java/org/neo4j/commandline/dbms/StoreLockChecker.java b/community/dbms/src/main/java/org/neo4j/commandline/dbms/StoreLockChecker.java new file mode 100644 index 0000000000000..17f51fada950f --- /dev/null +++ b/community/dbms/src/main/java/org/neo4j/commandline/dbms/StoreLockChecker.java @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2002-2016 "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 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.commandline.dbms; + +import java.io.Closeable; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.neo4j.commandline.admin.CommandFailed; +import org.neo4j.io.fs.DefaultFileSystemAbstraction; +import org.neo4j.kernel.internal.StoreLocker; + +public class StoreLockChecker +{ + public Closeable withLock( Path databaseDirectory ) throws CommandFailed, CannotWriteException + { + Path lockFile = databaseDirectory.resolve( StoreLocker.STORE_LOCK_FILENAME ); + if ( Files.exists( lockFile ) ) + { + if ( Files.isWritable( lockFile ) ) + { + StoreLocker storeLocker = new StoreLocker( new DefaultFileSystemAbstraction() ); + + storeLocker.checkLock( databaseDirectory.toFile() ); + + return storeLocker::release; + } + else + { + throw new CannotWriteException( "Store is not writable. Check permissions and try again." ); + } + } + return () -> + { + }; + } +} diff --git a/community/dbms/src/test/java/org/neo4j/commandline/dbms/DumpCommandTest.java b/community/dbms/src/test/java/org/neo4j/commandline/dbms/DumpCommandTest.java index 004e2192dfa7a..29f93be02bb92 100644 --- a/community/dbms/src/test/java/org/neo4j/commandline/dbms/DumpCommandTest.java +++ b/community/dbms/src/test/java/org/neo4j/commandline/dbms/DumpCommandTest.java @@ -22,6 +22,7 @@ import java.io.Closeable; import java.io.File; import java.io.IOException; +import java.nio.file.AccessDeniedException; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.NoSuchFileException; @@ -117,7 +118,7 @@ public void shouldNotCalculateTheArchiveNameIfPassedAnExistingFile() } @Test - public void shouldRespectTheStoreLock() throws IOException, IncorrectUsage + public void shouldRespectTheStoreLock() throws IOException, IncorrectUsage, CommandFailed { Path databaseDirectory = homeDir.resolve( "data/databases/foo.db" ); Files.createDirectories( databaseDirectory ); @@ -318,7 +319,7 @@ public void shouldGiveAClearMessageIfTheArchivesParentDoesntExist() throws IOExc } } - private void execute( final String database ) throws IncorrectUsage, CommandFailed + private void execute( final String database ) throws IncorrectUsage, CommandFailed, AccessDeniedException { new DumpCommand( homeDir, configDir, dumper ) .execute( new String[]{"--database=" + database, "--to=" + archive} ); diff --git a/enterprise/core-edge/src/main/java/org/neo4j/commandline/dbms/UnbindFromClusterCommand.java b/enterprise/core-edge/src/main/java/org/neo4j/commandline/dbms/UnbindFromClusterCommand.java index a8541202a15be..31e224bf0e0eb 100644 --- a/enterprise/core-edge/src/main/java/org/neo4j/commandline/dbms/UnbindFromClusterCommand.java +++ b/enterprise/core-edge/src/main/java/org/neo4j/commandline/dbms/UnbindFromClusterCommand.java @@ -20,21 +20,27 @@ package org.neo4j.commandline.dbms; import java.io.IOException; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Optional; import org.neo4j.commandline.admin.AdminCommand; import org.neo4j.commandline.admin.CommandFailed; import org.neo4j.commandline.admin.IncorrectUsage; import org.neo4j.commandline.admin.OutsideWorld; +import org.neo4j.dbms.DatabaseManagementSystemSettings; +import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.helpers.Args; -import org.neo4j.io.fs.DefaultFileSystemAbstraction; -import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.fs.FileUtils; import org.neo4j.kernel.StoreLockException; -import org.neo4j.kernel.internal.StoreLocker; +import org.neo4j.kernel.configuration.Config; +import org.neo4j.kernel.impl.util.Converters; +import org.neo4j.kernel.impl.util.Validators; +import org.neo4j.server.configuration.ConfigLoader; import static java.lang.String.format; import static java.nio.file.Files.exists; @@ -65,17 +71,17 @@ public String description() @Override public AdminCommand create( Path homeDir, Path configDir, OutsideWorld outsideWorld ) { - return new UnbindFromClusterCommand( homeDir, new DefaultFileSystemAbstraction() ); + return new UnbindFromClusterCommand( homeDir, configDir ); } } private Path homeDir; - private FileSystemAbstraction fsa; + private Path configDir; - UnbindFromClusterCommand( Path homeDir, FileSystemAbstraction fsa ) + UnbindFromClusterCommand( Path homeDir, Path configDir ) { this.homeDir = homeDir; - this.fsa = fsa; + this.configDir = configDir; } @Override @@ -85,54 +91,38 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed try { - Path pathToSpecificDatabase = - homeDir.resolve( "data" ).resolve( "databases" ).resolve( databaseNameFrom( parsedArgs ) ); - confirmTargetDatabaseDirectoryExists( pathToSpecificDatabase ); + Config config = loadNeo4jConfig( homeDir, configDir, databaseNameFrom( parsedArgs ) ); + Path pathToSpecificDatabase = config.get( DatabaseManagementSystemSettings.database_path ).toPath(); + + Validators.CONTAINS_EXISTING_DATABASE.validate( pathToSpecificDatabase.toFile() ); confirmClusterStateDirectoryExists( Paths.get( pathToSpecificDatabase.toString(), "cluster-state" ) ); confirmTargetDirectoryIsWritable( pathToSpecificDatabase ); deleteClusterStateIn( clusterStateFrom( pathToSpecificDatabase ) ); } + catch ( StoreLockException e ) + { + throw new CommandFailed( + "Database is currently locked. Please check that no other Neo4j instance using it.", e ); + } catch ( IllegalArgumentException e ) { throw new IncorrectUsage( e.getMessage() ); } - catch ( UnbindFailureException e ) + catch ( UnbindFailureException | CannotWriteException e ) { throw new CommandFailed( "Unbind failed: " + e.getMessage(), e ); } } - private Path clusterStateFrom( Path target ) - { - return Paths.get( target.toString(), CLUSTER_STATE_DIRECTORY_NAME ); - } - - private void confirmTargetDirectoryIsWritable( Path dbDir ) throws CommandFailed + private void confirmTargetDirectoryIsWritable( Path pathToSpecificDatabase ) + throws CommandFailed, CannotWriteException { - Path lockFile = dbDir.resolve( StoreLocker.STORE_LOCK_FILENAME ); - if ( exists( lockFile ) ) - { - if ( Files.isWritable( lockFile ) ) - { - StoreLocker storeLocker = new StoreLocker( fsa ); - try - { - storeLocker.checkLock( dbDir.toFile() ); - } - catch ( StoreLockException e ) - { - throw new CommandFailed( "Database is currently locked. Is a Neo4j instance still using it?" ); - } - } - } + new StoreLockChecker().withLock( pathToSpecificDatabase ); } - private void confirmTargetDatabaseDirectoryExists( Path target ) + private Path clusterStateFrom( Path target ) { - if ( !exists( target ) ) - { - throw new IllegalArgumentException( format( "Database %s does not exist", target ) ); - } + return Paths.get( target.toString(), CLUSTER_STATE_DIRECTORY_NAME ); } private void confirmClusterStateDirectoryExists( Path clusterStateDirectory ) throws UnbindFailureException @@ -157,16 +147,7 @@ private void deleteClusterStateIn( Path target ) throws UnbindFailureException private String databaseNameFrom( Args parsedArgs ) { - String databaseName = parsedArgs.get( "database" ); - if ( databaseName == null ) - { - throw new IllegalArgumentException( - "No database name specified. Usage: neo4j-admin " + "unbind --database-" ); - } - else - { - return databaseName; - } + return parsedArgs.interpretOption( "database", Converters.mandatory(), s -> s ); } private class UnbindFailureException extends Exception @@ -181,4 +162,19 @@ private class UnbindFailureException extends Exception super( format( message, args ) ); } } + + private static Config loadNeo4jConfig( Path homeDir, Path configDir, String databaseName ) + { + ConfigLoader configLoader = new ConfigLoader( settings() ); + Config config = configLoader.loadConfig( Optional.of( homeDir.toFile() ), + Optional.of( configDir.resolve( "neo4j.conf" ).toFile() ) ); + Map additionalConfig = new HashMap<>(); + additionalConfig.put( DatabaseManagementSystemSettings.active_database.name(), databaseName ); + return config.with( additionalConfig ); + } + + private static List> settings() + { + return Arrays.asList( GraphDatabaseSettings.class, DatabaseManagementSystemSettings.class ); + } } diff --git a/enterprise/core-edge/src/test/java/org/neo4j/commandline/dbms/UnbindFromClusterCommandTest.java b/enterprise/core-edge/src/test/java/org/neo4j/commandline/dbms/UnbindFromClusterCommandTest.java index 4c95a4893df93..b668400deda2c 100644 --- a/enterprise/core-edge/src/test/java/org/neo4j/commandline/dbms/UnbindFromClusterCommandTest.java +++ b/enterprise/core-edge/src/test/java/org/neo4j/commandline/dbms/UnbindFromClusterCommandTest.java @@ -23,7 +23,6 @@ import org.junit.Test; import java.io.IOException; -import java.io.PrintStream; import java.nio.channels.FileChannel; import java.nio.channels.FileLock; import java.nio.file.Files; @@ -33,7 +32,6 @@ import org.neo4j.commandline.admin.CommandFailed; import org.neo4j.commandline.admin.IncorrectUsage; -import org.neo4j.commandline.admin.OutsideWorld; import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction; import org.neo4j.io.fs.DefaultFileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction; @@ -60,7 +58,7 @@ public void shouldFailIfDatabaseNotSpecified() throws Exception fsa.mkdir( testDir.directory() ); UnbindFromClusterCommand command = - new UnbindFromClusterCommand( testDir.directory().toPath(), fsa ); + new UnbindFromClusterCommand( testDir.directory().toPath(), testDir.directory( "conf" ).toPath() ); try { @@ -71,7 +69,7 @@ public void shouldFailIfDatabaseNotSpecified() throws Exception catch ( IncorrectUsage e ) { // then - assertThat( e.getMessage(), containsString( "No database name specified" ) ); + assertThat( e.getMessage(), containsString( "Missing argument 'database'" ) ); } } @@ -83,7 +81,7 @@ public void shouldFailIfSpecifiedDatabaseDoesNotExist() throws Exception fsa.mkdir( testDir.directory() ); UnbindFromClusterCommand command = - new UnbindFromClusterCommand( testDir.directory().toPath(), fsa ); + new UnbindFromClusterCommand( testDir.directory().toPath(), testDir.directory( "conf" ).toPath() ); try { @@ -94,7 +92,7 @@ public void shouldFailIfSpecifiedDatabaseDoesNotExist() throws Exception catch ( IncorrectUsage e ) { // then - assertThat( e.getMessage(), containsString( "does not exist" ) ); + assertThat( e.getMessage(), containsString( "does not contain a database" ) ); } } @@ -106,9 +104,9 @@ public void shouldFailToUnbindLiveDatabase() throws Exception fsa.mkdir( testDir.directory() ); UnbindFromClusterCommand command = - new UnbindFromClusterCommand( testDir.directory().toPath(), fsa ); + new UnbindFromClusterCommand( testDir.directory().toPath(), testDir.directory( "conf" ).toPath() ); - Path fakeDbDir = createLockedFakeDbDir( testDir.directory().toPath() ); + FileLock fileLock = createLockedFakeDbDir( testDir.directory().toPath() ); try { @@ -119,8 +117,12 @@ public void shouldFailToUnbindLiveDatabase() throws Exception catch ( CommandFailed e ) { // then - assertThat( e.getMessage(), - containsString( "Database is currently locked. Is a Neo4j instance still using it?" ) ); + assertThat( e.getMessage(), containsString( + "Database is currently locked. Please check that no other Neo4j instance using it." ) ); + } + finally + { + fileLock.release(); } } @@ -135,7 +137,7 @@ public void shouldRemoveClusterStateDirectoryForGivenDatabase() throws Exception Path fakeDbDir = createUnlockedFakeDbDir( testDir.directory().toPath() ); UnbindFromClusterCommand command = - new UnbindFromClusterCommand( testDir.directory().toPath(), fsa ); + new UnbindFromClusterCommand( testDir.directory().toPath(), testDir.directory( "conf" ).toPath() ); // when command.execute( databaseName( "graph.db" ) ); @@ -155,7 +157,7 @@ public void shouldReportWhenClusterStateDirectoryIsNotPresent() throws Exception Files.delete( Paths.get( fakeDbDir.toString(), "cluster-state" ) ); UnbindFromClusterCommand command = - new UnbindFromClusterCommand( testDir.directory().toPath(), fsa ); + new UnbindFromClusterCommand( testDir.directory().toPath(), testDir.directory( "conf" ).toPath() ); // when try @@ -177,15 +179,17 @@ private String[] databaseName( String databaseName ) private Path createUnlockedFakeDbDir( Path parent ) throws IOException { - return createFakeDbDir( parent, false ); + Path fakeDbDir = createFakeDbDir( parent ); + Files.createFile( Paths.get( fakeDbDir.toString(), STORE_LOCK_FILENAME ) ); + return fakeDbDir; } - private Path createLockedFakeDbDir( Path parent ) throws IOException + private FileLock createLockedFakeDbDir( Path parent ) throws IOException { - return createFakeDbDir( parent, true ); + return createLockedStoreLockFileIn( createFakeDbDir( parent ) ); } - private Path createFakeDbDir( Path parent, boolean locked ) throws IOException + private Path createFakeDbDir( Path parent ) throws IOException { Path data = createDirectory( parent, "data" ); Path database = createDirectory( data, "databases" ); @@ -225,14 +229,6 @@ private Path createFakeDbDir( Path parent, boolean locked ) throws IOException createFile( graph_db, "neostore.schemastore.db" ); createFile( graph_db, "neostore.schemastore.db.id" ); createFile( graph_db, "neostore.transaction.db.0" ); - if ( locked ) - { - createLockedStoreLockFileIn( graph_db ); - } - else - { - createFile( graph_db, "store_lock" ); - } return graph_db; }