Skip to content

Commit

Permalink
Clean up various things in the consistency checker and address PR rev…
Browse files Browse the repository at this point in the history
…iew comments.
  • Loading branch information
chrisvest committed Sep 5, 2018
1 parent 8a9c4a0 commit 2254b90
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 62 deletions.
Expand Up @@ -22,7 +22,6 @@
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;
Expand All @@ -42,6 +41,7 @@
import org.neo4j.consistency.store.synthetic.IndexRecord;
import org.neo4j.consistency.store.synthetic.LabelScanIndex;
import org.neo4j.helpers.collection.BoundedIterable;
import org.neo4j.helpers.collection.Iterables;
import org.neo4j.helpers.progress.ProgressMonitorFactory;
import org.neo4j.kernel.api.labelscan.LabelScanStore;
import org.neo4j.kernel.impl.index.labelscan.NativeLabelScanStore;
Expand Down Expand Up @@ -150,14 +150,14 @@ public List<ConsistencyCheckerTask> createTasksForFullCheck( boolean checkLabelS
new IterableStore<>( nativeStores.getPropertyStore(), true ) ) );

// Checking that relationships are in their expected relationship indexes.
List<StoreIndexDescriptor> relationshipIndexes = StreamSupport.stream( indexes.onlineRules().spliterator(), false )
List<StoreIndexDescriptor> relationshipIndexes = Iterables.stream( indexes.onlineRules() )
.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 ),
new RelationshipIndexProcessor( reporter, indexes, propertyReader, relationshipIndexes ),
CheckStage.Stage9_RS_Indexes,
ROUND_ROBIN,
new IterableStore<>( nativeStores.getPropertyStore(), true ) ) );
Expand Down
Expand Up @@ -28,24 +28,24 @@
import org.neo4j.storageengine.api.EntityType;
import org.neo4j.storageengine.api.schema.StoreIndexDescriptor;

