From c6a975551ae9c25b7cb814e1af7064ac618c0ff6 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Fri, 31 Aug 2018 16:50:33 +0200 Subject: [PATCH] Add tests to prove that the concistency checker can find fulltext index inconsistencies, and add relationship index checking to the consistency checker. --- .../consistency/checking/full/CheckStage.java | 5 +- .../checking/full/ConsistencyCheckTasks.java | 19 +++- .../PropertyAndNode2LabelIndexProcessor.java | 2 +- .../full/PropertyAndNodeIndexedCheck.java | 32 ++++-- ...ropertyReader.java => PropertyReader.java} | 37 ++++-- .../full/RelationshipIndexProcessor.java | 47 ++++++++ .../full/RelationshipToIndexCheck.java | 107 ++++++++++++++++++ .../consistency/report/ConsistencyReport.java | 6 + 8 files changed, 232 insertions(+), 23 deletions(-) rename community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/{NodePropertyReader.java => PropertyReader.java} (71%) create mode 100644 community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/RelationshipIndexProcessor.java create mode 100644 community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/RelationshipToIndexCheck.java diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/CheckStage.java b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/CheckStage.java index a0a53e250c121..68b0fef789727 100644 --- a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/CheckStage.java +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/CheckStage.java @@ -40,8 +40,9 @@ public enum CheckStage implements Stage Stage7_RS_Backward( true, false, "RelationshipStore pass - reverse scan of source chain using the cache", 1, 1, ID_SLOT_SIZE, ID_SLOT_SIZE, 1 ), Stage8_PS_Props( true, true, "PropertyStore and Node to Index check pass" ), - Stage9_NS_LabelCounts( true, true, "NodeStore pass - Label counts" ), - Stage10_NS_PropertyRelocator( true, true, "Property store relocation" ); + Stage9_RS_Indexes( true, true, "Relationship to Index check pass" ), + Stage10_NS_LabelCounts( true, true, "NodeStore pass - Label counts" ), + Stage11_NS_PropertyRelocator( true, true, "Property store relocation" ); private final boolean parallel; private final boolean forward; diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/ConsistencyCheckTasks.java b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/ConsistencyCheckTasks.java index bd5092a0b965f..935b7fd019aae 100644 --- a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/ConsistencyCheckTasks.java +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/ConsistencyCheckTasks.java @@ -21,6 +21,8 @@ import java.util.ArrayList; import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import org.neo4j.consistency.RecordType; import org.neo4j.consistency.checking.NodeRecordCheck; @@ -48,6 +50,7 @@ import org.neo4j.kernel.impl.store.SchemaStorage; import org.neo4j.kernel.impl.store.StoreAccess; import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; +import org.neo4j.storageengine.api.EntityType; import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; import static java.lang.String.format; @@ -138,7 +141,7 @@ public List createTasksForFullCheck( boolean checkLabelS tasks.add( create( "RelationshipGroupStore-RelGrp", nativeStores.getRelationshipGroupStore(), relGrpProcessor, ROUND_ROBIN ) ); - NodePropertyReader propertyReader = new NodePropertyReader( nativeStores ); + PropertyReader propertyReader = new PropertyReader( nativeStores ); tasks.add( recordScanner( CheckStage.Stage8_PS_Props.name(), new IterableStore<>( nativeStores.getNodeStore(), true ), new PropertyAndNode2LabelIndexProcessor( reporter, checkIndexes ? indexes : null, @@ -146,6 +149,20 @@ public List createTasksForFullCheck( boolean checkLabelS CheckStage.Stage8_PS_Props, ROUND_ROBIN, new IterableStore<>( nativeStores.getPropertyStore(), true ) ) ); + // Checking that relationships are in their expected relationship indexes. + List relationshipIndexes = StreamSupport.stream( indexes.onlineRules().spliterator(), false ) + .filter( rule -> rule.schema().entityType() == EntityType.RELATIONSHIP ) + .collect( Collectors.toList() ); + if ( checkIndexes && !relationshipIndexes.isEmpty() ) + { + tasks.add( recordScanner( CheckStage.Stage9_RS_Indexes.name(), + new IterableStore<>( nativeStores.getRelationshipStore(), true ), + new RelationshipIndexProcessor( reporter, indexes, propertyReader, cacheAccess, relationshipIndexes ), + CheckStage.Stage9_RS_Indexes, + ROUND_ROBIN, + new IterableStore<>( nativeStores.getPropertyStore(), true ) ) ); + } + tasks.add( create( "StringStore-Str", nativeStores.getStringStore(), multiPass.processor( Stage.SEQUENTIAL_FORWARD, STRINGS ), ROUND_ROBIN ) ); tasks.add( create( "ArrayStore-Arrays", nativeStores.getArrayStore(), diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyAndNode2LabelIndexProcessor.java b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyAndNode2LabelIndexProcessor.java index 06d4998b3079c..7a21fafc49923 100644 --- a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyAndNode2LabelIndexProcessor.java +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyAndNode2LabelIndexProcessor.java @@ -46,7 +46,7 @@ public class PropertyAndNode2LabelIndexProcessor extends RecordProcessor.Adapter public PropertyAndNode2LabelIndexProcessor( ConsistencyReporter reporter, IndexAccessors indexes, - NodePropertyReader propertyReader, + PropertyReader propertyReader, CacheAccess cacheAccess, Function> mandatoryProperties ) { diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyAndNodeIndexedCheck.java b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyAndNodeIndexedCheck.java index f7b5361772e82..0b39259aa1645 100644 --- a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyAndNodeIndexedCheck.java +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyAndNodeIndexedCheck.java @@ -29,8 +29,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; import org.neo4j.consistency.checking.ChainCheck; import org.neo4j.consistency.checking.CheckerEngine; @@ -61,10 +59,10 @@ public class PropertyAndNodeIndexedCheck implements RecordCheck { private final IndexAccessors indexes; - private final NodePropertyReader propertyReader; + private final PropertyReader propertyReader; private final CacheAccess cacheAccess; - public PropertyAndNodeIndexedCheck( IndexAccessors indexes, NodePropertyReader propertyReader, CacheAccess cacheAccess ) + public PropertyAndNodeIndexedCheck( IndexAccessors indexes, PropertyReader propertyReader, CacheAccess cacheAccess ) { this.indexes = indexes; this.propertyReader = propertyReader; @@ -108,9 +106,10 @@ private void matchIndexesToNode( } int[] indexPropertyIds = schema.getPropertyIds(); - if ( nodeHasSchemaProperties( nodePropertyMap, indexPropertyIds ) ) + boolean requireAllProperties = schema.propertySchemaType() == SchemaDescriptor.PropertySchemaType.COMPLETE_ALL_TOKENS; + if ( requireAllProperties ? hasAllProperties( nodePropertyMap, indexPropertyIds ) : hasAnyProperty( nodePropertyMap, indexPropertyIds ) ) { - Value[] values = getPropertyValues( nodePropertyMap, indexPropertyIds ); + Value[] values = getPropertyValues( propertyReader, nodePropertyMap, indexPropertyIds ); try ( IndexReader reader = indexes.accessorFor( indexRule ).newReader() ) { long nodeId = record.getId(); @@ -204,7 +203,7 @@ private void checkProperty( NodeRecord record, } } - private Value[] getPropertyValues( IntObjectMap propertyMap, int[] indexPropertyIds ) + static Value[] getPropertyValues( PropertyReader propertyReader, IntObjectMap propertyMap, int[] indexPropertyIds ) { Value[] values = new Value[indexPropertyIds.length]; for ( int i = 0; i < indexPropertyIds.length; i++ ) @@ -215,7 +214,7 @@ private Value[] getPropertyValues( IntObjectMap propertyMap, int[ return values; } - private IntObjectMap properties( List propertyBlocks ) + static IntObjectMap properties( List propertyBlocks ) { final MutableIntObjectMap propertyIds = new IntObjectHashMap<>(); for ( PropertyBlock propertyBlock : propertyBlocks ) @@ -255,16 +254,27 @@ private LongIterator queryIndexOrEmpty( IndexReader reader, IndexQuery[] query ) ? indexedNodeIds : LookupFilter.exactIndexMatches( propertyReader, indexedNodeIds, query ); } - private static boolean nodeHasSchemaProperties( - IntObjectMap nodePropertyMap, int[] indexPropertyIds ) + static boolean hasAllProperties( IntObjectMap blockMap, int[] indexPropertyIds ) { for ( int indexPropertyId : indexPropertyIds ) { - if ( !nodePropertyMap.containsKey( indexPropertyId ) ) + if ( !blockMap.containsKey( indexPropertyId ) ) { return false; } } return true; } + + static boolean hasAnyProperty( IntObjectMap blockMap, int[] indexPropertyIds ) + { + for ( int indexPropertyId : indexPropertyIds ) + { + if ( blockMap.containsKey( indexPropertyId ) ) + { + return true; + } + } + return false; + } } diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/NodePropertyReader.java b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyReader.java similarity index 71% rename from community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/NodePropertyReader.java rename to community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyReader.java index 343e99385ca80..406198dc9c1a3 100644 --- a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/NodePropertyReader.java +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/PropertyReader.java @@ -27,33 +27,38 @@ import org.neo4j.kernel.api.index.NodePropertyAccessor; 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.StoreAccess; import org.neo4j.kernel.impl.store.record.NodeRecord; +import org.neo4j.kernel.impl.store.record.PrimitiveRecord; import org.neo4j.kernel.impl.store.record.PropertyBlock; import org.neo4j.kernel.impl.store.record.PropertyRecord; import org.neo4j.kernel.impl.store.record.Record; +import org.neo4j.kernel.impl.store.record.RelationshipRecord; import org.neo4j.values.storable.Value; import org.neo4j.values.storable.Values; import static org.neo4j.kernel.impl.store.record.RecordLoad.FORCE; -public class NodePropertyReader implements NodePropertyAccessor +public class PropertyReader implements NodePropertyAccessor { private final PropertyStore propertyStore; private final NodeStore nodeStore; + private final RelationshipStore relationshipStore; - public NodePropertyReader( StoreAccess storeAccess ) + PropertyReader( StoreAccess storeAccess ) { this.propertyStore = storeAccess.getRawNeoStores().getPropertyStore(); this.nodeStore = storeAccess.getRawNeoStores().getNodeStore(); + this.relationshipStore = storeAccess.getRawNeoStores().getRelationshipStore(); } - public Collection getPropertyRecordChain( NodeRecord nodeRecord ) + Collection getPropertyRecordChain( PrimitiveRecord entityRecord ) { - return getPropertyRecordChain( nodeRecord.getNextProp() ); + return getPropertyRecordChain( entityRecord.getNextProp() ); } - public Collection getPropertyRecordChain( long firstId ) + private Collection getPropertyRecordChain( long firstId ) { long nextProp = firstId; List toReturn = new LinkedList<>(); @@ -66,7 +71,7 @@ public Collection getPropertyRecordChain( long firstId ) return toReturn; } - public List propertyBlocks( Collection records ) + List propertyBlocks( Collection records ) { List propertyBlocks = new ArrayList<>(); for ( PropertyRecord record : records ) @@ -79,9 +84,9 @@ public List propertyBlocks( Collection records ) return propertyBlocks; } - public List propertyBlocks( NodeRecord nodeRecord ) + private List propertyBlocks( PrimitiveRecord entityRecord ) { - Collection records = propertyStore.getPropertyRecordChain( nodeRecord.getNextProp() ); + Collection records = propertyStore.getPropertyRecordChain( entityRecord.getNextProp() ); List propertyBlocks = new ArrayList<>(); for ( PropertyRecord record : records ) { @@ -114,4 +119,20 @@ public Value getNodePropertyValue( long nodeId, int propertyKeyId ) } return Values.NO_VALUE; } + + public Value getRelationshipPropertyValue( long relId, int propertyKeyId ) + { + RelationshipRecord record = relationshipStore.newRecord(); + if ( relationshipStore.getRecord( relId, record, FORCE ).inUse() ) + { + for ( PropertyBlock block : propertyBlocks( record ) ) + { + if ( block.getKeyIndexId() == propertyKeyId ) + { + return propertyValue( block ); + } + } + } + return Values.NO_VALUE; + } } diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/RelationshipIndexProcessor.java b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/RelationshipIndexProcessor.java new file mode 100644 index 0000000000000..5ee83dc701832 --- /dev/null +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/RelationshipIndexProcessor.java @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2002-2018 "Neo4j," + * Neo4j Sweden AB [http://neo4j.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.consistency.checking.full; + +import java.util.List; + +import org.neo4j.consistency.checking.cache.CacheAccess; +import org.neo4j.consistency.checking.index.IndexAccessors; +import org.neo4j.consistency.report.ConsistencyReporter; +import org.neo4j.kernel.impl.store.record.RelationshipRecord; +import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; + +public class RelationshipIndexProcessor extends RecordProcessor.Adapter +{ + private final ConsistencyReporter reporter; + private final RelationshipToIndexCheck checker; + + RelationshipIndexProcessor( ConsistencyReporter reporter, IndexAccessors indexes, PropertyReader propertyReader, CacheAccess cacheAccess, + List relationshipIndexes ) + { + this.reporter = reporter; + checker = new RelationshipToIndexCheck( relationshipIndexes, indexes, propertyReader, cacheAccess ); + } + + @Override + public void process( RelationshipRecord relationshipRecord ) + { + reporter.forRelationship( relationshipRecord, checker ); + } +} diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/RelationshipToIndexCheck.java b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/RelationshipToIndexCheck.java new file mode 100644 index 0000000000000..c1a5e50f6086a --- /dev/null +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/checking/full/RelationshipToIndexCheck.java @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2002-2018 "Neo4j," + * Neo4j Sweden AB [http://neo4j.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.consistency.checking.full; + +import org.apache.commons.lang3.ArrayUtils; +import org.eclipse.collections.api.map.primitive.IntObjectMap; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +import org.neo4j.consistency.checking.CheckerEngine; +import org.neo4j.consistency.checking.RecordCheck; +import org.neo4j.consistency.checking.cache.CacheAccess; +import org.neo4j.consistency.checking.index.IndexAccessors; +import org.neo4j.consistency.report.ConsistencyReport; +import org.neo4j.consistency.store.RecordAccess; +import org.neo4j.internal.kernel.api.schema.SchemaDescriptor; +import org.neo4j.kernel.impl.store.record.PropertyBlock; +import org.neo4j.kernel.impl.store.record.PropertyRecord; +import org.neo4j.kernel.impl.store.record.RelationshipRecord; +import org.neo4j.storageengine.api.schema.IndexReader; +import org.neo4j.storageengine.api.schema.StoreIndexDescriptor; +import org.neo4j.values.storable.Value; +import org.neo4j.values.storable.Values; + +import static org.neo4j.consistency.checking.full.PropertyAndNodeIndexedCheck.getPropertyValues; +import static org.neo4j.consistency.checking.full.PropertyAndNodeIndexedCheck.hasAllProperties; +import static org.neo4j.consistency.checking.full.PropertyAndNodeIndexedCheck.hasAnyProperty; +import static org.neo4j.consistency.checking.full.PropertyAndNodeIndexedCheck.properties; + +public class RelationshipToIndexCheck implements RecordCheck +{ + private final IndexAccessors indexes; + private final StoreIndexDescriptor[] relationshipIndexes; + private final PropertyReader propertyReader; + + RelationshipToIndexCheck( List relationshipIndexes, IndexAccessors indexes, PropertyReader propertyReader, CacheAccess cacheAccess ) + { + this.relationshipIndexes = relationshipIndexes.toArray( new StoreIndexDescriptor[0] ); + this.indexes = indexes; + this.propertyReader = propertyReader; + } + + @Override + public void check( RelationshipRecord record, CheckerEngine engine, + RecordAccess records ) + { + IntObjectMap nodePropertyMap = null; + for ( StoreIndexDescriptor index : relationshipIndexes ) + { + SchemaDescriptor schema = index.schema(); + if ( ArrayUtils.contains( schema.getEntityTokenIds(), record.getType() ) ) + { + if ( nodePropertyMap == null ) + { + Collection propertyRecs = propertyReader.getPropertyRecordChain( record ); + nodePropertyMap = properties( propertyReader.propertyBlocks( propertyRecs ) ); + } + + int[] indexPropertyIds = schema.getPropertyIds(); + boolean requireAllProperties = schema.propertySchemaType() == SchemaDescriptor.PropertySchemaType.COMPLETE_ALL_TOKENS; + if ( requireAllProperties ? hasAllProperties( nodePropertyMap, indexPropertyIds ) : hasAnyProperty( nodePropertyMap, indexPropertyIds ) ) + { + Value[] values = getPropertyValues( propertyReader, nodePropertyMap, indexPropertyIds ); + try ( IndexReader reader = indexes.accessorFor( index ).newReader() ) + { + assert !index.canSupportUniqueConstraint(); + long entityId = record.getId(); + long count = reader.countIndexedNodes( entityId, indexPropertyIds, values ); + reportIncorrectIndexCount( values, engine, index, count ); + } + } + } + } + } + + private void reportIncorrectIndexCount( Value[] values, CheckerEngine engine, + StoreIndexDescriptor index, long count ) + { + if ( count == 0 ) + { + engine.report().notIndexed( index, Values.asObjects( values ) ); + } + else if ( count != 1 ) + { + engine.report().indexedMultipleTimes( index, Values.asObjects( values ), count ); + } + } +} diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/report/ConsistencyReport.java b/community/consistency-check/src/main/java/org/neo4j/consistency/report/ConsistencyReport.java index d2874fab4e67d..6f0d675e02c24 100644 --- a/community/consistency-check/src/main/java/org/neo4j/consistency/report/ConsistencyReport.java +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/report/ConsistencyReport.java @@ -268,6 +268,12 @@ interface RelationshipConsistencyReport @Documented( "The next record in the target chain does not have this record as its previous record." ) void targetNextDoesNotReferenceBack( RelationshipRecord relationship ); + + @Documented( "This relationship was not found in the expected index." ) + void notIndexed( StoreIndexDescriptor index, Object[] propertyValues ); + + @Documented( "This relationship was found in the expected index, although multiple times" ) + void indexedMultipleTimes( StoreIndexDescriptor index, Object[] propertyValues, long count ); } interface PropertyConsistencyReport extends ConsistencyReport