Addressing pull request comments.

neo4j · Jan 16, 2017 · 25e5700 · 25e5700
1 parent 32be0cd
commit 25e5700
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 41 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/StoreUpgrader.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/StoreUpgrader.java
@@ -22,6 +22,8 @@
 import java.io.File;
 import java.io.FilenameFilter;
 import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.NoSuchFileException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.regex.Pattern;
@@ -244,7 +246,7 @@ private void migrateToIsolatedDirectory( File storeDir, File migrationDirectory,
                 index++;
             }
         }
-        catch ( IOException e )
+        catch ( IOException | UncheckedIOException e )
         {
             throw new UnableToUpgradeException( "Failure doing migration", e );
         }
@@ -256,31 +258,27 @@ private void migrateToIsolatedDirectory( File storeDir, File migrationDirectory,
 
     private void cleanMigrationDirectory( File migrationDirectory )
     {
-        // We use the page cache here to make sure that the migration directory is clean even if we are using a block
-        // device.
         try
         {
-            Iterable<FileHandle> fileHandles = pageCache.streamFilesRecursive( migrationDirectory )::iterator;
+            if ( migrationDirectory.exists() )
-            for ( FileHandle fh : fileHandles )
             {
-                fh.delete();
+                fileSystem.deleteRecursively( migrationDirectory );
             }
-        }
+            // We use the page cache here to make sure that the migration directory is clean even if we are using a
-        catch ( IOException e )
+            // block device.
-        {
-            // This means that we had no files to clean, this is fine.
-        }
-        if ( migrationDirectory.exists() )
-        {
             try
             {
-                fileSystem.deleteRecursively( migrationDirectory );
+                pageCache.streamFilesRecursive( migrationDirectory ).forEach( FileHandle.HANDLE_DELETE );
             }
-            catch ( IOException e )
+            catch ( NoSuchFileException e )
             {
-                throw new UnableToUpgradeException( "Failure deleting upgrade directory " + migrationDirectory, e );
+                // This means that we had no files to clean, this is fine.
             }
         }
+        catch ( IOException | UncheckedIOException e )
+        {
+            throw new UnableToUpgradeException( "Failure deleting upgrade directory " + migrationDirectory, e );
+        }
         fileSystem.mkdir( migrationDirectory );
     }
 

diff --git a/.../kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java b/.../kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java
@@ -40,6 +40,7 @@
 import java.util.function.BiConsumer;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
+import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
 import org.neo4j.helpers.collection.Iterables;
@@ -70,6 +71,7 @@
 import org.neo4j.kernel.impl.store.format.standard.MetaDataRecordFormat;
 import org.neo4j.kernel.impl.store.format.standard.NodeRecordFormat;
 import org.neo4j.kernel.impl.store.format.standard.RelationshipRecordFormat;
+import org.neo4j.kernel.impl.store.format.standard.StandardV2_0;
 import org.neo4j.kernel.impl.store.format.standard.StandardV2_1;
 import org.neo4j.kernel.impl.store.format.standard.StandardV2_2;
 import org.neo4j.kernel.impl.store.id.ReadOnlyIdGeneratorFactory;
@@ -172,6 +174,13 @@ public StoreMigrator( FileSystemAbstraction fileSystem, PageCache pageCache, Con
     public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor.Section progressMonitor,
             String versionToMigrateFrom, String versionToMigrateTo ) throws IOException
     {
+        if ( versionToMigrateFrom.equals( StandardV2_0.STORE_VERSION ) ||
+             versionToMigrateFrom.equals( StandardV2_1.STORE_VERSION ) ||
+             versionToMigrateFrom.equals( StandardV2_2.STORE_VERSION ) )
+        {
+            // These versions are not supported for block devices.
+            CustomIOConfigValidator.assertCustomIOConfigNotUsed( config, CUSTOM_IO_EXCEPTION_MESSAGE );
+        }
         // Extract information about the last transaction from legacy neostore
         File neoStore = new File( storeDir, MetaDataStore.DEFAULT_NAME );
         long lastTxId = MetaDataStore.getRecord( pageCache, neoStore, Position.LAST_TRANSACTION_ID );
@@ -367,7 +376,6 @@ private void removeDuplicateEntityProperties( File storeDir, File migrationDir,
             SchemaIndexProvider schemaIndexProvider, RecordFormats oldFormat )
             throws IOException
     {
-        CustomIOConfigValidator.assertCustomIOConfigNotUsed( config, CUSTOM_IO_EXCEPTION_MESSAGE );
         StoreFile.fileOperation( COPY, fileSystem, storeDir, migrationDir, Iterables.iterable(
                 StoreFile.PROPERTY_STORE,
                 StoreFile.PROPERTY_KEY_TOKEN_NAMES_STORE,
@@ -480,23 +488,13 @@ private void migrateWithBatchImporter( File storeDir, File migrationDir, long la
             // When migrating on a block device there might be some files only accessible via the page cache.
             try
             {
-                Iterable<FileHandle> fileHandles = pageCache.streamFilesRecursive( migrationDir )::iterator;
+                Predicate<FileHandle> fileHandlePredicate = fileHandle -> storesToDeleteFromMigratedDirectory.stream()
-                for ( FileHandle fh : fileHandles )
+                        .anyMatch( storeFile -> storeFile.fileName( StoreFileType.STORE )
-                {
+                                .equals( fileHandle.getFile().getName() ) );
-                    Predicate<StoreFile> predicate =
+                pageCache.streamFilesRecursive( migrationDir ).filter( fileHandlePredicate )
-                            storeFile -> storeFile.fileName( StoreFileType.STORE ).equals( fh.getFile().getName() );
+                        .forEach( FileHandle.HANDLE_DELETE );
-                    if ( storesToDeleteFromMigratedDirectory.stream().anyMatch( predicate ) )
-                    {
-                        final Optional<PagedFile> optionalPagedFile = pageCache.getExistingMapping( fh.getFile() );
-                        if ( optionalPagedFile.isPresent() )
-                        {
-                            optionalPagedFile.get().close();
-                        }
-                        fh.delete();
-                    }
-                }
             }
-            catch ( IOException e )
+            catch ( NoSuchFileException e )
             {
                 // This means that we had no files only present in the page cache, this is fine.
             }
@@ -558,7 +556,7 @@ private void prepareBatchImportMigration( File storeDir, File migrationDir, Reco
                 }
             }
 
-            // The id files are to be kept on the normal file system, hence we use fileOperation to copy them.
+            // The ID files are to be kept on the normal file system, hence we use fileOperation to copy them.
             StoreFile.fileOperation( COPY, fileSystem, storeDir, migrationDir, Arrays.asList( storesFilesToMigrate ),
                     true, // OK if it's not there (1.9)
                     ExistingTargetStrategy.FAIL, StoreFileType.ID);
@@ -585,8 +583,7 @@ private void prepareBatchImportMigration( File storeDir, File migrationDir, Reco
     private void createStore( File migrationDir, RecordFormats newFormat )
     {
         StoreFactory storeFactory = new StoreFactory( new File( migrationDir.getPath() ), pageCache, fileSystem,
-                newFormat,
+                newFormat, NullLogProvider.getInstance() );
-                NullLogProvider.getInstance() );
         try ( NeoStores neoStores = storeFactory.openAllNeoStores( true ) )
         {
             neoStores.getMetaDataStore();
@@ -734,7 +731,7 @@ public void moveMigratedFiles( File migrationDir, File storeDir, String versionT
                 }
             }
         }
-        catch ( IOException e )
+        catch ( NoSuchFileException e )
         {
             //This means that we had no files only present in the page cache, this is fine.
         }
@@ -773,8 +770,6 @@ public void rebuildCounts( File storeDir, String versionToMigrateFrom, String ve
         if ( StandardV2_1.STORE_VERSION.equals( versionToMigrateFrom ) ||
              StandardV2_2.STORE_VERSION.equals( versionToMigrateFrom ) )
         {
-            // These versions are not supported for block devices.
-            CustomIOConfigValidator.assertCustomIOConfigNotUsed( config, CUSTOM_IO_EXCEPTION_MESSAGE );
             // create counters from scratch
             Iterable<StoreFile> countsStoreFiles =
                     Iterables.iterable( StoreFile.COUNTS_STORE_LEFT, StoreFile.COUNTS_STORE_RIGHT );

diff --git a/community/neo4j/src/test/java/upgrade/PlatformConstraintStoreUpgradeTest.java b/community/neo4j/src/test/java/upgrade/PlatformConstraintStoreUpgradeTest.java
@@ -30,6 +30,8 @@
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
 import org.neo4j.io.fs.DefaultFileSystemAbstraction;
 import org.neo4j.io.fs.FileSystemAbstraction;
+import org.neo4j.kernel.impl.store.format.standard.StandardV2_0;
+import org.neo4j.kernel.impl.store.format.standard.StandardV2_1;
 import org.neo4j.kernel.impl.store.format.standard.StandardV2_2;
 import org.neo4j.kernel.impl.storemigration.participant.StoreMigrator;
 import org.neo4j.test.TestGraphDatabaseFactory;
@@ -58,9 +60,29 @@ public void setup()
     }
 
     @Test
-    public void shouldFailToStartWithCustomIOConfigurationTest() throws IOException
+    public void shouldFailToStartWithCustomIOConfigurationTest20() throws IOException
     {
-        prepareSampleLegacyDatabase( StandardV2_2.STORE_VERSION, fileSystem, workingDir, prepareDir );
+        String storeVersion = StandardV2_0.STORE_VERSION;
+        checkForStoreVersion( storeVersion );
+    }
+
+    @Test
+    public void shouldFailToStartWithCustomIOConfigurationTest21() throws IOException
+    {
+        String storeVersion = StandardV2_1.STORE_VERSION;
+        checkForStoreVersion( storeVersion );
+    }
+
+    @Test
+    public void shouldFailToStartWithCustomIOConfigurationTest22() throws IOException
+    {
+        String storeVersion = StandardV2_2.STORE_VERSION;
+        checkForStoreVersion( storeVersion );
+    }
+
+    protected void checkForStoreVersion( String storeVersion ) throws IOException
+    {
+        prepareSampleLegacyDatabase( storeVersion, fileSystem, workingDir, prepareDir );
         try
         {
             createGraphDatabaseService();