public class IndexCheck implements RecordCheck<IndexEntry, ConsistencyReport.IndexConsistencyReport>
public class IndexCheck implements RecordCheck<IndexEntry,ConsistencyReport.IndexConsistencyReport>
{
private final long[] entityTokenLongIds;
private final SchemaDescriptor.PropertySchemaType propertySchemaType;
private final EntityType entityType;
private final StoreIndexDescriptor indexRule;
private NodeInUseWithCorrectLabelsCheck<IndexEntry,ConsistencyReport.IndexConsistencyReport> nodeChecker;
private RelationshipInUseWithCorrectRelationshipTypeCheck<IndexEntry,ConsistencyReport.IndexConsistencyReport> relationshipChecker;

public IndexCheck( StoreIndexDescriptor indexRule )
IndexCheck( StoreIndexDescriptor indexRule )
{
this.indexRule = indexRule;
SchemaDescriptor schema = indexRule.schema();
int[] entityTokenIntIds = schema.getEntityTokenIds();
entityTokenLongIds = new long[entityTokenIntIds.length];
long[] entityTokenLongIds = new long[entityTokenIntIds.length];
for ( int i = 0; i < entityTokenIntIds.length; i++ )
{
entityTokenLongIds[i] = entityTokenIntIds[i];
}
propertySchemaType = schema.propertySchemaType();
SchemaDescriptor.PropertySchemaType propertySchemaType = schema.propertySchemaType();
entityType = schema.entityType();
if ( entityType == EntityType.NODE )
{
Expand All @@ -58,19 +58,22 @@ public IndexCheck( StoreIndexDescriptor indexRule )
}

@Override
public void check( IndexEntry record, CheckerEngine<IndexEntry, ConsistencyReport.IndexConsistencyReport> engine, RecordAccess records )
public void check( IndexEntry record, CheckerEngine<IndexEntry,ConsistencyReport.IndexConsistencyReport> engine, RecordAccess records )
{
long id = record.getId();
if ( entityType == EntityType.NODE )
switch ( entityType )
{
case NODE:
engine.comparativeCheck( records.node( id ), nodeChecker );
}
else if ( entityType == EntityType.RELATIONSHIP )
{
break;
case RELATIONSHIP:
if ( indexRule.canSupportUniqueConstraint() )
{
engine.report().relationshipConstraintIndex();
}
engine.comparativeCheck( records.relationship( id ), relationshipChecker );
}
else
{
break;
default:
throw new IllegalStateException( "Don't know how to check index entry of entity type " + entityType );
}
}
Expand Down
Expand Up @@ -62,7 +62,7 @@ public class PropertyAndNodeIndexedCheck implements RecordCheck<NodeRecord, Cons
private final PropertyReader propertyReader;
private final CacheAccess cacheAccess;

public PropertyAndNodeIndexedCheck( IndexAccessors indexes, PropertyReader propertyReader, CacheAccess cacheAccess )
PropertyAndNodeIndexedCheck( IndexAccessors indexes, PropertyReader propertyReader, CacheAccess cacheAccess )
{
this.indexes = indexes;
this.propertyReader = propertyReader;
Expand Down Expand Up @@ -105,11 +105,9 @@ private void matchIndexesToNode(
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 ) )
if ( entityIntersectsSchema( nodePropertyMap, schema ) )
{
Value[] values = getPropertyValues( propertyReader, nodePropertyMap, indexPropertyIds );
Value[] values = getPropertyValues( propertyReader, nodePropertyMap, schema.getPropertyIds() );
try ( IndexReader reader = indexes.accessorFor( indexRule ).newReader() )
{
long nodeId = record.getId();
Expand All @@ -120,7 +118,7 @@ private void matchIndexesToNode(
}
else
{
long count = reader.countIndexedNodes( nodeId, indexPropertyIds, values );
long count = reader.countIndexedNodes( nodeId, schema.getPropertyIds(), values );
reportIncorrectIndexCount( values, engine, indexRule, count );
}
}
Expand Down Expand Up @@ -254,7 +252,20 @@ private LongIterator queryIndexOrEmpty( IndexReader reader, IndexQuery[] query )
? indexedNodeIds : LookupFilter.exactIndexMatches( propertyReader, indexedNodeIds, query );
}

static boolean hasAllProperties( IntObjectMap<PropertyBlock> blockMap, int[] indexPropertyIds )
static boolean entityIntersectsSchema( IntObjectMap<PropertyBlock> entityPropertyMap, SchemaDescriptor schema )
{
boolean requireAllTokens = schema.propertySchemaType() == SchemaDescriptor.PropertySchemaType.COMPLETE_ALL_TOKENS;
if ( requireAllTokens )
{
return hasAllProperties( entityPropertyMap, schema.getPropertyIds() );
}
else
{
return hasAnyProperty( entityPropertyMap, schema.getPropertyIds() );
}
}

private static boolean hasAllProperties( IntObjectMap<PropertyBlock> blockMap, int[] indexPropertyIds )
{
for ( int indexPropertyId : indexPropertyIds )
{
Expand All @@ -266,7 +277,7 @@ static boolean hasAllProperties( IntObjectMap<PropertyBlock> blockMap, int[] ind
return true;
}

static boolean hasAnyProperty( IntObjectMap<PropertyBlock> blockMap, int[] indexPropertyIds )
private static boolean hasAnyProperty( IntObjectMap<PropertyBlock> blockMap, int[] indexPropertyIds )
{
for ( int indexPropertyId : indexPropertyIds )
{
Expand Down
Expand Up @@ -27,14 +27,12 @@
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;

Expand All @@ -44,13 +42,11 @@ public class PropertyReader implements NodePropertyAccessor
{
private final PropertyStore propertyStore;
private final NodeStore nodeStore;
private final RelationshipStore relationshipStore;

PropertyReader( StoreAccess storeAccess )
{
this.propertyStore = storeAccess.getRawNeoStores().getPropertyStore();
this.nodeStore = storeAccess.getRawNeoStores().getNodeStore();
this.relationshipStore = storeAccess.getRawNeoStores().getRelationshipStore();
}

Collection<PropertyRecord> getPropertyRecordChain( PrimitiveRecord entityRecord )
Expand Down Expand Up @@ -119,20 +115,4 @@ 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;
}
}
Expand Up @@ -21,7 +21,6 @@

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;
Expand All @@ -32,11 +31,11 @@ public class RelationshipIndexProcessor extends RecordProcessor.Adapter<Relation
private final ConsistencyReporter reporter;
private final RelationshipToIndexCheck checker;

RelationshipIndexProcessor( ConsistencyReporter reporter, IndexAccessors indexes, PropertyReader propertyReader, CacheAccess cacheAccess,
RelationshipIndexProcessor( ConsistencyReporter reporter, IndexAccessors indexes, PropertyReader propertyReader,
List<StoreIndexDescriptor> relationshipIndexes )
{
this.reporter = reporter;
checker = new RelationshipToIndexCheck( relationshipIndexes, indexes, propertyReader, cacheAccess );
checker = new RelationshipToIndexCheck( relationshipIndexes, indexes, propertyReader );
}

@Override
Expand Down
Expand Up @@ -22,13 +22,11 @@
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;
Expand All @@ -41,9 +39,8 @@
import org.neo4j.values.storable.Value;
import org.neo4j.values.storable.Values;

import static org.neo4j.consistency.checking.full.PropertyAndNodeIndexedCheck.entityIntersectsSchema;
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<RelationshipRecord, ConsistencyReport.RelationshipConsistencyReport>
Expand All @@ -52,7 +49,7 @@ public class RelationshipToIndexCheck implements RecordCheck<RelationshipRecord,
private final StoreIndexDescriptor[] relationshipIndexes;
private final PropertyReader propertyReader;

RelationshipToIndexCheck( List<StoreIndexDescriptor> relationshipIndexes, IndexAccessors indexes, PropertyReader propertyReader, CacheAccess cacheAccess )
RelationshipToIndexCheck( List<StoreIndexDescriptor> relationshipIndexes, IndexAccessors indexes, PropertyReader propertyReader )
{
this.relationshipIndexes = relationshipIndexes.toArray( new StoreIndexDescriptor[0] );
this.indexes = indexes;
Expand All @@ -63,28 +60,25 @@ public class RelationshipToIndexCheck implements RecordCheck<RelationshipRecord,
public void check( RelationshipRecord record, CheckerEngine<RelationshipRecord,ConsistencyReport.RelationshipConsistencyReport> engine,
RecordAccess records )
{
IntObjectMap<PropertyBlock> nodePropertyMap = null;
IntObjectMap<PropertyBlock> propertyMap = null;
for ( StoreIndexDescriptor index : relationshipIndexes )
{
SchemaDescriptor schema = index.schema();
if ( ArrayUtils.contains( schema.getEntityTokenIds(), record.getType() ) )
{
if ( nodePropertyMap == null )
if ( propertyMap == null )
{
Collection<PropertyRecord> propertyRecs = propertyReader.getPropertyRecordChain( record );
nodePropertyMap = properties( propertyReader.propertyBlocks( propertyRecs ) );
propertyMap = 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 ) )
if ( entityIntersectsSchema( propertyMap, schema ) )
{
Value[] values = getPropertyValues( propertyReader, nodePropertyMap, indexPropertyIds );
Value[] values = getPropertyValues( propertyReader, propertyMap, schema.getPropertyIds() );
try ( IndexReader reader = indexes.accessorFor( index ).newReader() )
{
assert !index.canSupportUniqueConstraint();
long entityId = record.getId();
long count = reader.countIndexedNodes( entityId, indexPropertyIds, values );
long count = reader.countIndexedNodes( entityId, schema.getPropertyIds(), values );
reportIncorrectIndexCount( values, engine, index, count );
}
}
Expand Down
Expand Up @@ -536,6 +536,9 @@ interface IndexConsistencyReport extends NodeInUseWithCorrectLabelsReport, Relat
@Warning
@Documented( "Index was not properly shutdown and rebuild is required." )
void dirtyIndex();

@Documented( "This index entry is for a relationship index, but it is used as a constraint index" )
void relationshipConstraintIndex();
}

interface CountsConsistencyReport extends ConsistencyReport
Expand Down
Expand Up @@ -35,6 +35,7 @@ public interface IndexReader extends Resource
{
/**
* @param nodeId node id to match.
* @param propertyKeyIds the property key ids that correspond to each of the property values.
* @param propertyValues property values to match.
* @return number of index entries for the given {@code nodeId} and {@code propertyValues}.
*/
Expand Down

0 comments on commit 2254b90

Please sign in to comment.