From 936762e51abbb9b9eae4a3ade5bbbb2425d59d19 Mon Sep 17 00:00:00 2001 From: Mattias Persson Date: Fri, 5 Feb 2016 12:34:30 +0100 Subject: [PATCH] Hooks in call to RecordFormat.prepare as to properly make the decision about the secondary record unit before writing commands to the log --- .../impl/store/ComposableRecordStore.java | 6 + .../kernel/impl/store/MetaDataStore.java | 5 + .../neo4j/kernel/impl/store/RecordStore.java | 8 + .../impl/store/RelationshipGroupStore.java | 6 + .../state/TransactionRecordState.java | 31 ++- .../impl/store/CommonAbstractStoreTest.java | 5 + .../state/PrepareTrackingRecordFormats.java | 191 ++++++++++++++++++ .../state/TransactionRecordStateTest.java | 45 ++++- .../java/org/neo4j/test/NeoStoresRule.java | 14 +- 9 files changed, 302 insertions(+), 9 deletions(-) create mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/PrepareTrackingRecordFormats.java diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/ComposableRecordStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/ComposableRecordStore.java index 0675362ae95df..c7e154516585c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/ComposableRecordStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/ComposableRecordStore.java @@ -81,6 +81,12 @@ protected void writeRecord( PageCursor cursor, RECORD record ) throws IOExceptio recordFormat.write( record, cursor, recordSize, storeFile ); } + @Override + public void prepareForCommit( RECORD record ) + { + recordFormat.prepare( record, recordSize, this ); + } + @Override public long getNextRecordReference( RECORD record ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/MetaDataStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/MetaDataStore.java index 4c7c71cedf353..64244f515f677 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/MetaDataStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/MetaDataStore.java @@ -846,4 +846,9 @@ protected boolean isInUse( PageCursor cursor ) { return cursor.getByte() == Record.IN_USE.byteValue(); } + + @Override + public void prepareForCommit( NeoStoreActualRecord record ) + { // No need to do anything with these records before commit + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordStore.java index 410445a9c73a3..d4cf7c81657ef 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordStore.java @@ -236,6 +236,8 @@ public interface RecordStore extends IdSequen */ int getStoreHeaderInt(); + void prepareForCommit( RECORD record ); + Predicate IN_USE = AbstractBaseRecord::inUse; class Delegator implements RecordStore @@ -372,6 +374,12 @@ public void ensureHeavy( R record ) { actual.ensureHeavy( record ); } + + @Override + public void prepareForCommit( R record ) + { + actual.prepareForCommit( record ); + } } @SuppressWarnings( "unchecked" ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RelationshipGroupStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RelationshipGroupStore.java index 0deb2c9489a20..24f5759607c21 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RelationshipGroupStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RelationshipGroupStore.java @@ -54,4 +54,10 @@ public void accept( Processor processor, Re { processor.processRelationshipGroup( this, record ); } + + @Override + public void updateRecord( RelationshipGroupRecord record ) + { + super.updateRecord( record ); + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordState.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordState.java index 5ba76816bd315..5d70be934a242 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordState.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordState.java @@ -30,7 +30,11 @@ import org.neo4j.kernel.impl.store.MetaDataStore; import org.neo4j.kernel.impl.store.NeoStores; import org.neo4j.kernel.impl.store.NodeStore; +import org.neo4j.kernel.impl.store.PropertyStore; +import org.neo4j.kernel.impl.store.RecordStore; +import org.neo4j.kernel.impl.store.RelationshipStore; import org.neo4j.kernel.impl.store.SchemaStore; +import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; import org.neo4j.kernel.impl.store.record.DynamicRecord; import org.neo4j.kernel.impl.store.record.IndexRule; import org.neo4j.kernel.impl.store.record.LabelTokenRecord; @@ -82,6 +86,9 @@ public class TransactionRecordState implements RecordState private final NeoStores neoStores; private final IntegrityValidator integrityValidator; private final NodeStore nodeStore; + private final RelationshipStore relationshipStore; + private final PropertyStore propertyStore; + private final RecordStore relationshipGroupStore; private final MetaDataStore metaDataStore; private final SchemaStore schemaStore; private final RecordAccessSet recordChangeSet; @@ -95,6 +102,7 @@ public class TransactionRecordState implements RecordState private RecordChanges neoStoreRecord; private boolean prepared; + public TransactionRecordState( NeoStores neoStores, IntegrityValidator integrityValidator, RecordChangeSet recordChangeSet, long lastCommittedTxWhenTransactionStarted, ResourceLocker locks, @@ -105,6 +113,9 @@ public TransactionRecordState( NeoStores neoStores, IntegrityValidator integrity { this.neoStores = neoStores; this.nodeStore = neoStores.getNodeStore(); + this.relationshipStore = neoStores.getRelationshipStore(); + this.propertyStore = neoStores.getPropertyStore(); + this.relationshipGroupStore = neoStores.getRelationshipGroupStore(); this.metaDataStore = neoStores.getMetaDataStore(); this.schemaStore = neoStores.getSchemaStore(); this.integrityValidator = integrityValidator; @@ -157,7 +168,7 @@ public void extractCommands( Collection commands ) throws Transa int i = 0; for ( RecordProxy change : recordChangeSet.getNodeRecords().changes() ) { - NodeRecord record = change.forReadingLinkage(); + NodeRecord record = prepared( change, nodeStore ); integrityValidator.validateNodeRecord( record ); nodeCommands[i++] = new Command.NodeCommand( change.getBefore(), record ); } @@ -171,7 +182,8 @@ public void extractCommands( Collection commands ) throws Transa int i = 0; for ( RecordProxy change : recordChangeSet.getRelRecords().changes() ) { - relCommands[i++] = new Command.RelationshipCommand( change.getBefore(), change.forReadingLinkage() ); + relCommands[i++] = new Command.RelationshipCommand( change.getBefore(), + prepared( change, relationshipStore ) ); } Arrays.sort( relCommands, COMMAND_SORTER ); } @@ -184,7 +196,8 @@ public void extractCommands( Collection commands ) throws Transa for ( RecordProxy change : recordChangeSet.getPropertyRecords().changes() ) { - propCommands[i++] = new Command.PropertyCommand( change.getBefore(), change.forReadingLinkage() ); + propCommands[i++] = new Command.PropertyCommand( change.getBefore(), + prepared( change, propertyStore ) ); } Arrays.sort( propCommands, COMMAND_SORTER ); } @@ -197,8 +210,8 @@ public void extractCommands( Collection commands ) throws Transa for ( RecordProxy change : recordChangeSet.getRelGroupRecords().changes() ) { - relGroupCommands[i++] = - new Command.RelationshipGroupCommand( change.getBefore(), change.forReadingData() ); + relGroupCommands[i++] = new Command.RelationshipGroupCommand( change.getBefore(), + prepared( change, relationshipGroupStore ) ); } Arrays.sort( relGroupCommands, COMMAND_SORTER ); } @@ -226,6 +239,14 @@ public void extractCommands( Collection commands ) throws Transa prepared = true; } + private RECORD prepared( + RecordProxy proxy, RecordStore store ) + { + RECORD after = proxy.forReadingLinkage(); + store.prepareForCommit( after ); + return after; + } + public void relCreate( long id, int typeId, long startNodeId, long endNodeId ) { relationshipCreator.relationshipCreate( id, typeId, startNodeId, endNodeId, recordChangeSet, locks ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java index 7a01a9d8d2044..4357b36c14eda 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java @@ -132,5 +132,10 @@ protected boolean isInUse( PageCursor cursor ) { return false; } + + @Override + public void prepareForCommit( AbstractBaseRecord record ) + { + } } } 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 new file mode 100644 index 0000000000000..14b4a3da32eb9 --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/PrepareTrackingRecordFormats.java @@ -0,0 +1,191 @@ +/* + * Copyright (c) 2002-2016 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.kernel.impl.transaction.state; + +import java.io.IOException; +import java.util.HashSet; +import java.util.Set; +import org.neo4j.io.pagecache.PageCursor; +import org.neo4j.io.pagecache.PagedFile; +import org.neo4j.kernel.impl.store.StoreHeader; +import org.neo4j.kernel.impl.store.format.RecordFormat; +import org.neo4j.kernel.impl.store.format.RecordFormats; +import org.neo4j.kernel.impl.store.id.IdSequence; +import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; +import org.neo4j.kernel.impl.store.record.DynamicRecord; +import org.neo4j.kernel.impl.store.record.LabelTokenRecord; +import org.neo4j.kernel.impl.store.record.NodeRecord; +import org.neo4j.kernel.impl.store.record.PropertyKeyTokenRecord; +import org.neo4j.kernel.impl.store.record.PropertyRecord; +import org.neo4j.kernel.impl.store.record.RecordLoad; +import org.neo4j.kernel.impl.store.record.RelationshipGroupRecord; +import org.neo4j.kernel.impl.store.record.RelationshipRecord; +import org.neo4j.kernel.impl.store.record.RelationshipTypeTokenRecord; + +public class PrepareTrackingRecordFormats implements RecordFormats +{ + private final RecordFormats actual; + private final Set nodePrepare = new HashSet<>(); + private final Set relationshipPrepare = new HashSet<>(); + private final Set relationshipGroupPrepare = new HashSet<>(); + private final Set propertyPrepare = new HashSet<>(); + private final Set dynamicPrepare = new HashSet<>(); + private final Set propertyKeyTokenPrepare = new HashSet<>(); + private final Set labelTokenPrepare = new HashSet<>(); + private final Set relationshipTypeTokenPrepare = new HashSet<>(); + + public PrepareTrackingRecordFormats( RecordFormats actual ) + { + this.actual = actual; + } + + @Override + public String storeVersion() + { + return actual.storeVersion(); + } + + @Override + public PrepareTrackingRecordFormat node() + { + return new PrepareTrackingRecordFormat<>( actual.node(), nodePrepare ); + } + + @Override + public PrepareTrackingRecordFormat relationshipGroup() + { + return new PrepareTrackingRecordFormat<>( actual.relationshipGroup(), relationshipGroupPrepare ); + } + + @Override + public PrepareTrackingRecordFormat relationship() + { + return new PrepareTrackingRecordFormat<>( actual.relationship(), relationshipPrepare ); + } + + @Override + public PrepareTrackingRecordFormat property() + { + return new PrepareTrackingRecordFormat<>( actual.property(), propertyPrepare ); + } + + @Override + public PrepareTrackingRecordFormat labelToken() + { + return new PrepareTrackingRecordFormat<>( actual.labelToken(), labelTokenPrepare ); + } + + @Override + public PrepareTrackingRecordFormat propertyKeyToken() + { + return new PrepareTrackingRecordFormat<>( actual.propertyKeyToken(), propertyKeyTokenPrepare ); + } + + @Override + public PrepareTrackingRecordFormat relationshipTypeToken() + { + return new PrepareTrackingRecordFormat<>( actual.relationshipTypeToken(), relationshipTypeTokenPrepare ); + } + + @Override + public PrepareTrackingRecordFormat dynamic() + { + return new PrepareTrackingRecordFormat<>( actual.dynamic(), dynamicPrepare ); + } + + public class PrepareTrackingRecordFormat implements RecordFormat + { + private final RecordFormat actual; + private final Set prepare; + + PrepareTrackingRecordFormat( RecordFormat actual, Set prepare ) + { + this.actual = actual; + this.prepare = prepare; + } + + @Override + public RECORD newRecord() + { + return actual.newRecord(); + } + + @Override + public int getRecordSize( StoreHeader storeHeader ) + { + return actual.getRecordSize( storeHeader ); + } + + @Override + public int getRecordHeaderSize() + { + return actual.getRecordHeaderSize(); + } + + @Override + public boolean isInUse( PageCursor cursor ) + { + return actual.isInUse( cursor ); + } + + @Override + public void read( RECORD record, PageCursor cursor, RecordLoad mode, int recordSize, PagedFile storeFile ) + throws IOException + { + actual.read( record, cursor, mode, recordSize, storeFile ); + } + + @Override + public void prepare( RECORD record, int recordSize, IdSequence idSequence ) + { + prepare.add( record ); + actual.prepare( record, recordSize, idSequence ); + } + + @Override + public void write( RECORD record, PageCursor cursor, int recordSize, PagedFile storeFile ) throws IOException + { + actual.write( record, cursor, recordSize, storeFile ); + } + + @Override + public long getNextRecordReference( RECORD record ) + { + return actual.getNextRecordReference( record ); + } + + @Override + public boolean equals( Object otherFormat ) + { + return actual.equals( otherFormat ); + } + + @Override + public int hashCode() + { + return actual.hashCode(); + } + + public boolean prepared( RECORD record ) + { + return prepare.contains( record ); + } + } +} diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordStateTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordStateTest.java index 476b684c55773..26620c16cb6da 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordStateTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/TransactionRecordStateTest.java @@ -53,6 +53,7 @@ import org.neo4j.kernel.impl.store.NeoStores; import org.neo4j.kernel.impl.store.NodeStore; import org.neo4j.kernel.impl.store.RecordStore; +import org.neo4j.kernel.impl.store.format.InternalRecordFormatSelector; import org.neo4j.kernel.impl.store.record.DynamicRecord; import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.kernel.impl.store.record.PropertyBlock; @@ -66,6 +67,7 @@ import org.neo4j.kernel.impl.transaction.command.Command.NodeCommand; import org.neo4j.kernel.impl.transaction.command.Command.PropertyCommand; import org.neo4j.kernel.impl.transaction.command.Command.RelationshipCommand; +import org.neo4j.kernel.impl.transaction.command.Command.RelationshipGroupCommand; import org.neo4j.kernel.impl.transaction.command.CommandHandlerContract; import org.neo4j.kernel.impl.transaction.command.NeoStoreBatchTransactionApplier; import org.neo4j.kernel.impl.transaction.log.FlushableChannel; @@ -93,7 +95,6 @@ import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; - import static org.neo4j.collection.primitive.PrimitiveLongCollections.EMPTY_LONG_ARRAY; import static org.neo4j.graphdb.Direction.INCOMING; import static org.neo4j.graphdb.Direction.OUTGOING; @@ -1081,6 +1082,48 @@ public void shouldSortRelationshipGroups() throws Throwable } } + @Test + public void shouldPrepareRelevantRecords() throws Exception + { + // GIVEN + PrepareTrackingRecordFormats format = new PrepareTrackingRecordFormats( InternalRecordFormatSelector.select() ); + NeoStores neoStores = neoStoresRule.open( format, + GraphDatabaseSettings.dense_node_threshold.name(), "1" ); + + // WHEN + TransactionRecordState state = newTransactionRecordState( neoStores ); + state.nodeCreate( 0 ); + state.relCreate( 0, 0, 0, 0 ); + state.relCreate( 1, 0, 0, 0 ); + state.relCreate( 2, 0, 0, 0 ); + List commands = new ArrayList<>(); + state.extractCommands( commands ); + + // THEN + int nodes = 0, rels = 0, groups = 0; + for ( StorageCommand command : commands ) + { + if ( command instanceof NodeCommand ) + { + assertTrue( format.node().prepared( ((NodeCommand) command).getAfter() ) ); + nodes++; + } + else if ( command instanceof RelationshipCommand ) + { + assertTrue( format.relationship().prepared( ((RelationshipCommand) command).getAfter() ) ); + rels++; + } + else if ( command instanceof RelationshipGroupCommand ) + { + assertTrue( format.relationshipGroup().prepared( ((RelationshipGroupCommand) command).getAfter() ) ); + groups++; + } + } + assertEquals( 1, nodes ); + assertEquals( 3, rels ); + assertEquals( 1, groups ); + } + private long[] createRelationships( NeoStores neoStores, TransactionRecordState tx, long nodeId, int type, Direction direction, int count ) { diff --git a/community/kernel/src/test/java/org/neo4j/test/NeoStoresRule.java b/community/kernel/src/test/java/org/neo4j/test/NeoStoresRule.java index 37baedcea69b2..476f5a3e2ad1a 100644 --- a/community/kernel/src/test/java/org/neo4j/test/NeoStoresRule.java +++ b/community/kernel/src/test/java/org/neo4j/test/NeoStoresRule.java @@ -29,6 +29,8 @@ import org.neo4j.kernel.impl.pagecache.ConfiguringPageCacheFactory; import org.neo4j.kernel.impl.store.NeoStores; import org.neo4j.kernel.impl.store.StoreFactory; +import org.neo4j.kernel.impl.store.format.InternalRecordFormatSelector; +import org.neo4j.kernel.impl.store.format.RecordFormats; import org.neo4j.logging.NullLog; import org.neo4j.logging.NullLogProvider; @@ -48,6 +50,7 @@ public class NeoStoresRule extends ExternalResource private EphemeralFileSystemAbstraction efs; private PageCache pageCache; private StoreFactory storeFactory; + private final RecordFormats format = InternalRecordFormatSelector.select(); public NeoStoresRule( Class testClass ) { @@ -55,23 +58,28 @@ public NeoStoresRule( Class testClass ) } public NeoStores open( String... config ) + { + return open( InternalRecordFormatSelector.select(), config ); + } + + public NeoStores open( RecordFormats format, String... config ) { efs = new EphemeralFileSystemAbstraction(); Config conf = new Config( stringMap( config ) ); ConfiguringPageCacheFactory pageCacheFactory = new ConfiguringPageCacheFactory( efs, conf, NULL, NullLog.getInstance() ); pageCache = pageCacheFactory.getOrCreatePageCache(); - return open( efs, pageCache, config ); + return open( efs, pageCache, format, config ); } - public NeoStores open( FileSystemAbstraction fs, PageCache pageCache, String... config ) + public NeoStores open( FileSystemAbstraction fs, PageCache pageCache, RecordFormats format, String... config ) { assert neoStores == null : "Already opened"; TargetDirectory targetDirectory = new TargetDirectory( fs, testClass ); File storeDir = targetDirectory.makeGraphDbDir(); Config configuration = new Config( stringMap( config ) ); storeFactory = new StoreFactory( storeDir, configuration, new DefaultIdGeneratorFactory( fs ), - pageCache, fs, NullLogProvider.getInstance() ); + pageCache, fs, NullLogProvider.getInstance(), format ); return neoStores = storeFactory.openAllNeoStores( true ); }