From c5d3a89d5dbc2c3742c46373575d0e905d8c0537 Mon Sep 17 00:00:00 2001 From: Ragnar Mellbin Date: Fri, 25 May 2018 14:09:53 +0200 Subject: [PATCH] Index updates for relationship indexes wip --- ...or.java => PropertyCommandsExtractor.java} | 61 +++++++++---- .../PropertyPhysicalToLogicalConverter.java | 16 ++-- .../recordstorage/RecordStorageEngine.java | 10 ++- .../impl/store/record/PropertyRecord.java | 5 ++ .../command/IndexBatchTransactionApplier.java | 33 ++++--- .../transaction/command/IndexUpdatesWork.java | 5 +- .../transaction/state/DirectIndexUpdates.java | 4 +- .../impl/transaction/state/IndexUpdates.java | 9 +- .../transaction/state/OnlineIndexUpdates.java | 90 ++++++++++++++++--- .../impl/api/index/IndexingServiceTest.java | 6 +- .../TransactionRecordStateTest.java | 42 ++++++--- .../IndexBatchTransactionApplierTest.java | 6 +- .../NeoStoreTransactionApplierTest.java | 3 +- .../NeoTransactionIndexApplierTest.java | 6 +- .../state/SchemaRuleCommandTest.java | 9 +- 15 files changed, 215 insertions(+), 90 deletions(-) rename community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/{NodePropertyCommandsExtractor.java => PropertyCommandsExtractor.java} (65%) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/NodePropertyCommandsExtractor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/PropertyCommandsExtractor.java similarity index 65% rename from community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/NodePropertyCommandsExtractor.java rename to community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/PropertyCommandsExtractor.java index b4bd45cec7963..3276621fd3175 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/NodePropertyCommandsExtractor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/PropertyCommandsExtractor.java @@ -29,6 +29,7 @@ import org.neo4j.kernel.impl.api.BatchTransactionApplier; import org.neo4j.kernel.impl.api.TransactionApplier; import org.neo4j.kernel.impl.locking.LockGroup; +import org.neo4j.kernel.impl.transaction.command.Command; import org.neo4j.kernel.impl.transaction.command.Command.NodeCommand; import org.neo4j.kernel.impl.transaction.command.Command.PropertyCommand; import org.neo4j.storageengine.api.CommandsToApply; @@ -39,11 +40,13 @@ * Implements both BatchTransactionApplier and TransactionApplier in order to reduce garbage. * Gathers node/property commands by node id, preparing for extraction of {@link EntityUpdates updates}. */ -public class NodePropertyCommandsExtractor extends TransactionApplier.Adapter +public class PropertyCommandsExtractor extends TransactionApplier.Adapter implements BatchTransactionApplier { private final MutableLongObjectMap nodeCommandsById = new LongObjectHashMap<>(); + private final MutableLongObjectMap relationshipCommandsById = new LongObjectHashMap<>(); private final MutableLongObjectMap> propertyCommandsByNodeIds = new LongObjectHashMap<>(); + private final MutableLongObjectMap> propertyCommandsByRelationshipIds = new LongObjectHashMap<>(); private boolean hasUpdates; @Override @@ -62,7 +65,9 @@ public TransactionApplier startTx( CommandsToApply transaction, LockGroup lockGr public void close() { nodeCommandsById.clear(); + relationshipCommandsById.clear(); propertyCommandsByNodeIds.clear(); + propertyCommandsByRelationshipIds.clear(); } @Override @@ -76,7 +81,15 @@ public boolean visitNodeCommand( NodeCommand command ) return false; } - public static boolean mayResultInIndexUpdates( NodeCommand command ) + @Override + public boolean visitRelationshipCommand( Command.RelationshipCommand command ) + { + relationshipCommandsById.put( command.getKey(), command ); + hasUpdates = true; + return false; + } + + private static boolean mayResultInIndexUpdates( NodeCommand command ) { long before = command.getBefore().getLabelField(); long after = command.getAfter().getLabelField(); @@ -84,32 +97,34 @@ public static boolean mayResultInIndexUpdates( NodeCommand command ) // Because we don't know here, there may have been changes to a dynamic label record // even though it still points to the same one fieldPointsToDynamicRecordOfLabels( before ) || fieldPointsToDynamicRecordOfLabels( after ); - - } - - public static boolean mayResultInIndexUpdates( PropertyCommand command ) - { - return command.getAfter().isNodeSet(); } @Override public boolean visitPropertyCommand( PropertyCommand command ) { - if ( mayResultInIndexUpdates( command ) ) + if ( command.getAfter().isNodeSet() ) { - long nodeId = command.getAfter().getNodeId(); - List group = propertyCommandsByNodeIds.get( nodeId ); - if ( group == null ) - { - propertyCommandsByNodeIds.put( nodeId, group = new ArrayList<>() ); - } - group.add( command ); - hasUpdates = true; + createOrAddToGroup( command, command.getAfter().getNodeId(), propertyCommandsByNodeIds ); + } + else if ( command.getAfter().isRelSet() ) + { + createOrAddToGroup( command, command.getAfter().getRelId(), propertyCommandsByRelationshipIds ); } return false; } - public boolean containsAnyNodeOrPropertyUpdate() + private void createOrAddToGroup( PropertyCommand command, long entityId, MutableLongObjectMap> propertyCommandsByEntityIds ) + { + List group = propertyCommandsByEntityIds.get( entityId ); + if ( group == null ) + { + this.propertyCommandsByNodeIds.put( entityId, group = new ArrayList<>() ); + } + group.add( command ); + hasUpdates = true; + } + + public boolean containsAnyEntityOrPropertyUpdate() { return hasUpdates; } @@ -119,8 +134,18 @@ public LongObjectMap nodeCommandsById() return nodeCommandsById; } + public LongObjectMap relationshipCommandsById() + { + return relationshipCommandsById; + } + public LongObjectMap> propertyCommandsByNodeIds() { return propertyCommandsByNodeIds; } + + public LongObjectMap> propertyCommandsByRelationshipIds() + { + return propertyCommandsByRelationshipIds; + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/PropertyPhysicalToLogicalConverter.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/PropertyPhysicalToLogicalConverter.java index 2a2bc96cc2111..e3c1e8ac63fdd 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/PropertyPhysicalToLogicalConverter.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/PropertyPhysicalToLogicalConverter.java @@ -42,14 +42,14 @@ public PropertyPhysicalToLogicalConverter( PropertyStore propertyStore ) } /** - * Converts physical changes to PropertyRecords for a node into logical updates + * Converts physical changes to PropertyRecords for a entity into logical updates */ - public void convertPropertyRecord( long nodeId, Iterable changes, + public void convertPropertyRecord( long entityId, Iterable changes, EntityUpdates.Builder properties ) { MutableIntObjectMap beforeMap = new IntObjectHashMap<>(); MutableIntObjectMap afterMap = new IntObjectHashMap<>(); - mapBlocks( nodeId, changes, beforeMap, afterMap ); + mapBlocks( entityId, changes, beforeMap, afterMap ); final IntIterator uniqueIntIterator = uniqueIntIterator( beforeMap, afterMap ); while ( uniqueIntIterator.hasNext() ) @@ -95,21 +95,21 @@ private IntIterator uniqueIntIterator( IntObjectMap beforeMap, In return keys.intIterator(); } - private void mapBlocks( long nodeId, Iterable changes, + private void mapBlocks( long entityId, Iterable changes, MutableIntObjectMap beforeMap, MutableIntObjectMap afterMap ) { for ( PropertyRecordChange change : changes ) { - equalCheck( change.getBefore().getNodeId(), nodeId ); - equalCheck( change.getAfter().getNodeId(), nodeId ); + equalCheck( change.getBefore().getEntityId(), entityId ); + equalCheck( change.getAfter().getEntityId(), entityId ); mapBlocks( change.getBefore(), beforeMap ); mapBlocks( change.getAfter(), afterMap ); } } - private void equalCheck( long nodeId, long expectedNodeId ) + private void equalCheck( long entityId, long expectedEntityId ) { - assert nodeId == expectedNodeId : "Node id differs expected " + expectedNodeId + ", but was " + nodeId; + assert entityId == expectedEntityId : "Entity id differs expected " + expectedEntityId + ", but was " + entityId; } private void mapBlocks( PropertyRecord record, MutableIntObjectMap blocks ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java index 3bc4c92b48c9d..e57b2a3481001 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java @@ -196,11 +196,15 @@ public RecordStorageEngine( labelScanStore = new NativeLabelScanStore( pageCache, storeDir, fs, new FullLabelStream( neoStoreIndexStoreView ), readOnly, monitors, recoveryCleanupWorkCollector ); + // We need to load the property tokens here, since we need them before we load the indexes. + propertyKeyTokenHolder.setInitialTokens( + neoStores.getPropertyKeyTokenStore().getTokens( Integer.MAX_VALUE ) ); + indexStoreView = new DynamicIndexStoreView( neoStoreIndexStoreView, labelScanStore, lockService, neoStores, logProvider ); this.indexProviderMap = indexProviderMap; - indexingService = IndexingServiceFactory.createIndexingService( config, scheduler, this.indexProviderMap, + indexingService = IndexingServiceFactory.createIndexingService( config, scheduler, indexProviderMap, indexStoreView, tokenNameLookup, - Iterators.asList( new SchemaStorage( neoStores.getSchemaStore() ).indexesGetAll() ), logProvider, + Iterators.asList( schemaStorage.indexesGetAll() ), logProvider, indexingServiceMonitor, schemaState ); integrityValidator = new IntegrityValidator( neoStores, indexingService ); @@ -332,7 +336,7 @@ protected BatchTransactionApplierFacade applier( TransactionApplicationMode mode // Schema index application appliers.add( new IndexBatchTransactionApplier( indexingService, labelScanStoreSync, indexUpdatesSync, - neoStores.getNodeStore(), + neoStores.getNodeStore(), neoStores.getRelationshipStore(), indexUpdatesConverter ) ); // Explicit index application diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/record/PropertyRecord.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/record/PropertyRecord.java index 3a07b5967f449..2c10874ec52b5 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/record/PropertyRecord.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/record/PropertyRecord.java @@ -140,6 +140,11 @@ public long getRelId() return -1; } + public long getEntityId() + { + return entityId; + } + /** * Gets the sum of the sizes of the blocks in this record, in bytes. */ diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplier.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplier.java index 79e60d194f1bc..49fe22d230b56 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplier.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplier.java @@ -30,15 +30,16 @@ import org.neo4j.kernel.api.exceptions.index.IndexPopulationFailedKernelException; import org.neo4j.kernel.api.labelscan.LabelScanWriter; import org.neo4j.kernel.api.labelscan.NodeLabelUpdate; +import org.neo4j.kernel.api.schema.index.StoreIndexDescriptor; import org.neo4j.kernel.impl.api.BatchTransactionApplier; import org.neo4j.kernel.impl.api.TransactionApplier; import org.neo4j.kernel.impl.api.index.IndexingService; import org.neo4j.kernel.impl.api.index.IndexingUpdateService; -import org.neo4j.kernel.impl.api.index.NodePropertyCommandsExtractor; +import org.neo4j.kernel.impl.api.index.PropertyCommandsExtractor; import org.neo4j.kernel.impl.api.index.PropertyPhysicalToLogicalConverter; import org.neo4j.kernel.impl.store.NodeLabels; import org.neo4j.kernel.impl.store.NodeStore; -import org.neo4j.kernel.api.schema.index.StoreIndexDescriptor; +import org.neo4j.kernel.impl.store.RelationshipStore; import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.kernel.impl.transaction.command.Command.PropertyCommand; import org.neo4j.kernel.impl.transaction.state.IndexUpdates; @@ -64,17 +65,15 @@ public class IndexBatchTransactionApplier extends BatchTransactionApplier.Adapte private List labelUpdates; private IndexUpdates indexUpdates; - public IndexBatchTransactionApplier( IndexingService indexingService, - WorkSync,LabelUpdateWork> labelScanStoreSync, - WorkSync indexUpdatesSync, - NodeStore nodeStore, + public IndexBatchTransactionApplier( IndexingService indexingService, WorkSync,LabelUpdateWork> labelScanStoreSync, + WorkSync indexUpdatesSync, NodeStore nodeStore, RelationshipStore relationshipStore, PropertyPhysicalToLogicalConverter indexUpdateConverter ) { this.indexingService = indexingService; this.labelScanStoreSync = labelScanStoreSync; this.indexUpdatesSync = indexUpdatesSync; this.indexUpdateConverter = indexUpdateConverter; - this.transactionApplier = new SingleTransactionApplier( nodeStore ); + this.transactionApplier = new SingleTransactionApplier( nodeStore, relationshipStore ); } @Override @@ -133,23 +132,25 @@ public void close() throws Exception private class SingleTransactionApplier extends TransactionApplier.Adapter { private final NodeStore nodeStore; - private final NodePropertyCommandsExtractor indexUpdatesExtractor = new NodePropertyCommandsExtractor(); + private RelationshipStore relationshipStore; + private final PropertyCommandsExtractor indexUpdatesExtractor = new PropertyCommandsExtractor(); private List createdIndexes; - SingleTransactionApplier( NodeStore nodeStore ) + SingleTransactionApplier( NodeStore nodeStore, RelationshipStore relationshipStore ) { this.nodeStore = nodeStore; + this.relationshipStore = relationshipStore; } @Override public void close() throws Exception { - if ( indexUpdatesExtractor.containsAnyNodeOrPropertyUpdate() ) + if ( indexUpdatesExtractor.containsAnyEntityOrPropertyUpdate() ) { // Queue the index updates. When index updates from all transactions in this batch have been accumulated // we'll feed them to the index updates work sync at the end of the batch - indexUpdates().feed( indexUpdatesExtractor.propertyCommandsByNodeIds(), - indexUpdatesExtractor.nodeCommandsById() ); + indexUpdates().feed( indexUpdatesExtractor.propertyCommandsByNodeIds(), indexUpdatesExtractor.propertyCommandsByRelationshipIds(), + indexUpdatesExtractor.nodeCommandsById(), indexUpdatesExtractor.relationshipCommandsById() ); indexUpdatesExtractor.close(); } @@ -165,7 +166,7 @@ private IndexUpdates indexUpdates() { if ( indexUpdates == null ) { - indexUpdates = new OnlineIndexUpdates( nodeStore, indexingService, indexUpdateConverter ); + indexUpdates = new OnlineIndexUpdates( nodeStore, relationshipStore, indexingService, indexUpdateConverter ); } return indexUpdates; } @@ -198,6 +199,12 @@ public boolean visitNodeCommand( Command.NodeCommand command ) return indexUpdatesExtractor.visitNodeCommand( command ); } + @Override + public boolean visitRelationshipCommand( Command.RelationshipCommand command ) throws IOException + { + return indexUpdatesExtractor.visitRelationshipCommand( command ); + } + @Override public boolean visitPropertyCommand( PropertyCommand command ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexUpdatesWork.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexUpdatesWork.java index 4083f96248929..793c482e354b6 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexUpdatesWork.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/command/IndexUpdatesWork.java @@ -87,8 +87,9 @@ protected Iterator> createNestedIterator( Ind } @Override - public void feed( LongObjectMap> propCommands, - LongObjectMap nodeCommands ) + public void feed( LongObjectMap> propCommandsByNodeId, + LongObjectMap> propCommandsByRelationshipId, LongObjectMap nodeCommands, + LongObjectMap relationshipCommandPrimitiveLongObjectMap ) { throw new UnsupportedOperationException(); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/DirectIndexUpdates.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/DirectIndexUpdates.java index 6cc294e276bf6..36742339da7f5 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/DirectIndexUpdates.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/DirectIndexUpdates.java @@ -26,6 +26,7 @@ import org.neo4j.internal.kernel.api.schema.SchemaDescriptor; import org.neo4j.kernel.api.index.IndexEntryUpdate; +import org.neo4j.kernel.impl.transaction.command.Command; import org.neo4j.kernel.impl.transaction.command.Command.NodeCommand; import org.neo4j.kernel.impl.transaction.command.Command.PropertyCommand; @@ -48,7 +49,8 @@ public Iterator> iterator() } @Override - public void feed( LongObjectMap> propCommands, LongObjectMap nodeCommands ) + public void feed( LongObjectMap> propCommandsByNodeId, LongObjectMap> propCommandsByRelationshipId, + LongObjectMap nodeCommands, LongObjectMap relationshipCommandPrimitiveLongObjectMap ) { throw new UnsupportedOperationException(); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/IndexUpdates.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/IndexUpdates.java index 2d024fd8fdb3e..f4a57bef38ae3 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/IndexUpdates.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/IndexUpdates.java @@ -25,6 +25,7 @@ import org.neo4j.internal.kernel.api.schema.SchemaDescriptor; import org.neo4j.kernel.api.index.IndexEntryUpdate; +import org.neo4j.kernel.impl.transaction.command.Command; import org.neo4j.kernel.impl.transaction.command.Command.NodeCommand; import org.neo4j.kernel.impl.transaction.command.Command.PropertyCommand; @@ -35,11 +36,13 @@ public interface IndexUpdates extends Iterable> propCommands, LongObjectMap nodeCommands ); + void feed( LongObjectMap> propCommandsByNodeId, LongObjectMap> propCommandsByRelationshipId, + LongObjectMap nodeCommands, LongObjectMap relationshipCommandPrimitiveLongObjectMap ); boolean hasUpdates(); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/OnlineIndexUpdates.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/OnlineIndexUpdates.java index 6d7e487a74082..1e0ae54638160 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/OnlineIndexUpdates.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/OnlineIndexUpdates.java @@ -19,6 +19,7 @@ */ package org.neo4j.kernel.impl.transaction.state; +import org.eclipse.collections.api.iterator.LongIterator; import org.eclipse.collections.api.map.primitive.LongObjectMap; import org.eclipse.collections.api.set.primitive.LongSet; import org.eclipse.collections.api.set.primitive.MutableLongSet; @@ -36,37 +37,42 @@ import org.neo4j.kernel.impl.api.index.IndexingUpdateService; import org.neo4j.kernel.impl.api.index.PropertyPhysicalToLogicalConverter; import org.neo4j.kernel.impl.store.NodeStore; +import org.neo4j.kernel.impl.store.RelationshipStore; import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.kernel.impl.store.record.RecordLoad; +import org.neo4j.kernel.impl.store.record.RelationshipRecord; 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.storageengine.api.EntityType; import static org.neo4j.kernel.impl.store.NodeLabelsField.parseLabelsField; /** - * Derives logical index updates from physical records, provided by {@link NodeCommand node commands} and - * {@link PropertyCommand property commands}. For some types of updates state from store is also needed, - * for example if adding a label to a node which already has properties matching existing and online indexes; - * in that case the properties for that node needs to be read from store since the commands in that transaction - * cannot itself provide enough information. + * Derives logical index updates from physical records, provided by {@link NodeCommand node commands}, + * {@link RelationshipCommand relationship commands} and {@link PropertyCommand property commands}. For some + * types of updates state from store is also needed, for example if adding a label to a node which already has + * properties matching existing and online indexes; in that case the properties for that node needs to be read + * from store since the commands in that transaction cannot itself provide enough information. * - * One instance can be {@link #feed(LongObjectMap, LongObjectMap) fed} data about + * One instance can be {@link #feed(LongObjectMap, LongObjectMap, LongObjectMap, LongObjectMap) fed} data about * multiple transactions, to be {@link #iterator() accessed} later. */ public class OnlineIndexUpdates implements IndexUpdates { private final NodeStore nodeStore; + private final RelationshipStore relationshipStore; private final IndexingUpdateService updateService; private final PropertyPhysicalToLogicalConverter converter; private final Collection> updates = new ArrayList<>(); private NodeRecord nodeRecord; + private RelationshipRecord relationshipRecord; - public OnlineIndexUpdates( NodeStore nodeStore, - IndexingUpdateService updateService, - PropertyPhysicalToLogicalConverter converter ) + public OnlineIndexUpdates( NodeStore nodeStore, RelationshipStore relationshipStore, IndexingUpdateService updateService, + PropertyPhysicalToLogicalConverter converter ) { this.nodeStore = nodeStore; + this.relationshipStore = relationshipStore; this.updateService = updateService; this.converter = converter; } @@ -78,11 +84,22 @@ public Iterator> iterator() } @Override - public void feed( LongObjectMap> propertyCommands, LongObjectMap nodeCommands ) + public void feed( LongObjectMap> propCommandsByNodeId, + LongObjectMap> propertyCommandsByRelationshipId, LongObjectMap nodeCommands, + LongObjectMap relationshipCommands ) { - allKeys( nodeCommands, propertyCommands ).forEach( nodeId -> - gatherUpdatesFor( nodeId, nodeCommands.get( nodeId ), propertyCommands.get( nodeId ) ) - ); + LongIterator nodeIds = allKeys( nodeCommands, propCommandsByNodeId ).longIterator(); + while ( nodeIds.hasNext() ) + { + long nodeId = nodeIds.next(); + gatherUpdatesFor( nodeId, nodeCommands.get( nodeId ), propCommandsByNodeId.get( nodeId ) ); + } + LongIterator relationshipIds = allKeys( relationshipCommands, propertyCommandsByRelationshipId ).longIterator(); + while ( relationshipIds.hasNext() ) + { + long relationshipId = relationshipIds.next(); + gatherUpdatesFor( relationshipId, relationshipCommands.get( relationshipId ), propertyCommandsByRelationshipId.get( relationshipId ) ); + } } private LongSet allKeys( LongObjectMap... maps ) @@ -115,6 +132,19 @@ private void gatherUpdatesFor( long nodeId, NodeCommand nodeCommand, List propertyCommands ) + { + EntityUpdates.Builder relationshipPropertyUpdate = gatherUpdatesFromCommandsForRelationship( reltionshipId, relationshipCommand, propertyCommands ); + + EntityUpdates entityUpdates = relationshipPropertyUpdate.build(); + // we need to materialize the IndexEntryUpdates here, because when we + // consume (later in separate thread) the store might have changed. + for ( IndexEntryUpdate update : updateService.convertToIndexUpdates( entityUpdates, EntityType.RELATIONSHIP ) ) + { + updates.add( update ); + } + } + private EntityUpdates.Builder gatherUpdatesFromCommandsForNode( long nodeId, NodeCommand nodeChanges, List propertyCommandsForNode ) @@ -158,6 +188,30 @@ private EntityUpdates.Builder gatherUpdatesFromCommandsForNode( long nodeId, return nodePropertyUpdates; } + private EntityUpdates.Builder gatherUpdatesFromCommandsForRelationship( long relationshipId, RelationshipCommand relationshipCommand, + List propertyCommands ) + { + long reltypeBefore; + long reltypeAfter; + if ( relationshipCommand != null ) + { + reltypeBefore = relationshipCommand.getBefore().getType(); + reltypeAfter = relationshipCommand.getAfter().getType(); + } + else + { + RelationshipRecord relationshipRecord = loadRelationship( relationshipId ); + reltypeBefore = reltypeAfter = relationshipRecord.getType(); + } + EntityUpdates.Builder relationshipPropertyUpdates = + EntityUpdates.forEntity( relationshipId ).withTokensBefore( reltypeBefore ).withTokensAfter( reltypeAfter ); + if ( propertyCommands != null ) + { + converter.convertPropertyRecord( relationshipId, Iterables.cast( propertyCommands ), relationshipPropertyUpdates ); + } + return relationshipPropertyUpdates; + } + private NodeRecord loadNode( long nodeId ) { if ( nodeRecord == null ) @@ -167,4 +221,14 @@ private NodeRecord loadNode( long nodeId ) nodeStore.getRecord( nodeId, nodeRecord, RecordLoad.NORMAL ); return nodeRecord; } + + private RelationshipRecord loadRelationship( long relationshipId ) + { + if ( relationshipRecord == null ) + { + relationshipRecord = relationshipStore.newRecord(); + } + relationshipStore.getRecord( relationshipId, relationshipRecord, RecordLoad.NORMAL ); + return relationshipRecord; + } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java index 3333ca9923dc4..06c850533ef57 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexingServiceTest.java @@ -77,6 +77,7 @@ import org.neo4j.kernel.impl.storemigration.StoreMigrationParticipant; 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.state.DefaultIndexProviderMap; import org.neo4j.kernel.impl.transaction.state.DirectIndexUpdates; import org.neo4j.kernel.impl.transaction.state.IndexUpdates; @@ -292,7 +293,7 @@ public void indexPopulationScanComplete() order.verify( populator ).includeSample( add( 1, "value1" ) ); order.verify( populator, times( 2 ) ).add( any( Collection.class ) ); - // invoked from indexAllNodes(), empty because the id we added (2) is bigger than the one we indexed (1) + // invoked from indexAllEntities(), empty because the id we added (2) is bigger than the one we indexed (1) // // (We don't get an update for value2 here because we mock a fake store that doesn't contain it // just for the purpose of testing this behavior) @@ -733,7 +734,8 @@ public Iterator> iterator() } @Override - public void feed( LongObjectMap> propCommands, LongObjectMap nodeCommands ) + public void feed( LongObjectMap> propCommandsByNodeId, LongObjectMap> propCommandsByRelationshipId, + LongObjectMap nodeCommands, LongObjectMap relationshipCommands ) { throw new UnsupportedOperationException(); } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/TransactionRecordStateTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/TransactionRecordStateTest.java index ca3c8104a466c..841ff876f4c86 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/TransactionRecordStateTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/TransactionRecordStateTest.java @@ -45,7 +45,7 @@ import org.neo4j.kernel.impl.api.TransactionToApply; import org.neo4j.kernel.impl.api.index.EntityUpdates; import org.neo4j.kernel.impl.api.index.IndexingUpdateService; -import org.neo4j.kernel.impl.api.index.NodePropertyCommandsExtractor; +import org.neo4j.kernel.impl.api.index.PropertyCommandsExtractor; import org.neo4j.kernel.impl.api.index.PropertyPhysicalToLogicalConverter; import org.neo4j.kernel.impl.core.CacheAccessBackDoor; import org.neo4j.kernel.impl.locking.Lock; @@ -111,6 +111,7 @@ import static org.neo4j.helpers.collection.Iterables.count; import static org.neo4j.helpers.collection.Iterables.filter; import static org.neo4j.kernel.api.schema.SchemaDescriptorFactory.forLabel; +import static org.neo4j.kernel.api.schema.SchemaDescriptorFactory.forRelType; import static org.neo4j.kernel.api.schema.constaints.ConstraintDescriptorFactory.uniqueForLabel; import static org.neo4j.kernel.api.schema.index.IndexDescriptorFactory.forSchema; import static org.neo4j.kernel.impl.api.index.TestIndexProviderDescriptor.PROVIDER_DESCRIPTOR; @@ -202,7 +203,7 @@ private static int manuallyCountRelationships( RecordChangeSet recordChangeSet, private RecordChangeSet recordChangeSet; @Test - public void shouldCreateEqualNodePropertyUpdatesOnRecoveryOfCreatedNode() throws Exception + public void shouldCreateEqualEntityPropertyUpdatesOnRecoveryOfCreatedEntities() throws Exception { /* There was an issue where recovering a tx where a node with a label and a property * was created resulted in two exact copies of NodePropertyUpdates. */ @@ -210,35 +211,48 @@ public void shouldCreateEqualNodePropertyUpdatesOnRecoveryOfCreatedNode() throws // GIVEN NeoStores neoStores = neoStoresRule.builder().build(); long nodeId = 0; + long relId = 1; int labelId = 5; + int relTypeId = 4; int propertyKeyId = 7; - // -- an index - long ruleId = 0; + // -- indexes + long nodeRuleId = 0; TransactionRecordState recordState = newTransactionRecordState( neoStores ); - SchemaRule rule = forSchema( forLabel( labelId, propertyKeyId ), PROVIDER_DESCRIPTOR ).withId( ruleId ); - recordState.createSchemaRule( rule ); + SchemaRule nodeRule = forSchema( forLabel( labelId, propertyKeyId ), PROVIDER_DESCRIPTOR ).withId( nodeRuleId ); + recordState.createSchemaRule( nodeRule ); + long relRuleId = 1; + SchemaRule relRule = forSchema( forRelType( relTypeId, propertyKeyId ), PROVIDER_DESCRIPTOR ).withId( relRuleId ); + recordState.createSchemaRule( relRule ); apply( neoStores, recordState ); - // -- and a tx creating a node with that label and property key + // -- and a tx creating a node and a rel for those indexes recordState = newTransactionRecordState( neoStores ); recordState.nodeCreate( nodeId ); recordState.addLabelToNode( labelId, nodeId ); recordState.nodeAddProperty( nodeId, propertyKeyId, Values.of( "Neo" ) ); + recordState.relCreate( relId, relTypeId, nodeId, nodeId ); + recordState.relAddProperty( relId, propertyKeyId, Values.of( "Oen" ) ); // WHEN PhysicalTransactionRepresentation transaction = transactionRepresentationOf( recordState ); - NodePropertyCommandsExtractor extractor = new NodePropertyCommandsExtractor(); + PropertyCommandsExtractor extractor = new PropertyCommandsExtractor(); transaction.accept( extractor ); // THEN - // -- later recovering that tx, there should be only one update - assertTrue( extractor.containsAnyNodeOrPropertyUpdate() ); + // -- later recovering that tx, there should be only one update for each type + assertTrue( extractor.containsAnyEntityOrPropertyUpdate() ); MutableLongSet recoveredNodeIds = new LongHashSet(); recoveredNodeIds.addAll( extractor.nodeCommandsById().keySet() ); recoveredNodeIds.addAll( extractor.propertyCommandsByNodeIds().keySet() ); assertEquals( 1, recoveredNodeIds.size() ); assertEquals( nodeId, recoveredNodeIds.longIterator().next() ); + + MutableLongSet recoveredRelIds = new LongHashSet(); + recoveredRelIds.addAll( extractor.relationshipCommandsById().keySet() ); + recoveredRelIds.addAll( extractor.propertyCommandsByRelationshipIds().keySet() ); + assertEquals( 1, recoveredRelIds.size() ); + assertEquals( relId, recoveredRelIds.longIterator().next() ); } @Test @@ -1253,14 +1267,14 @@ private Iterable indexUpdatesOf( NeoStores neoStores, Transaction private Iterable indexUpdatesOf( NeoStores neoStores, TransactionRepresentation transaction ) throws IOException { - NodePropertyCommandsExtractor extractor = new NodePropertyCommandsExtractor(); + PropertyCommandsExtractor extractor = new PropertyCommandsExtractor(); transaction.accept( extractor ); CollectingIndexingUpdateService indexingUpdateService = new CollectingIndexingUpdateService(); - OnlineIndexUpdates onlineIndexUpdates = new OnlineIndexUpdates( neoStores.getNodeStore(), - indexingUpdateService, + OnlineIndexUpdates onlineIndexUpdates = new OnlineIndexUpdates( neoStores.getNodeStore(), neoStores.getRelationshipStore(), indexingUpdateService, new PropertyPhysicalToLogicalConverter( neoStores.getPropertyStore() ) ); - onlineIndexUpdates.feed( extractor.propertyCommandsByNodeIds(), extractor.nodeCommandsById() ); + onlineIndexUpdates.feed( extractor.propertyCommandsByNodeIds(), extractor.propertyCommandsByRelationshipIds(), extractor.nodeCommandsById(), + extractor.relationshipCommandsById() ); return indexingUpdateService.entityUpdatesList; } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplierTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplierTest.java index ed73ee82bb573..ae592f4e00512 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplierTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/IndexBatchTransactionApplierTest.java @@ -34,6 +34,7 @@ import org.neo4j.kernel.impl.store.NodeLabelsField; import org.neo4j.kernel.impl.store.NodeStore; import org.neo4j.kernel.impl.store.PropertyStore; +import org.neo4j.kernel.impl.store.RelationshipStore; import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.kernel.impl.transaction.command.Command.NodeCommand; import org.neo4j.storageengine.api.EntityType; @@ -63,9 +64,8 @@ public void shouldProvideLabelScanStoreUpdatesSortedByNodeId() throws Exception WorkSync indexUpdatesSync = new WorkSync<>( indexing ); TransactionToApply tx = mock( TransactionToApply.class ); PropertyStore propertyStore = mock( PropertyStore.class ); - try ( IndexBatchTransactionApplier applier = new IndexBatchTransactionApplier( indexing, labelScanSync, - indexUpdatesSync, mock( NodeStore.class ), - new PropertyPhysicalToLogicalConverter( propertyStore ) ) ) + try ( IndexBatchTransactionApplier applier = new IndexBatchTransactionApplier( indexing, labelScanSync, indexUpdatesSync, mock( NodeStore.class ), + mock( RelationshipStore.class ), new PropertyPhysicalToLogicalConverter( propertyStore ) ) ) { try ( TransactionApplier txApplier = applier.startTx( tx ) ) { diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/NeoStoreTransactionApplierTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/NeoStoreTransactionApplierTest.java index 04beaf939c46d..e048392880c60 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/NeoStoreTransactionApplierTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/NeoStoreTransactionApplierTest.java @@ -907,8 +907,7 @@ private BatchTransactionApplier newApplierFacade( BatchTransactionApplier... app private BatchTransactionApplier newIndexApplier() { return new IndexBatchTransactionApplier( indexingService, labelScanStoreSynchronizer, - indexUpdatesSync, nodeStore, - new PropertyPhysicalToLogicalConverter( propertyStore ) ); + indexUpdatesSync, nodeStore, neoStores.getRelationshipStore(), new PropertyPhysicalToLogicalConverter( propertyStore ) ); } // SCHEMA RULE COMMAND diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/NeoTransactionIndexApplierTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/NeoTransactionIndexApplierTest.java index 1a5125dfd159d..e36c0fa133e1b 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/NeoTransactionIndexApplierTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/NeoTransactionIndexApplierTest.java @@ -38,6 +38,7 @@ import org.neo4j.kernel.impl.api.index.PropertyPhysicalToLogicalConverter; import org.neo4j.kernel.impl.store.NodeStore; import org.neo4j.kernel.impl.store.PropertyStore; +import org.neo4j.kernel.impl.store.RelationshipStore; import org.neo4j.kernel.impl.store.record.DynamicRecord; import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.storageengine.api.EntityType; @@ -100,9 +101,8 @@ public void shouldUpdateLabelStoreScanOnNodeCommands() throws Exception private IndexBatchTransactionApplier newIndexTransactionApplier() { PropertyStore propertyStore = mock( PropertyStore.class ); - return new IndexBatchTransactionApplier( indexingService, - labelScanStoreSynchronizer, indexUpdatesSync, mock( NodeStore.class ), - new PropertyPhysicalToLogicalConverter( propertyStore ) ); + return new IndexBatchTransactionApplier( indexingService, labelScanStoreSynchronizer, indexUpdatesSync, mock( NodeStore.class ), + mock( RelationshipStore.class ), new PropertyPhysicalToLogicalConverter( propertyStore ) ); } @Test diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/SchemaRuleCommandTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/SchemaRuleCommandTest.java index 6f709b575c384..6a5d7d54e001a 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/SchemaRuleCommandTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/SchemaRuleCommandTest.java @@ -27,6 +27,7 @@ import org.neo4j.internal.kernel.api.schema.SchemaDescriptorPredicates; import org.neo4j.kernel.api.labelscan.LabelScanWriter; import org.neo4j.kernel.api.schema.constaints.ConstraintDescriptorFactory; +import org.neo4j.kernel.api.schema.index.StoreIndexDescriptor; import org.neo4j.kernel.api.schema.index.TestIndexDescriptorFactory; import org.neo4j.kernel.impl.api.BatchTransactionApplier; import org.neo4j.kernel.impl.api.TransactionToApply; @@ -42,7 +43,6 @@ import org.neo4j.kernel.impl.store.SchemaStore; import org.neo4j.kernel.impl.store.record.ConstraintRule; import org.neo4j.kernel.impl.store.record.DynamicRecord; -import org.neo4j.kernel.api.schema.index.StoreIndexDescriptor; import org.neo4j.kernel.impl.store.record.SchemaRecord; import org.neo4j.kernel.impl.store.record.SchemaRuleSerialization; import org.neo4j.kernel.impl.transaction.command.BaseCommandReader; @@ -67,7 +67,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.neo4j.kernel.impl.api.index.TestIndexProviderDescriptor.PROVIDER_DESCRIPTOR; public class SchemaRuleCommandTest { @@ -88,9 +87,9 @@ public class SchemaRuleCommandTest new WorkSync<>( labelScanStore ); private final WorkSync indexUpdatesSync = new WorkSync<>( indexes ); private final PropertyStore propertyStore = mock( PropertyStore.class ); - private final IndexBatchTransactionApplier indexApplier = new IndexBatchTransactionApplier( indexes, - labelScanStoreSynchronizer, indexUpdatesSync, mock( NodeStore.class ), - new PropertyPhysicalToLogicalConverter( propertyStore ) ); + private final IndexBatchTransactionApplier indexApplier = + new IndexBatchTransactionApplier( indexes, labelScanStoreSynchronizer, indexUpdatesSync, mock( NodeStore.class ), neoStores.getRelationshipStore(), + new PropertyPhysicalToLogicalConverter( propertyStore ) ); private final BaseCommandReader reader = new PhysicalLogCommandReaderV3_0_2(); private final StoreIndexDescriptor rule = TestIndexDescriptorFactory.forLabel( labelId, propertyKey ).withId( id );