Skip to content

Commit

Permalink
Merge pull request #11441 from tinwelint/3.4-cheap-migration
Browse files Browse the repository at this point in the history
Avoid unnecessary 3.3 -> 3.4 migration
  • Loading branch information
tinwelint committed Apr 4, 2018
2 parents 94c0161 + 5c50077 commit bb01881
Show file tree
Hide file tree
Showing 14 changed files with 240 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ long record = getRecord( pageCache, neoStoreFileName, STORE_VERSION );
private boolean isCompatibleFormats( RecordFormats storeFormat )
{
return FormatFamily.isSameFamily( recordFormats, storeFormat ) &&
recordFormats.hasSameCapabilities( storeFormat, CapabilityType.FORMAT ) &&
recordFormats.hasCompatibleCapabilities( storeFormat, CapabilityType.FORMAT ) &&
recordFormats.generation() >= storeFormat.generation();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,19 +123,33 @@ public boolean hasCapability( Capability capability )
return contains( capabilities(), capability );
}

public static boolean hasSameCapabilities( RecordFormats one, RecordFormats other, CapabilityType type )
public static boolean hasCompatibleCapabilities( RecordFormats one, RecordFormats other, CapabilityType type )
{
Set<Capability> myFormatCapabilities = Stream.of( one.capabilities() )
.filter( capability -> capability.isType( type ) ).collect( toSet() );
Set<Capability> otherFormatCapabilities = Stream.of( other.capabilities() )
.filter( capability -> capability.isType( type ) ).collect( toSet() );

return myFormatCapabilities.equals( otherFormatCapabilities );
if ( myFormatCapabilities.equals( otherFormatCapabilities ) )
{
// If they have the same capabilities then of course they are compatible
return true;
}

boolean capabilitiesNotRemoved = otherFormatCapabilities.containsAll( myFormatCapabilities );

otherFormatCapabilities.removeAll( myFormatCapabilities );
boolean allAddedAreAdditive = otherFormatCapabilities.stream().allMatch( Capability::isAdditive );

// Even if capabilities of the two aren't the same then there's a special case where if the additional
// capabilities of the other format are all additive then they are also compatible because no data
// in the existing store needs to be migrated.
return capabilitiesNotRemoved && allAddedAreAdditive;
}

