From c20c778abd6d27d380b4884cfd5dee280e12266d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mattias=20Finn=C3=A9?= Date: Sun, 1 Apr 2018 10:26:48 +0200 Subject: [PATCH] Introduces 'additive' property of store capability So that it can be checked when comparing capabilities of two formats Purely additive capability additions can be seen as compatible too. --- .../neo4j/kernel/impl/store/NeoStores.java | 2 +- .../impl/store/format/BaseRecordFormats.java | 27 ++++- .../kernel/impl/store/format/Capability.java | 31 ++++-- .../impl/store/format/RecordFormats.java | 2 +- .../participant/ExplicitIndexMigrator.java | 2 +- .../participant/SchemaIndexMigrator.java | 2 +- .../participant/StoreMigrator.java | 2 +- .../store/format/BaseRecordFormatsTest.java | 99 +++++++++++++++++++ .../ForcedSecondaryUnitRecordFormats.java | 4 +- .../RecordFormatPropertyConfiguratorTest.java | 2 +- .../state/PrepareTrackingRecordFormats.java | 4 +- .../HighLimitStoreMigrationTest.java | 2 +- 12 files changed, 155 insertions(+), 24 deletions(-) create mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/BaseRecordFormatsTest.java diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/NeoStores.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/NeoStores.java index 37a107faf848..8575f86415fb 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/NeoStores.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/NeoStores.java @@ -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(); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseRecordFormats.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseRecordFormats.java index 513abe326744..89f41a92ff11 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseRecordFormats.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseRecordFormats.java @@ -19,6 +19,7 @@ */ package org.neo4j.kernel.impl.store.format; +import java.util.HashSet; import java.util.Set; import java.util.stream.Stream; @@ -123,19 +124,37 @@ 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 myFormatCapabilities = Stream.of( one.capabilities() ) .filter( capability -> capability.isType( type ) ).collect( toSet() ); Set 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; + } + + Set removedCapabilities = + new HashSet<>( myFormatCapabilities ).stream().filter( capability -> !otherFormatCapabilities.contains( capability ) ).collect( toSet() ); + Set addedCapabilities = + new HashSet<>( otherFormatCapabilities ).stream().filter( capability -> !myFormatCapabilities.contains( capability ) ).collect( toSet() ); + boolean allAddedAreAdditive = addedCapabilities.stream().allMatch( Capability::isAdditive ); + if ( removedCapabilities.isEmpty() && allAddedAreAdditive ) + { + // 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 true; + } + return false; } @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 ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/Capability.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/Capability.java index 298554c9a431..b9a3038545d0 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/Capability.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/Capability.java @@ -30,47 +30,49 @@ public enum Capability /** * Store has schema support */ - SCHEMA( CapabilityType.STORE ), + SCHEMA( false, CapabilityType.STORE ), /** * Store has dense node support */ - DENSE_NODES( CapabilityType.FORMAT, CapabilityType.STORE ), + DENSE_NODES( false, CapabilityType.FORMAT, CapabilityType.STORE ), /** * 3 bytes relationship type support */ - RELATIONSHIP_TYPE_3BYTES( CapabilityType.FORMAT, CapabilityType.STORE ), + RELATIONSHIP_TYPE_3BYTES( false, CapabilityType.FORMAT, CapabilityType.STORE ), /** * Lucene version 3.x */ - LUCENE_3( CapabilityType.INDEX ), + LUCENE_3( false, CapabilityType.INDEX ), /** * Lucene version 5.x */ - LUCENE_5( CapabilityType.INDEX ), + LUCENE_5( false, CapabilityType.INDEX ), /** * 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 ); + SECONDARY_RECORD_UNITS( false, CapabilityType.FORMAT ); private final CapabilityType[] types; + private boolean additive; - Capability( CapabilityType... types ) + Capability( boolean additive, CapabilityType... types ) { + this.additive = additive; this.types = types; } @@ -78,4 +80,15 @@ 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; + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/RecordFormats.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/RecordFormats.java index 9a18153023ae..1556af8ff650 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/RecordFormats.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/RecordFormats.java @@ -115,7 +115,7 @@ public Factory( String key, String... altKeys ) * @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}. */ - boolean hasSameCapabilities( RecordFormats other, CapabilityType type ); + boolean hasCompatibleCapabilities( RecordFormats other, CapabilityType type ); /** * Record format name diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/ExplicitIndexMigrator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/ExplicitIndexMigrator.java index 928d31b34aed..ecf9f87ca549 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/ExplicitIndexMigrator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/ExplicitIndexMigrator.java @@ -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 ); 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 4a2823042e4c..6aa2de24a1c6 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 @@ -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 ) 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 5021996d6f75..e42751aa046f 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 @@ -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 diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/BaseRecordFormatsTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/BaseRecordFormatsTest.java new file mode 100644 index 000000000000..61c5ffd30106 --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/BaseRecordFormatsTest.java @@ -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 . + */ +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; + } +} diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/ForcedSecondaryUnitRecordFormats.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/ForcedSecondaryUnitRecordFormats.java index 590c61aef3b1..c3f3bf0d4358 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/ForcedSecondaryUnitRecordFormats.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/ForcedSecondaryUnitRecordFormats.java @@ -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 diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/RecordFormatPropertyConfiguratorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/RecordFormatPropertyConfiguratorTest.java index ff077e5ae39f..bf3728746225 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/RecordFormatPropertyConfiguratorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/RecordFormatPropertyConfiguratorTest.java @@ -187,7 +187,7 @@ public FormatFamily getFormatFamily() } @Override - public boolean hasSameCapabilities( RecordFormats other, CapabilityType type ) + public boolean hasCompatibleCapabilities( RecordFormats other, CapabilityType type ) { return false; } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/PrepareTrackingRecordFormats.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/PrepareTrackingRecordFormats.java index cb50f36b1506..ad8f85e54bea 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/PrepareTrackingRecordFormats.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/PrepareTrackingRecordFormats.java @@ -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 diff --git a/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/store/format/highlimit/HighLimitStoreMigrationTest.java b/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/store/format/highlimit/HighLimitStoreMigrationTest.java index 9b54804e39b5..b3eaa98ea814 100644 --- a/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/store/format/highlimit/HighLimitStoreMigrationTest.java +++ b/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/store/format/highlimit/HighLimitStoreMigrationTest.java @@ -59,7 +59,7 @@ public class HighLimitStoreMigrationTest @Test public void haveDifferentFormatCapabilitiesAsHighLimit3_0() { - assertFalse( HighLimit.RECORD_FORMATS.hasSameCapabilities( HighLimitV3_0_0.RECORD_FORMATS, CapabilityType.FORMAT ) ); + assertFalse( HighLimit.RECORD_FORMATS.hasCompatibleCapabilities( HighLimitV3_0_0.RECORD_FORMATS, CapabilityType.FORMAT ) ); } @Test