From 1c4f20c895a677bb6c69b4eec0dda42ada75ffe0 Mon Sep 17 00:00:00 2001 From: Mattias Persson Date: Mon, 7 Dec 2015 23:58:20 +0100 Subject: [PATCH] Progress reporting on all migration participants also makes sure that the migration progress is visible to the user by using the user log. --- .../org/neo4j/kernel/NeoStoreDataSource.java | 2 +- .../StoreMigrationParticipant.java | 14 ++- .../impl/storemigration/StoreUpgrader.java | 12 +- .../monitoring/MigrationProgressMonitor.java | 37 +++++- .../SilentMigrationProgressMonitor.java | 26 ++++- .../VisibleMigrationProgressMonitor.java | 63 +++++++++-- ...=> AbstractStoreMigrationParticipant.java} | 19 +++- .../participant/LegacyIndexMigrator.java | 38 +++++-- .../participant/SchemaIndexMigrator.java | 9 +- .../participant/StoreMigrator.java | 45 +++++--- ...CoarseBoundedProgressExecutionMonitor.java | 85 +++++++------- .../VisibleMigrationProgressMonitorTest.java | 65 +++++++++++ .../participant/LegacyIndexMigratorTest.java | 25 +++-- .../participant/SchemaIndexMigratorTest.java | 2 +- .../participant/StoreMigratorTest.java | 6 +- ...seBoundedProgressExecutionMonitorTest.java | 105 ++++++++++++++++++ .../lucene/LuceneLegacyIndexUpgrader.java | 26 ++++- .../lucene/LuceneLegacyIndexUpgraderTest.java | 18 +-- ...stAccumulatorMigrationProgressMonitor.java | 41 +++++-- .../StoreUpgraderInterruptionTestIT.java | 3 +- .../test/java/upgrade/StoreUpgraderTest.java | 17 +-- .../java/upgrade/StoreMigratorFrom19IT.java | 4 +- .../java/upgrade/StoreMigratorFrom20IT.java | 2 +- .../LegacyIndexesUpgradeTest.java | 17 ++- .../neo4j/tools/migration/StoreMigration.java | 4 +- 25 files changed, 528 insertions(+), 157 deletions(-) rename community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/{BaseStoreMigrationParticipant.java => AbstractStoreMigrationParticipant.java} (84%) create mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/monitoring/VisibleMigrationProgressMonitorTest.java create mode 100644 community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/staging/CoarseBoundedProgressExecutionMonitorTest.java diff --git a/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java b/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java index e59dab5f9b921..bb65c71204af3 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/NeoStoreDataSource.java @@ -551,7 +551,7 @@ private void upgradeStore() HighestSelectionStrategy.getInstance() ); VisibleMigrationProgressMonitor progressMonitor = - new VisibleMigrationProgressMonitor( logService.getInternalLog( StoreMigrator.class ) ); + new VisibleMigrationProgressMonitor( logService.getUserLog( StoreMigrator.class ) ); new DatabaseMigrator( progressMonitor, fs, diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/StoreMigrationParticipant.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/StoreMigrationParticipant.java index 5d0dfc85376f6..766b1cd8b1dcd 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/StoreMigrationParticipant.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/StoreMigrationParticipant.java @@ -23,7 +23,7 @@ import java.io.IOException; import org.neo4j.kernel.impl.storemigration.monitoring.MigrationProgressMonitor; -import org.neo4j.kernel.impl.storemigration.participant.BaseStoreMigrationParticipant; +import org.neo4j.kernel.impl.storemigration.participant.AbstractStoreMigrationParticipant; import org.neo4j.kernel.impl.util.UnsatisfiedDependencyException; public interface StoreMigrationParticipant @@ -31,7 +31,7 @@ public interface StoreMigrationParticipant /** * Default empty implementation of StoreMigrationParticipant */ - StoreMigrationParticipant NOT_PARTICIPATING = new BaseStoreMigrationParticipant(); + StoreMigrationParticipant NOT_PARTICIPATING = new AbstractStoreMigrationParticipant( "Nothing" ); /** * Performs migration of data this participant is responsible for if necessary. @@ -42,19 +42,19 @@ public interface StoreMigrationParticipant * * @param storeDir data to migrate. * @param migrationDir place to migrate to. - * @param progressMonitor migration progress monitor + * @param progress migration progress monitor * @param versionToMigrateFrom the version to migrate from * @throws IOException if there was an error migrating. * @throws UnsatisfiedDependencyException if one or more dependencies were unsatisfied. */ - void migrate( File storeDir, File migrationDir, MigrationProgressMonitor progressMonitor, + void migrate( File storeDir, File migrationDir, MigrationProgressMonitor.Section progress, String versionToMigrateFrom ) throws IOException; /** * After a successful migration, move all affected files from {@code upgradeDirectory} over to * the {@code workingDirectory}, effectively activating the migration changes. * @param migrationDir directory where the - * {@link #migrate(File, File, MigrationProgressMonitor, String) migration} put its files. + * {@link #migrate(File, File, MigrationProgressMonitor.Section, String) migration} put its files. * @param storeDir directory the store directory of the to move the migrated files to. * @param versionToMigrateFrom the version we have migrated from * @throws IOException if unable to move one or more files. @@ -78,4 +78,8 @@ void migrate( File storeDir, File migrationDir, MigrationProgressMonitor progres */ void cleanup( File migrationDir ) throws IOException; + /** + * @return descriptive name of this migration participant. + */ + String getName(); } 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 index 5da6e56c844b9..794a628e69b4a 100644 --- 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 @@ -31,9 +31,12 @@ import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.impl.storemigration.monitoring.MigrationProgressMonitor; +import org.neo4j.kernel.impl.storemigration.monitoring.MigrationProgressMonitor.Section; import org.neo4j.logging.Log; import org.neo4j.logging.LogProvider; +import static java.lang.String.format; + /** * A migration process to migrate {@link StoreMigrationParticipant migration participants}, if there's * need for it, before the database fully starts. Participants can @@ -144,7 +147,7 @@ public void migrateIfNeeded( File storeDirectory) cleanup( participants, migrationDirectory ); - progressMonitor.finished(); + progressMonitor.completed(); } private boolean isUpgradeAllowed() @@ -223,9 +226,14 @@ private void migrateToIsolatedDirectory( File storeDir, File migrationDirectory, { try { + int index = 1; for ( StoreMigrationParticipant participant : participants ) { - participant.migrate( storeDir, migrationDirectory, progressMonitor, versionToMigrateFrom ); + Section section = progressMonitor.startSection( + format( "%s (%d/%d)", participant.getName(), index, participants.size() ) ); + participant.migrate( storeDir, migrationDirectory, section, versionToMigrateFrom ); + section.completed(); + index++; } } catch ( IOException e ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/MigrationProgressMonitor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/MigrationProgressMonitor.java index fc89fd0b17df4..ba991415c9454 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/MigrationProgressMonitor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/MigrationProgressMonitor.java @@ -21,10 +21,41 @@ public interface MigrationProgressMonitor { + /** + * Signals that the migration process has started. + */ void started(); + /** - * @param percent 0..100 + * Signals that migration goes into section with given {@code name}. + * + * @param name descriptive name of the section to migration. + * @return {@link Section} which should be notified about progress in the given section. */ - void percentComplete(int percent); - void finished(); + Section startSection( String name ); + + /** + * The migration process has completed successfully. + */ + void completed(); + + interface Section + { + /** + * @param max max progress, which {@link #progress(long)} moves towards. + */ + void start( long max ); + + /** + * Percentage completeness for the current section. + * + * @param add progress to add towards a maximum. + */ + void progress( long add ); + + /** + * Called if this section was completed successfully. + */ + void completed(); + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/SilentMigrationProgressMonitor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/SilentMigrationProgressMonitor.java index fa070541923f6..dc7a3c68111a0 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/SilentMigrationProgressMonitor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/SilentMigrationProgressMonitor.java @@ -21,15 +21,37 @@ public class SilentMigrationProgressMonitor implements MigrationProgressMonitor { + private static final Section SECTION = new Section() + { + @Override + public void progress( long add ) + { + } + + @Override + public void start( long max ) + { + } + + @Override + public void completed() + { + } + }; + + @Override public void started() { } - public void percentComplete( int percent ) + @Override + public Section startSection( String name ) { + return SECTION; } - public void finished() + @Override + public void completed() { } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/VisibleMigrationProgressMonitor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/VisibleMigrationProgressMonitor.java index ddb0cfb24e181..5ffd28c11154b 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/VisibleMigrationProgressMonitor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/monitoring/VisibleMigrationProgressMonitor.java @@ -25,6 +25,9 @@ public class VisibleMigrationProgressMonitor implements MigrationProgressMonitor { + static final String MESSAGE_STARTED = "Starting upgrade of database"; + static final String MESSAGE_COMPLETED = "Successfully finished upgrade of database"; + private final Log log; public VisibleMigrationProgressMonitor( Log log ) @@ -35,21 +38,65 @@ public VisibleMigrationProgressMonitor( Log log ) @Override public void started() { - log.info( "Starting upgrade of database store files" ); + log.info( MESSAGE_STARTED ); } @Override - public void percentComplete( int percent ) + public Section startSection( String name ) { - if (percent % 10 == 0) - { - log.info( format( "Store upgrade %d%% complete", percent ) ); - } + log.info( "Migrating " + name + ":" ); + + return new ProgressSection(); } @Override - public void finished() + public void completed() + { + log.info( MESSAGE_COMPLETED ); + } + + private class ProgressSection implements Section { - log.info( "Finished upgrade of database store files" ); + private static final int STRIDE = 10; + + private long current; + private int currentPercent; + private long max; + + @Override + public void progress( long add ) + { + current += add; + int percent = max == 0 ? 100 : (int) (current*100/max); + ensurePercentReported( percent ); + } + + private void ensurePercentReported( int percent ) + { + while ( currentPercent < percent ) + { + reportPercent( ++currentPercent ); + } + } + + private void reportPercent( int percent ) + { + if ( percent % STRIDE == 0 ) + { + log.info( format( " %d%% completed", percent ) ); + } + } + + @Override + public void start( long max ) + { + this.max = max; + } + + @Override + public void completed() + { + ensurePercentReported( 100 ); + } } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/BaseStoreMigrationParticipant.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/AbstractStoreMigrationParticipant.java similarity index 84% rename from community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/BaseStoreMigrationParticipant.java rename to community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/AbstractStoreMigrationParticipant.java index 85ab5144158ed..89acfb1ee3c84 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/BaseStoreMigrationParticipant.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/AbstractStoreMigrationParticipant.java @@ -31,30 +31,39 @@ * * @see org.neo4j.kernel.impl.storemigration.StoreUpgrader */ -public class BaseStoreMigrationParticipant implements StoreMigrationParticipant +public class AbstractStoreMigrationParticipant implements StoreMigrationParticipant { + protected final String name; + + public AbstractStoreMigrationParticipant( String name ) + { + this.name = name; + } + @Override - public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor progressMonitor, + public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor.Section progressMonitor, String versionToMigrateFrom ) throws IOException { - } @Override public void moveMigratedFiles( File migrationDir, File storeDir, String versionToMigrateFrom ) throws IOException { - } @Override public void rebuildCounts( File storeDir, String versionToMigrateFrom ) throws IOException { - } @Override public void cleanup( File migrationDir ) throws IOException { + } + @Override + public String getName() + { + return name; } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/LegacyIndexMigrator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/LegacyIndexMigrator.java index d834982271a34..855175d4f140c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/LegacyIndexMigrator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/LegacyIndexMigrator.java @@ -36,31 +36,33 @@ import org.neo4j.logging.LogProvider; import org.neo4j.upgrade.lucene.LegacyIndexMigrationException; import org.neo4j.upgrade.lucene.LuceneLegacyIndexUpgrader; +import org.neo4j.upgrade.lucene.LuceneLegacyIndexUpgrader.Monitor; /** * Migrates legacy lucene indexes between different neo4j versions. * Participates in store upgrade as one of the migration participants. */ -public class LegacyIndexMigrator extends BaseStoreMigrationParticipant +public class LegacyIndexMigrator extends AbstractStoreMigrationParticipant { private static final String LUCENE_LEGACY_INDEX_PROVIDER_NAME = "lucene"; - private Map indexProviders; + private final Map indexProviders; private final FileSystemAbstraction fileSystem; private File migrationLegacyIndexesRoot; private File originalLegacyIndexesRoot; - private Log log; + private final Log log; private boolean legacyIndexMigrated = false; public LegacyIndexMigrator( FileSystemAbstraction fileSystem, Map indexProviders, LogProvider logProvider ) { + super( "Legacy indexes" ); this.fileSystem = fileSystem; this.indexProviders = indexProviders; this.log = logProvider.getLog( getClass() ); } @Override - public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor progressMonitor, + public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor.Section progressMonitor, String versionToMigrateFrom ) throws IOException { IndexImplementation indexImplementation = indexProviders.get( LUCENE_LEGACY_INDEX_PROVIDER_NAME ); @@ -77,7 +79,7 @@ public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor migrationLegacyIndexesRoot = indexImplementation.getIndexImplementationDirectory( migrationDir ); if ( isNotEmptyDirectory( originalLegacyIndexesRoot ) ) { - migrateLegacyIndexes(); + migrateLegacyIndexes( progressMonitor ); legacyIndexMigrated = true; } break; @@ -116,13 +118,13 @@ private boolean isIndexMigrationDirectoryExists() return migrationLegacyIndexesRoot != null && fileSystem.fileExists( migrationLegacyIndexesRoot ); } - private void migrateLegacyIndexes() throws IOException + private void migrateLegacyIndexes( MigrationProgressMonitor.Section progressMonitor ) throws IOException { try { fileSystem.copyRecursively( originalLegacyIndexesRoot, migrationLegacyIndexesRoot ); Path indexRootPath = migrationLegacyIndexesRoot.toPath(); - LuceneLegacyIndexUpgrader indexUpgrader = createLuceneLegacyIndexUpgrader( indexRootPath ); + LuceneLegacyIndexUpgrader indexUpgrader = createLuceneLegacyIndexUpgrader( indexRootPath, progressMonitor ); indexUpgrader.upgradeIndexes(); } catch ( LegacyIndexMigrationException lime ) @@ -143,9 +145,27 @@ private boolean isNotEmptyDirectory(File file) return false; } - LuceneLegacyIndexUpgrader createLuceneLegacyIndexUpgrader( Path indexRootPath ) + LuceneLegacyIndexUpgrader createLuceneLegacyIndexUpgrader( Path indexRootPath, + MigrationProgressMonitor.Section progressMonitor ) { - return new LuceneLegacyIndexUpgrader( indexRootPath ); + return new LuceneLegacyIndexUpgrader( indexRootPath, progressMonitor( progressMonitor ) ); } + private Monitor progressMonitor( MigrationProgressMonitor.Section progressMonitor ) + { + return new Monitor() + { + @Override + public void starting( int total ) + { + progressMonitor.start( total ); + } + + @Override + public void migrated( String name ) + { + progressMonitor.progress( 1 ); + } + }; + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/SchemaIndexMigrator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/SchemaIndexMigrator.java index 38a24a5c79b3a..91f27e4cc661f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/SchemaIndexMigrator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/SchemaIndexMigrator.java @@ -38,25 +38,26 @@ *

* Since index format can be completely incompatible between version should be executed before {@link StoreMigrator} */ -public class SchemaIndexMigrator extends BaseStoreMigrationParticipant +public class SchemaIndexMigrator extends AbstractStoreMigrationParticipant { private final FileSystemAbstraction fileSystem; private boolean deleteObsoleteIndexes = false; private File labelIndexDirectory; private File schemaIndexDirectory; - private SchemaIndexProvider schemaIndexProvider; - private LabelScanStoreProvider labelScanStoreProvider; + private final SchemaIndexProvider schemaIndexProvider; + private final LabelScanStoreProvider labelScanStoreProvider; public SchemaIndexMigrator( FileSystemAbstraction fileSystem, SchemaIndexProvider schemaIndexProvider, LabelScanStoreProvider labelScanStoreProvider ) { + super( "Indexes" ); this.fileSystem = fileSystem; this.schemaIndexProvider = schemaIndexProvider; this.labelScanStoreProvider = labelScanStoreProvider; } @Override - public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor progressMonitor, + public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor.Section progressMonitor, String versionToMigrateFrom ) throws IOException { switch ( versionToMigrateFrom ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java index d5833550f0469..d78e7a9ce172c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigrator.java @@ -127,7 +127,7 @@ * * @see StoreUpgrader */ -public class StoreMigrator extends BaseStoreMigrationParticipant +public class StoreMigrator extends AbstractStoreMigrationParticipant { // Developers: There is a benchmark, storemigrate-benchmark, that generates large stores and benchmarks // the upgrade process. Please utilize that when writing upgrade code to ensure the code is fast enough to @@ -143,6 +143,7 @@ public class StoreMigrator extends BaseStoreMigrationParticipant public StoreMigrator( FileSystemAbstraction fileSystem, PageCache pageCache, Config config, LogService logService, SchemaIndexProvider schemaIndexProvider ) { + super( "Store files" ); this.fileSystem = fileSystem; this.pageCache = pageCache; this.config = config; @@ -152,7 +153,7 @@ public StoreMigrator( FileSystemAbstraction fileSystem, PageCache pageCache, Con } @Override - public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor progressMonitor, + public void migrate( File storeDir, File migrationDir, MigrationProgressMonitor.Section progressMonitor, String versionToMigrateFrom ) throws IOException { // Extract information about the last transaction from legacy neostore @@ -349,7 +350,7 @@ private void rebuildCountsFromScratch( File storeDir, long lastTxId, PageCache p private void migrateWithBatchImporter( File storeDir, File migrationDir, long lastTxId, long lastTxChecksum, long lastTxLogVersion, long lastTxLogByteOffset, PageCache pageCache, - MigrationProgressMonitor progressMonitor, String versionToUpgradeFrom ) + MigrationProgressMonitor.Section progressMonitor, String versionToUpgradeFrom ) throws IOException { prepareBatchImportMigration( storeDir, migrationDir ); @@ -372,8 +373,8 @@ private void migrateWithBatchImporter( File storeDir, File migrationDir, long la readAdditionalIds( storeDir, lastTxId, lastTxChecksum, lastTxLogVersion, lastTxLogByteOffset ); BatchImporter importer = new ParallelBatchImporter( migrationDir.getAbsoluteFile(), fileSystem, importConfig, logService, - withDynamicProcessorAssignment( migrationBatchImporterMonitor( legacyStore, progressMonitor ), - importConfig ), additionalInitialIds ); + withDynamicProcessorAssignment( migrationBatchImporterMonitor( legacyStore, progressMonitor, + importConfig ), importConfig ), additionalInitialIds ); InputIterable nodes = legacyNodesAsInput( legacyStore ); InputIterable relationships = legacyRelationshipsAsInput( legacyStore ); File badFile = new File( storeDir, Configuration.BAD_FILE_NAME ); @@ -488,17 +489,11 @@ private int readHighIdFromIdFileIfExists( File storeDir, String storeName ) thro } private ExecutionMonitor migrationBatchImporterMonitor( LegacyStore legacyStore, - MigrationProgressMonitor progressMonitor ) + final MigrationProgressMonitor.Section progressMonitor, Configuration config ) { - return new CoarseBoundedProgressExecutionMonitor( - legacyStore.getNodeStoreReader().getMaxId(), legacyStore.getRelStoreReader().getMaxId() ) - { - @Override - protected void percent( int percent ) - { - progressMonitor.percentComplete( percent ); - } - }; + return new BatchImporterProgressMonitor( + legacyStore.getNodeStoreReader().getMaxId(), legacyStore.getRelStoreReader().getMaxId(), + config, progressMonitor ); } private StoreFactory storeFactory( PageCache pageCache, File migrationDir ) @@ -891,4 +886,24 @@ public String toString() { return "Kernel StoreMigrator"; } + + private class BatchImporterProgressMonitor extends CoarseBoundedProgressExecutionMonitor + { + private final MigrationProgressMonitor.Section progressMonitor; + + BatchImporterProgressMonitor( long highNodeId, long highRelationshipId, + org.neo4j.unsafe.impl.batchimport.staging.Configuration configuration, + MigrationProgressMonitor.Section progressMonitor ) + { + super( highNodeId, highRelationshipId, configuration ); + this.progressMonitor = progressMonitor; + this.progressMonitor.start( total() ); + } + + @Override + protected void progress( long progress ) + { + progressMonitor.progress( progress ); + } + } } diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/staging/CoarseBoundedProgressExecutionMonitor.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/staging/CoarseBoundedProgressExecutionMonitor.java index 227a717f58d9d..eb46ad6c72dc4 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/staging/CoarseBoundedProgressExecutionMonitor.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/staging/CoarseBoundedProgressExecutionMonitor.java @@ -21,8 +21,6 @@ import org.neo4j.unsafe.impl.batchimport.stats.Keys; -import static java.lang.Math.max; -import static java.lang.Math.min; import static java.util.concurrent.TimeUnit.SECONDS; import static org.neo4j.helpers.collection.IteratorUtil.last; @@ -31,81 +29,74 @@ * An {@link ExecutionMonitor} that prints progress in percent, knowing the max number of nodes and relationships * in advance. */ -public class CoarseBoundedProgressExecutionMonitor extends ExecutionMonitor.Adapter +public abstract class CoarseBoundedProgressExecutionMonitor extends ExecutionMonitor.Adapter { - private long totalDoneBatches; - private final long highNodeId; - private final long highRelationshipId; - private int previousPercent; + private final long totalNumberOfBatches; + private long[] prevDoneBatches; - public CoarseBoundedProgressExecutionMonitor( long highNodeId, long highRelationshipId ) + public CoarseBoundedProgressExecutionMonitor( long highNodeId, long highRelationshipId, + Configuration configuration ) { super( 1, SECONDS ); - this.highNodeId = highNodeId; - this.highRelationshipId = highRelationshipId; + // This calculation below is aware of internals of the parallel importer and may + // be wrong for other importers. + this.totalNumberOfBatches = + (highNodeId/configuration.batchSize()) * 2 + // node records encountered twice + (highRelationshipId/configuration.batchSize()) * 3; // rel records encountered three times; + } + + protected long total() + { + return totalNumberOfBatches; } @Override public void check( StageExecution[] executions ) { - updatePercent( executions ); + update( executions ); } - private void updatePercent( StageExecution[] executions ) + @Override + public void start( StageExecution[] executions ) { - int highestPercentThere = previousPercent; - for ( StageExecution execution : executions ) - { - // This calculation below is aware of internals of the parallel importer and may - // be wrong for other importers. - long maxNumberOfBatches = - (highNodeId/execution.getConfig().batchSize()) * 2 + // node records encountered twice - (highRelationshipId/execution.getConfig().batchSize()) * 3; // rel records encountered three times; - - long doneBatches = totalDoneBatches + doneBatches( execution ); - int percentThere = (int) ((doneBatches*100D)/maxNumberOfBatches); - percentThere = min( percentThere, 100 ); - highestPercentThere = max( percentThere, highestPercentThere ); - } - - applyPercentage( highestPercentThere ); + prevDoneBatches = new long[executions.length]; } - private void applyPercentage( int percentThere ) + private void update( StageExecution[] executions ) { - while ( previousPercent < percentThere ) + long diff = 0; + for ( int i = 0; i < executions.length; i++ ) { - percent( ++previousPercent ); + long doneBatches = doneBatches( executions[i] ); + diff += doneBatches - prevDoneBatches[i]; + prevDoneBatches[i] = doneBatches; } - } - - protected void percent( int percent ) - { - // TODO An execution monitor that does not accept the writer? A kitten dies every time this happens, you know - System.out.print( "." ); - if ( percent % 10 == 0 ) + if ( diff > 0 ) { - System.out.println( " " + percent + "%" ); + progress( diff ); } } + /** + * @param progress Relative progress. + */ + protected abstract void progress( long progress ); + private long doneBatches( StageExecution execution ) { return last( execution.steps() ).stats().stat( Keys.done_batches ).asLong(); } @Override - public void end( StageExecution[] executions, long totalTimeMillis ) + public void done( long totalTimeMillis, String additionalInformation ) { - for ( StageExecution execution : executions ) + long prev = 0; + for ( long done : prevDoneBatches ) { - this.totalDoneBatches += doneBatches( execution ); + prev += done; } - } - @Override - public void done( long totalTimeMillis, String additionalInformation ) - { - applyPercentage( 100 ); + // Just report the last progress + progress( totalNumberOfBatches-prev ); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/monitoring/VisibleMigrationProgressMonitorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/monitoring/VisibleMigrationProgressMonitorTest.java new file mode 100644 index 0000000000000..c261b90960984 --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/monitoring/VisibleMigrationProgressMonitorTest.java @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2002-2015 "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.kernel.impl.storemigration.monitoring; + +import org.junit.Test; + +import org.neo4j.kernel.impl.storemigration.monitoring.MigrationProgressMonitor.Section; +import org.neo4j.logging.AssertableLogProvider; +import org.neo4j.logging.Log; + +import static org.hamcrest.Matchers.containsString; + +public class VisibleMigrationProgressMonitorTest +{ + @Test + public void shouldReportAllPercentageSteps() throws Exception + { + // GIVEN + AssertableLogProvider logProvider = new AssertableLogProvider(); + Log log = logProvider.getLog( getClass() ); + VisibleMigrationProgressMonitor monitor = new VisibleMigrationProgressMonitor( log ); + monitor.started(); + + // WHEN + monitorSection( monitor, "First", 100, 40, 25, 23 /*these are too far*/ , 10, 50 ); + monitor.completed(); + + // THEN + logProvider.assertContainsMessageContaining( VisibleMigrationProgressMonitor.MESSAGE_STARTED ); + for ( int i = 10; i <= 100; i += 10 ) + { + logProvider.assertContainsMessageContaining( String.valueOf( i ) + "%" ); + } + logProvider.assertNone( logProvider.inLog( VisibleMigrationProgressMonitor.class ).info( containsString( "110%" ) ) ); + logProvider.assertContainsMessageContaining( VisibleMigrationProgressMonitor.MESSAGE_COMPLETED ); + } + + private void monitorSection( VisibleMigrationProgressMonitor monitor, String name, int max, int... steps ) + { + Section section = monitor.startSection( name ); + section.start( max ); + for ( int step : steps ) + { + section.progress( step ); + } + section.completed(); + } +} diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/LegacyIndexMigratorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/LegacyIndexMigratorTest.java index 17881000c2668..b8bf3879134df 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/LegacyIndexMigratorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/LegacyIndexMigratorTest.java @@ -49,13 +49,13 @@ public class LegacyIndexMigratorTest { - private FileSystemAbstraction fs = mock( FileSystemAbstraction.class ); - private LogProvider logProvider = mock( LogProvider.class ); - private MigrationProgressMonitor progressMonitor = mock( MigrationProgressMonitor.class ); - private File storeDir = mock( File.class ); - private File migrationDir = mock( File.class ); - private File originalIndexStore = mock( File.class ); - private File migratedIndexStore = new File( "." ); + private final FileSystemAbstraction fs = mock( FileSystemAbstraction.class ); + private final LogProvider logProvider = mock( LogProvider.class ); + private final MigrationProgressMonitor.Section progressMonitor = mock( MigrationProgressMonitor.Section.class ); + private final File storeDir = mock( File.class ); + private final File migrationDir = mock( File.class ); + private final File originalIndexStore = mock( File.class ); + private final File migratedIndexStore = new File( "." ); @Before public void setUp() @@ -156,7 +156,7 @@ private HashMap getIndexProviders() private class TestLegacyIndexMigrator extends LegacyIndexMigrator { - private boolean successfullMigration; + private final boolean successfullMigration; public TestLegacyIndexMigrator( FileSystemAbstraction fileSystem, Map indexProviders, LogProvider logProvider, boolean successfullMigration ) @@ -166,7 +166,8 @@ public TestLegacyIndexMigrator( FileSystemAbstraction fileSystem, } @Override - LuceneLegacyIndexUpgrader createLuceneLegacyIndexUpgrader( Path indexRootPath ) + LuceneLegacyIndexUpgrader createLuceneLegacyIndexUpgrader( Path indexRootPath, + MigrationProgressMonitor.Section progressMonitor ) { return new HumbleLegacyIndexUpgrader( indexRootPath, successfullMigration ); } @@ -174,11 +175,11 @@ LuceneLegacyIndexUpgrader createLuceneLegacyIndexUpgrader( Path indexRootPath ) private class HumbleLegacyIndexUpgrader extends LuceneLegacyIndexUpgrader { - private boolean successfulMigration; + private final boolean successfulMigration; public HumbleLegacyIndexUpgrader( Path indexRootPath, boolean successfulMigration ) { - super( indexRootPath ); + super( indexRootPath, NO_MONITOR ); this.successfulMigration = successfulMigration; } @@ -191,4 +192,4 @@ public void upgradeIndexes() throws LegacyIndexMigrationException } } } -} \ No newline at end of file +} diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/SchemaIndexMigratorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/SchemaIndexMigratorTest.java index bb43d9ba50a4c..1185ee456d18c 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/SchemaIndexMigratorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/SchemaIndexMigratorTest.java @@ -37,7 +37,7 @@ public class SchemaIndexMigratorTest { private final FileSystemAbstraction fs = mock( FileSystemAbstraction.class ); - private final MigrationProgressMonitor progressMonitor = mock( MigrationProgressMonitor.class ); + private final MigrationProgressMonitor.Section progressMonitor = mock( MigrationProgressMonitor.Section.class ); private final SchemaIndexProvider schemaIndexProvider = mock( SchemaIndexProvider.class ); private final LabelScanStoreProvider labelScanStoreProvider = mock( LabelScanStoreProvider.class ); private final File storeDir = new File( "store" ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigratorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigratorTest.java index 2e6d24711b057..fa8f04c10bce6 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigratorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/storemigration/participant/StoreMigratorTest.java @@ -112,7 +112,7 @@ public void shouldBeAbleToResumeMigrationOnMoving() throws Exception StoreMigrator migrator = new StoreMigrator( fs, pageCache, new Config(), logService, schemaIndexProvider ); File migrationDir = new File( storeDirectory, StoreUpgrader.MIGRATION_DIRECTORY ); fs.mkdirs( migrationDir ); - migrator.migrate( storeDirectory, migrationDir, progressMonitor, versionToMigrateFrom ); + migrator.migrate( storeDirectory, migrationDir, progressMonitor.startSection( "Test" ), versionToMigrateFrom ); // WHEN simulating resuming the migration progressMonitor = new SilentMigrationProgressMonitor(); @@ -143,7 +143,7 @@ public void shouldBeAbleToResumeMigrationOnRebuildingCounts() throws Exception StoreMigrator migrator = new StoreMigrator( fs, pageCache, new Config(), logService, schemaIndexProvider ); File migrationDir = new File( storeDirectory, StoreUpgrader.MIGRATION_DIRECTORY ); fs.mkdirs( migrationDir ); - migrator.migrate( storeDirectory, migrationDir, progressMonitor, versionToMigrateFrom ); + migrator.migrate( storeDirectory, migrationDir, progressMonitor.startSection( "Test" ), versionToMigrateFrom ); migrator.moveMigratedFiles( migrationDir, storeDirectory, versionToMigrateFrom ); // WHEN simulating resuming the migration @@ -177,7 +177,7 @@ public void shouldComputeTheLastTxLogPositionCorrectly() throws Throwable fs.mkdirs( migrationDir ); // WHEN migrating - migrator.migrate( storeDirectory, migrationDir, progressMonitor, versionToMigrateFrom ); + migrator.migrate( storeDirectory, migrationDir, progressMonitor.startSection( "Test" ), versionToMigrateFrom ); // THEN it should compute the correct last tx log position assertEquals( expectedLogPosition, readLastTxLogPosition( fs, migrationDir ) ); diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/staging/CoarseBoundedProgressExecutionMonitorTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/staging/CoarseBoundedProgressExecutionMonitorTest.java new file mode 100644 index 0000000000000..c413874fa65cb --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/staging/CoarseBoundedProgressExecutionMonitorTest.java @@ -0,0 +1,105 @@ +/* + * Copyright (c) 2002-2015 "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.unsafe.impl.batchimport.staging; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicLong; + +import org.neo4j.helpers.ArrayUtil; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import static java.util.Arrays.asList; + +import static org.neo4j.unsafe.impl.batchimport.stats.Keys.done_batches; + +@RunWith( Parameterized.class ) +public class CoarseBoundedProgressExecutionMonitorTest +{ + @Parameterized.Parameters(name = "{0}") + public static Iterable parameters() + { + List result = new ArrayList<>(); + result.add( ArrayUtil.array( 1 ) ); + result.add( ArrayUtil.array( 10 ) ); + result.add( ArrayUtil.array( 123 ) ); + return result; + } + + @Parameter( 0 ) + public int batchSize; + + @Test + public void shouldReportProgressOnSingleExecution() throws Exception + { + // GIVEN + final AtomicLong progress = new AtomicLong(); + Configuration config = config(); + CoarseBoundedProgressExecutionMonitor monitor = new CoarseBoundedProgressExecutionMonitor( + 100 * batchSize, 100 * batchSize, config ) + { + @Override + protected void progress( long add ) + { + progress.addAndGet( add ); + } + }; + + // WHEN + monitor.start( singleExecution( 0, config ) ); + long total = monitor.total(); + long part = total / 10; + for ( int i = 0; i < 9; i++ ) + { + monitor.check( singleExecution( part * (i+1), config ) ); + assertTrue( progress.get() < total ); + } + monitor.done( 0, "Test" ); + + // THEN + assertEquals( total, progress.get() ); + } + + private StageExecution[] singleExecution( long doneBatches, Configuration config ) + { + Step step = ControlledStep.stepWithStats( "Test", 0, done_batches, doneBatches ); + StageExecution execution = new StageExecution( "Test", config, asList( step ), 0 ); + return new StageExecution[] {execution}; + } + + private Configuration config() + { + return new Configuration.Overridden( Configuration.DEFAULT ) + { + @Override + public int batchSize() + { + return batchSize; + } + }; + } +} diff --git a/community/lucene-index-upgrade/src/main/java/org/neo4j/upgrade/lucene/LuceneLegacyIndexUpgrader.java b/community/lucene-index-upgrade/src/main/java/org/neo4j/upgrade/lucene/LuceneLegacyIndexUpgrader.java index aa601ecd1f2bd..179f78a0e8a71 100644 --- a/community/lucene-index-upgrade/src/main/java/org/neo4j/upgrade/lucene/LuceneLegacyIndexUpgrader.java +++ b/community/lucene-index-upgrade/src/main/java/org/neo4j/upgrade/lucene/LuceneLegacyIndexUpgrader.java @@ -19,7 +19,6 @@ */ package org.neo4j.upgrade.lucene; -import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -45,6 +44,25 @@ */ public class LuceneLegacyIndexUpgrader { + public interface Monitor + { + /** + * Upgrade is starting. + * + * @param count number of indexes to migrate. + */ + default void starting( int count ) {} + + /** + * Called after an index has been migrated, called for each migrated index. + * + * @param name name of the index. + */ + default void migrated( String name ) {} + } + + public static final Monitor NO_MONITOR = new Monitor() {}; + private static final String LIBRARY_DIRECTORY = "lib"; private static final String RESOURCE_SEPARATOR = "/"; private static final String LUCENE4_CORE_JAR_NAME = "lucene-core-4.10.4.jar"; @@ -53,9 +71,11 @@ public class LuceneLegacyIndexUpgrader public static final String SEGMENTS_FILE_NAME_PREFIX = "segments"; private final Path indexRootPath; + private final Monitor monitor; - public LuceneLegacyIndexUpgrader( Path indexRootPath ) + public LuceneLegacyIndexUpgrader( Path indexRootPath, Monitor monitor ) { + this.monitor = monitor; if ( Files.exists( indexRootPath ) && !Files.isDirectory( indexRootPath ) ) { throw new IllegalArgumentException( "Index path should be a directory" ); @@ -75,6 +95,7 @@ public void upgradeIndexes() throws LegacyIndexMigrationException { return; } + monitor.starting( (int) Files.walk( indexRootPath ).count() ); try ( Stream pathStream = Files.walk( indexRootPath ); IndexUpgraderWrapper lucene4Upgrader = createIndexUpgrader( getLucene4JarPaths() ); IndexUpgraderWrapper lucene5Upgrader = createIndexUpgrader( getLucene5JarPaths() ) ) @@ -86,6 +107,7 @@ public void upgradeIndexes() throws LegacyIndexMigrationException { lucene4Upgrader.upgradeIndex( indexPath ); lucene5Upgrader.upgradeIndex( indexPath ); + monitor.migrated( indexPath.toFile().getName() ); } catch ( Throwable e ) { diff --git a/community/lucene-index-upgrade/src/test/java/org/neo4j/upgrade/lucene/LuceneLegacyIndexUpgraderTest.java b/community/lucene-index-upgrade/src/test/java/org/neo4j/upgrade/lucene/LuceneLegacyIndexUpgraderTest.java index 981169c126378..89917d2e483e8 100644 --- a/community/lucene-index-upgrade/src/test/java/org/neo4j/upgrade/lucene/LuceneLegacyIndexUpgraderTest.java +++ b/community/lucene-index-upgrade/src/test/java/org/neo4j/upgrade/lucene/LuceneLegacyIndexUpgraderTest.java @@ -40,6 +40,8 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.neo4j.upgrade.lucene.LuceneLegacyIndexUpgrader.NO_MONITOR; + public class LuceneLegacyIndexUpgraderTest { @Rule @@ -51,7 +53,7 @@ public void failOnFileMigration() throws Exception Path indexFolder = createPathForResource("indexPretender.txt"); expectedException.expect( IllegalArgumentException.class ); - new LuceneLegacyIndexUpgrader(indexFolder); + new LuceneLegacyIndexUpgrader( indexFolder, NO_MONITOR ); } @Test @@ -94,7 +96,7 @@ private Path createPathForResource(String resourceName) throws URISyntaxExceptio private class LegacyIndexMigrationExceptionBaseMatcher extends TypeSafeDiagnosingMatcher { - private String[] failedIndexNames; + private final String[] failedIndexNames; public LegacyIndexMigrationExceptionBaseMatcher(String... failedIndexNames) { @@ -123,8 +125,8 @@ protected boolean matchesSafely( LegacyIndexMigrationException item, Description private class TrackingLuceneLegacyIndexUpgrader extends LuceneLegacyIndexUpgrader { - private Set migratedIndexes = new HashSet<>( ); - private boolean failIndexUpgrade; + private final Set migratedIndexes = new HashSet<>( ); + private final boolean failIndexUpgrade; public TrackingLuceneLegacyIndexUpgrader( Path indexRootPath) { @@ -133,7 +135,7 @@ public TrackingLuceneLegacyIndexUpgrader( Path indexRootPath) public TrackingLuceneLegacyIndexUpgrader( Path indexRootPath, boolean failIndexUpgrade ) { - super( indexRootPath ); + super( indexRootPath, NO_MONITOR ); this.failIndexUpgrade = failIndexUpgrade; } @@ -152,8 +154,8 @@ public Set getMigratedIndexes() private class IndexUpgraderWrapperStub extends IndexUpgraderWrapper { - private Set migratedIndexes; - private boolean failIndexUpgrade; + private final Set migratedIndexes; + private final boolean failIndexUpgrade; public IndexUpgraderWrapperStub( Supplier jarLoaderSupplier, Set migratedIndexes, boolean failIndexUpgrade ) @@ -173,4 +175,4 @@ public void upgradeIndex( Path indexPath ) throws Throwable migratedIndexes.add( indexPath.getFileName().toString() ); } } -} \ No newline at end of file +} diff --git a/community/neo4j/src/test/java/upgrade/ListAccumulatorMigrationProgressMonitor.java b/community/neo4j/src/test/java/upgrade/ListAccumulatorMigrationProgressMonitor.java index 79f2c08cd862f..ad5e61e9556ef 100644 --- a/community/neo4j/src/test/java/upgrade/ListAccumulatorMigrationProgressMonitor.java +++ b/community/neo4j/src/test/java/upgrade/ListAccumulatorMigrationProgressMonitor.java @@ -19,14 +19,15 @@ */ package upgrade; -import java.util.ArrayList; -import java.util.List; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; import org.neo4j.kernel.impl.storemigration.monitoring.MigrationProgressMonitor; public class ListAccumulatorMigrationProgressMonitor implements MigrationProgressMonitor { - private final List events = new ArrayList<>(); + private final Map events = new HashMap<>(); private boolean started = false; private boolean finished = false; @@ -37,20 +38,44 @@ public void started() } @Override - public void percentComplete( int percent ) + public Section startSection( String name ) { - events.add( percent ); + final AtomicLong progress = new AtomicLong(); + assert events.put( name, progress ) == null; + return new Section() + { + @Override + public void progress( long add ) + { + progress.addAndGet( add ); + } + + @Override + public void start( long max ) + { + } + + @Override + public void completed() + { + } + }; } @Override - public void finished() + public void completed() { finished = true; } - public int eventSize() + public Map progresses() { - return events.size(); + Map result = new HashMap<>(); + for ( Map.Entry entry : events.entrySet() ) + { + result.put( entry.getKey(), entry.getValue().longValue() ); + } + return result; } public boolean isStarted() diff --git a/community/neo4j/src/test/java/upgrade/StoreUpgraderInterruptionTestIT.java b/community/neo4j/src/test/java/upgrade/StoreUpgraderInterruptionTestIT.java index 668dab5df49c6..b3c798fe65519 100644 --- a/community/neo4j/src/test/java/upgrade/StoreUpgraderInterruptionTestIT.java +++ b/community/neo4j/src/test/java/upgrade/StoreUpgraderInterruptionTestIT.java @@ -114,7 +114,8 @@ public void shouldSucceedWithUpgradeAfterPreviousAttemptDiedDuringMigration() StoreMigrator failingStoreMigrator = new StoreMigrator(fs, pageCache, config, logService, schemaIndexProvider ) { @Override - public void migrate( File sourceStoreDir, File targetStoreDir, MigrationProgressMonitor progressMonitor, + public void migrate( File sourceStoreDir, File targetStoreDir, + MigrationProgressMonitor.Section progressMonitor, String versionToMigrateFrom ) throws IOException { super.migrate( sourceStoreDir, targetStoreDir, progressMonitor, versionToMigrateFrom ); diff --git a/community/neo4j/src/test/java/upgrade/StoreUpgraderTest.java b/community/neo4j/src/test/java/upgrade/StoreUpgraderTest.java index aeb6528e4dfd5..559c9b6f261e1 100644 --- a/community/neo4j/src/test/java/upgrade/StoreUpgraderTest.java +++ b/community/neo4j/src/test/java/upgrade/StoreUpgraderTest.java @@ -31,7 +31,6 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; import java.util.List; import org.neo4j.consistency.checking.full.ConsistencyCheckIncompleteException; @@ -50,9 +49,8 @@ import org.neo4j.kernel.impl.store.MetaDataStore; import org.neo4j.kernel.impl.store.NeoStores; import org.neo4j.kernel.impl.store.StoreFactory; -import org.neo4j.kernel.impl.storemigration.DatabaseMigrator; import org.neo4j.kernel.impl.storemigration.monitoring.MigrationProgressMonitor; -import org.neo4j.kernel.impl.storemigration.participant.BaseStoreMigrationParticipant; +import org.neo4j.kernel.impl.storemigration.participant.AbstractStoreMigrationParticipant; import org.neo4j.kernel.impl.storemigration.participant.SchemaIndexMigrator; import org.neo4j.kernel.impl.storemigration.StoreMigrationParticipant; import org.neo4j.kernel.impl.storemigration.participant.StoreMigrator; @@ -308,7 +306,7 @@ public void shouldContinueMovingFilesIfUpgradeCancelledWhileMoving() throws Exce // THEN verify( observingParticipant, Mockito.times( 0 ) ).migrate( any( File.class ), any( File.class ), - any( MigrationProgressMonitor.class ), eq( versionToMigrateFrom ) ); + any( MigrationProgressMonitor.Section.class ), eq( versionToMigrateFrom ) ); verify( observingParticipant, Mockito.times( 1 ) ). moveMigratedFiles( any( File.class ), any( File.class ), eq( versionToMigrateFrom ) ); @@ -385,7 +383,7 @@ public void upgraderShouldCleanupLegacyLeftoverAndMigrationDirs() throws Excepti private StoreMigrationParticipant participantThatWillFailWhenMoving( final String failureMessage ) { - return new BaseStoreMigrationParticipant() + return new AbstractStoreMigrationParticipant( "Failing" ) { @Override public void moveMigratedFiles( File migrationDir, File storeDir, String versionToUpgradeFrom ) @@ -424,15 +422,6 @@ private StoreUpgrader newUpgrader( UpgradableDatabase upgradableDatabase, PageCa return upgrader; } - private DatabaseMigrator getDatabaseMigrator( PageCache pageCache, Config config ) - { - check = new StoreVersionCheck( pageCache ); - SilentMigrationProgressMonitor progressMonitor = new SilentMigrationProgressMonitor(); - NullLogService logService = NullLogService.getInstance(); - return new DatabaseMigrator( progressMonitor, fileSystem, config, logService, - schemaIndexProvider, labelScanStoreProvider, new HashMap<>(), pageCache); - } - private List migrationHelperDirs() { File[] tmpDirs = dbDirectory.listFiles( ( file, name ) -> file.isDirectory() && diff --git a/enterprise/neo4j-enterprise/src/test/java/upgrade/StoreMigratorFrom19IT.java b/enterprise/neo4j-enterprise/src/test/java/upgrade/StoreMigratorFrom19IT.java index c1c44d8a0d276..3217155954c1e 100644 --- a/enterprise/neo4j-enterprise/src/test/java/upgrade/StoreMigratorFrom19IT.java +++ b/enterprise/neo4j-enterprise/src/test/java/upgrade/StoreMigratorFrom19IT.java @@ -28,6 +28,7 @@ import java.io.IOException; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import org.neo4j.consistency.checking.full.ConsistencyCheckIncompleteException; @@ -84,7 +85,6 @@ public class StoreMigratorFrom19IT { - @Rule public final TargetDirectory.TestDirectory storeDir = TargetDirectory.testDirForTest( getClass() ); @Rule @@ -127,7 +127,7 @@ public void shouldMigrate() throws IOException, ConsistencyCheckIncompleteExcept newStoreUpgrader().migrateIfNeeded( legacyStoreDir ); // THEN - assertEquals( 100, monitor.eventSize() ); + assertEquals( 2, monitor.progresses().size() ); assertTrue( monitor.isStarted() ); assertTrue( monitor.isFinished() ); diff --git a/enterprise/neo4j-enterprise/src/test/java/upgrade/StoreMigratorFrom20IT.java b/enterprise/neo4j-enterprise/src/test/java/upgrade/StoreMigratorFrom20IT.java index f34ed055a336c..a2360ae918341 100644 --- a/enterprise/neo4j-enterprise/src/test/java/upgrade/StoreMigratorFrom20IT.java +++ b/enterprise/neo4j-enterprise/src/test/java/upgrade/StoreMigratorFrom20IT.java @@ -115,7 +115,7 @@ public void shouldMigrate() throws IOException, ConsistencyCheckIncompleteExcept upgrader( indexMigrator, storeMigrator ).migrateIfNeeded( find20FormatStoreDirectory( storeDir.directory() ) ); // THEN - assertEquals( 100, monitor.eventSize() ); + assertEquals( 2, monitor.progresses().size() ); assertTrue( monitor.isStarted() ); assertTrue( monitor.isFinished() ); diff --git a/integrationtests/src/test/java/org/neo4j/storeupgrade/LegacyIndexesUpgradeTest.java b/integrationtests/src/test/java/org/neo4j/storeupgrade/LegacyIndexesUpgradeTest.java index 99839e4737bb2..08c55655200d2 100644 --- a/integrationtests/src/test/java/org/neo4j/storeupgrade/LegacyIndexesUpgradeTest.java +++ b/integrationtests/src/test/java/org/neo4j/storeupgrade/LegacyIndexesUpgradeTest.java @@ -44,6 +44,7 @@ import org.neo4j.index.lucene.ValueContext; import org.neo4j.io.fs.FileUtils; import org.neo4j.kernel.impl.storemigration.UpgradeNotAllowedByConfigurationException; +import org.neo4j.test.SuppressOutput; import org.neo4j.test.TargetDirectory; import org.neo4j.test.TestGraphDatabaseFactory; import org.neo4j.test.Unzip; @@ -61,6 +62,9 @@ public class LegacyIndexesUpgradeTest @Rule public ExpectedException expectedException = ExpectedException.none(); + @Rule + public SuppressOutput suppressOutput = SuppressOutput.suppressAll(); + public void setUp() throws IOException { FileUtils.deleteRecursively( testDir.graphDbDir() ); @@ -81,6 +85,8 @@ public void successfulMigrationLegacyIndexes() throws Exception startDbAndCheckData(); checkIndexData(); + + checkMigrationProgressFeedback(); } @Test @@ -247,8 +253,7 @@ private IntFunction basicKeyFactory() private class NestedThrowableMatcher extends TypeSafeMatcher { - - private Class expectedType; + private final Class expectedType; public NestedThrowableMatcher( Class expectedType ) { @@ -279,4 +284,12 @@ protected boolean matchesSafely( Throwable item ) return false; } } + + private void checkMigrationProgressFeedback() + { + suppressOutput.getOutputVoice().containsMessage( "Starting upgrade of database" ); + suppressOutput.getOutputVoice().containsMessage( "Successfully finished upgrade of database" ); + suppressOutput.getOutputVoice().containsMessage( "10%" ); + suppressOutput.getOutputVoice().containsMessage( "100%" ); + } } diff --git a/tools/src/main/java/org/neo4j/tools/migration/StoreMigration.java b/tools/src/main/java/org/neo4j/tools/migration/StoreMigration.java index fbdd89a9abf9b..afe90d05a0bf0 100644 --- a/tools/src/main/java/org/neo4j/tools/migration/StoreMigration.java +++ b/tools/src/main/java/org/neo4j/tools/migration/StoreMigration.java @@ -94,7 +94,7 @@ public void run( final FileSystemAbstraction fs, final File storeDirectory, Conf StoreLogService.withUserLogProvider( userLogProvider ).inStoreDirectory( fs, storeDirectory ); VisibleMigrationProgressMonitor progressMonitor = - new VisibleMigrationProgressMonitor( logService.getInternalLog( StoreMigration.class ) ); + new VisibleMigrationProgressMonitor( logService.getUserLog( StoreMigration.class ) ); LifeSupport life = new LifeSupport(); @@ -171,7 +171,7 @@ public File storeDir() private class LegacyIndexProvider implements IndexProviders { - private Map indexProviders = new HashMap<>(); + private final Map indexProviders = new HashMap<>(); public Map getIndexProviders() {