@Override
public boolean hasSameCapabilities( RecordFormats other, CapabilityType type )
public boolean hasCompatibleCapabilities( RecordFormats other, CapabilityType type )
{
return hasSameCapabilities( this, other, type );
return hasCompatibleCapabilities( this, other, type );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,27 +55,45 @@ public enum Capability
/**
* Point Geometries are an addition to the format, not a change
*/
POINT_PROPERTIES( CapabilityType.STORE ),
POINT_PROPERTIES( true, CapabilityType.STORE ),

/**
* Temporal types are an addition to the format, not a change
*/
TEMPORAL_PROPERTIES( CapabilityType.STORE ),
TEMPORAL_PROPERTIES( true, CapabilityType.STORE ),

/**
* Records can spill over into secondary units (another record with a header saying it's a secondary unit to another record).
*/
SECONDARY_RECORD_UNITS( CapabilityType.FORMAT );

private final CapabilityType[] types;
private boolean additive;

Capability( CapabilityType... types )
{
this( false, types );
}

Capability( boolean additive, CapabilityType... types )
{
this.additive = additive;
this.types = types;
}

public boolean isType( CapabilityType type )
{
return contains( types, type );
}

/**
* Whether or not this capability is additive. A capability is additive if data regarding this capability will not change
* any existing store and therefore not require migration of existing data.
*
* @return whether or not this capability is additive.
*/
public boolean isAdditive()
{
return additive;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,13 @@ public Factory( String key, String... altKeys )
FormatFamily getFormatFamily();

/**
* Whether or not this format has the same capabilities of the specific {@code type} as the {@code other} format.
* Whether or not changes in the {@code other} format, compared to this format, for the given {@code type}, are compatible.
*
* @param other {@link RecordFormats} to compare with.
* @param type {@link CapabilityType type} of capability to compare.
* @return true if both formats have the same set of capabilities of the given {@code type}.
* @param other {@link RecordFormats} to check compatibility with.
* @param type {@link CapabilityType type} of capability to check compatibility for.
* @return true if the {@code other} format have compatible capabilities of the given {@code type}.
*/
boolean hasSameCapabilities( RecordFormats other, CapabilityType type );
boolean hasCompatibleCapabilities( RecordFormats other, CapabilityType type );

/**
* Record format name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,13 @@
*/
public class CountsMigrator extends AbstractStoreMigrationParticipant
{

private static final Iterable<StoreFile> COUNTS_STORE_FILES = Iterables
.iterable( StoreFile.COUNTS_STORE_LEFT, StoreFile.COUNTS_STORE_RIGHT );

private final Config config;
private final FileSystemAbstraction fileSystem;
private final PageCache pageCache;
private boolean migrated;

public CountsMigrator( FileSystemAbstraction fileSystem, PageCache pageCache, Config config )
{
Expand Down Expand Up @@ -110,22 +111,27 @@ public void migrate( File storeDir, File migrationDir, ProgressReporter progress
rebuildCountsFromScratch( storeDir, migrationDir, lastTxId, progressMonitor, versionToMigrateFrom,
pageCache, NullLogProvider.getInstance() );
}
migrated = true;
}
}

@Override
public void moveMigratedFiles( File migrationDir, File storeDir, String versionToUpgradeFrom,
String versionToUpgradeTo ) throws IOException
{
// Delete any current count files in the store directory.
StoreFile.fileOperation( DELETE, fileSystem, storeDir, null, COUNTS_STORE_FILES, true, null,
StoreFileType.values() );
// Move the migrated ones into the store directory
StoreFile.fileOperation( MOVE, fileSystem, migrationDir, storeDir, COUNTS_STORE_FILES, true,
// allow to skip non existent source files
ExistingTargetStrategy.OVERWRITE, // allow to overwrite target files
StoreFileType.values() );
// We do not need to move files with the page cache, as the count files always reside on the normal file system.

if ( migrated )
{
// Delete any current count files in the store directory.
StoreFile.fileOperation( DELETE, fileSystem, storeDir, null, COUNTS_STORE_FILES, true, null,
StoreFileType.values() );
// Move the migrated ones into the store directory
StoreFile.fileOperation( MOVE, fileSystem, migrationDir, storeDir, COUNTS_STORE_FILES, true,
// allow to skip non existent source files
ExistingTargetStrategy.OVERWRITE, // allow to overwrite target files
StoreFileType.values() );
// We do not need to move files with the page cache, as the count files always reside on the normal file system.
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void migrate( File storeDir, File migrationDir, ProgressReporter progress
{
RecordFormats from = RecordFormatSelector.selectForVersion( versionToMigrateFrom );
RecordFormats to = RecordFormatSelector.selectForVersion( versionToMigrateTo );
if ( !from.hasSameCapabilities( to, CapabilityType.INDEX ) )
if ( !from.hasCompatibleCapabilities( to, CapabilityType.INDEX ) )
{
originalExplicitIndexesRoot = indexImplementation.getIndexImplementationDirectory( storeDir );
migrationExplicitIndexesRoot = indexImplementation.getIndexImplementationDirectory( migrationDir );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void migrate( File storeDir, File migrationDir, ProgressReporter progress
{
RecordFormats from = RecordFormatSelector.selectForVersion( versionToMigrateFrom );
RecordFormats to = RecordFormatSelector.selectForVersion( versionToMigrateTo );
if ( !from.hasSameCapabilities( to, CapabilityType.INDEX ) )
if ( !from.hasCompatibleCapabilities( to, CapabilityType.INDEX ) )
{
schemaIndexDirectory = indexProvider.directoryStructure().rootDirectory();
if ( schemaIndexDirectory != null )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public void migrate( File storeDir, File migrationDir, ProgressReporter progress

private boolean isDifferentCapabilities( RecordFormats oldFormat, RecordFormats newFormat )
{
return !oldFormat.hasSameCapabilities( newFormat, CapabilityType.FORMAT );
return !oldFormat.hasCompatibleCapabilities( newFormat, CapabilityType.FORMAT );
}

void writeLastTxInformation( File migrationDir, TransactionId txInfo ) throws IOException
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright (c) 2002-2018 "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 <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.store.format;

import org.junit.Rule;
import org.junit.Test;

import org.neo4j.test.rule.RandomRule;

import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.neo4j.helpers.collection.Iterators.array;

public class BaseRecordFormatsTest
{
private static final Capability[] CAPABILITIES = Capability.values();
private static final CapabilityType[] CAPABILITY_TYPES = CapabilityType.values();

@Rule
public final RandomRule random = new RandomRule();

@Test
public void shouldReportCompatibilityBetweenTwoEqualSetsOfCapabilities()
{
// given
Capability[] capabilities = random.selection( CAPABILITIES, CAPABILITIES.length / 2, CAPABILITIES.length, false );

// then
assertCompatibility( capabilities, capabilities, true, CAPABILITY_TYPES );
}

@Test
public void shouldReportCompatibilityForAdditiveAdditionalCapabilities()
{
// given
Capability[] from = array( Capability.SCHEMA );
Capability[] to = array( Capability.SCHEMA, Capability.POINT_PROPERTIES, Capability.TEMPORAL_PROPERTIES );

// then
assertCompatibility( from, to, true, CAPABILITY_TYPES );
}

@Test
public void shouldReportIncompatibilityForChangingAdditionalCapabilities()
{
// given
Capability[] from = array( Capability.SCHEMA );
Capability[] to = array( Capability.SCHEMA, Capability.DENSE_NODES );

// then
assertCompatibility( from, to, false, CapabilityType.STORE );
}

@Test
public void shouldReportIncompatibilityForAdditiveRemovedCapabilities()
{
// given
Capability[] from = array( Capability.SCHEMA, Capability.POINT_PROPERTIES, Capability.TEMPORAL_PROPERTIES );
Capability[] to = array( Capability.SCHEMA );

// then
assertCompatibility( from, to, false, CapabilityType.STORE );
}

private void assertCompatibility( Capability[] from, Capability[] to, boolean compatible, CapabilityType... capabilityTypes )
{
for ( CapabilityType type : capabilityTypes )
{
assertEquals( compatible, format( from ).hasCompatibleCapabilities( format( to ), type ) );
}
}

private RecordFormats format( Capability... capabilities )
{
RecordFormats formats = mock( BaseRecordFormats.class );
when( formats.capabilities() ).thenReturn( capabilities );
when( formats.hasCompatibleCapabilities( any( RecordFormats.class ), any( CapabilityType.class ) ) ).thenCallRealMethod();
return formats;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,9 @@ public FormatFamily getFormatFamily()
}

@Override
public boolean hasSameCapabilities( RecordFormats other, CapabilityType type )
public boolean hasCompatibleCapabilities( RecordFormats other, CapabilityType type )
{
return BaseRecordFormats.hasSameCapabilities( this, other, type );
return BaseRecordFormats.hasCompatibleCapabilities( this, other, type );
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ public FormatFamily getFormatFamily()
}

@Override
public boolean hasSameCapabilities( RecordFormats other, CapabilityType type )
public boolean hasCompatibleCapabilities( RecordFormats other, CapabilityType type )
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright (c) 2002-2018 "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 <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.storemigration.participant;

import org.junit.Rule;
import org.junit.Test;

import java.io.File;
import java.io.IOException;

import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.impl.store.format.StoreVersion;
import org.neo4j.kernel.impl.util.monitoring.SilentProgressReporter;
import org.neo4j.test.rule.TestDirectory;
import org.neo4j.test.rule.fs.EphemeralFileSystemRule;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.neo4j.kernel.impl.store.StoreFile.COUNTS_STORE_LEFT;
import static org.neo4j.kernel.impl.store.StoreFile.COUNTS_STORE_RIGHT;
import static org.neo4j.kernel.impl.storemigration.StoreFileType.STORE;

public class CountsMigratorTest
{
@Rule
public final EphemeralFileSystemRule fs = new EphemeralFileSystemRule();
@Rule
public final TestDirectory directory = TestDirectory.testDirectory( fs );

@Test
public void shouldNotAccidentallyDeleteStoreFilesIfNoMigrationWasRequired() throws IOException
{
// given
CountsMigrator migrator = new CountsMigrator( fs, null, Config.defaults() );
File storeDir = directory.graphDbDir();
File countsStoreFileA = new File( storeDir, COUNTS_STORE_LEFT.fileName( STORE ) );
File countsStoreFileB = new File( storeDir, COUNTS_STORE_RIGHT.fileName( STORE ) );
fs.create( countsStoreFileA );
fs.create( countsStoreFileB );
File migrationDir = new File( storeDir, "migration" );
fs.mkdirs( migrationDir );
String versionToMigrateFrom = StoreVersion.STANDARD_V3_2.versionString();
String versionToMigrateTo = StoreVersion.STANDARD_V3_4.versionString();
migrator.migrate( storeDir, migrationDir, SilentProgressReporter.INSTANCE, versionToMigrateFrom, versionToMigrateTo );
assertEquals( "Invalid test assumption: There should not have been migration for those versions", 0,
fs.listFiles( migrationDir ).length );

// when
migrator.moveMigratedFiles( migrationDir, storeDir, versionToMigrateFrom, versionToMigrateTo );

// then
assertTrue( fs.fileExists( countsStoreFileA ) );
assertTrue( fs.fileExists( countsStoreFileB ) );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ public FormatFamily getFormatFamily()
}

@Override
public boolean hasSameCapabilities( RecordFormats other, CapabilityType type )
public boolean hasCompatibleCapabilities( RecordFormats other, CapabilityType type )
{
return actual.hasSameCapabilities( other, type );
return actual.hasCompatibleCapabilities( other, type );
}

@Override
Expand Down

0 comments on commit bb01881

Please sign in to comment.