Skip to content

Commit

Permalink
Fixes issue with node <--> index check reporting wrong inconsistencies
Browse files Browse the repository at this point in the history
  • Loading branch information
@tinwelint
tinwelint committed Jan 13, 2017
1 parent 9d45c11 commit 70c478e
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 44 deletions.
2 changes: 1 addition & 1 deletion ...unity/consistency-check/src/main/java/org/neo4j/consistency/checking/full/IndexCheck.java
View file
Expand Up @@ -39,6 +39,6 @@ public IndexCheck( IndexRule indexRule )
public void check( IndexEntry record, CheckerEngine<IndexEntry, ConsistencyReport.IndexConsistencyReport> engine, RecordAccess records )
{
engine.comparativeCheck( records.node( record.getId() ),
new NodeInUseWithCorrectLabelsCheck<IndexEntry,ConsistencyReport.IndexConsistencyReport>(new long[] {indexRule.getLabel()}) );
new NodeInUseWithCorrectLabelsCheck<IndexEntry,ConsistencyReport.IndexConsistencyReport>(new long[] {indexRule.getLabel()}, false ) );
}
}
61 changes: 36 additions & 25 deletions ...ck/src/main/java/org/neo4j/consistency/checking/full/NodeInUseWithCorrectLabelsCheck.java
View file
Expand Up @@ -39,14 +39,16 @@ public class NodeInUseWithCorrectLabelsCheck
<RECORD extends AbstractBaseRecord, REPORT extends ConsistencyReport.NodeInUseWithCorrectLabelsReport>
implements ComparativeRecordChecker<RECORD, NodeRecord, REPORT>
{
private final long[] expectedLabels;
private final long[] indexLabels;
private final boolean checkStoreToIndex;

public NodeInUseWithCorrectLabelsCheck( long[] expectedLabels )
public NodeInUseWithCorrectLabelsCheck( long[] expectedLabels, boolean checkStoreToIndex )
{
this.expectedLabels = sortAndDeduplicate( expectedLabels );
this.checkStoreToIndex = checkStoreToIndex;
this.indexLabels = sortAndDeduplicate( expectedLabels );
}

private long[] sortAndDeduplicate( long[] labels )
private static long[] sortAndDeduplicate( long[] labels )
{
sort( labels );
return PrimitiveLongCollections.dedup( labels );
Expand All @@ -71,9 +73,9 @@ public void checkReference( RECORD record, NodeRecord nodeRecord,
}
else
{
long[] actualLabels = nodeLabels.get( null );
long[] storeLabels = nodeLabels.get( null );
REPORT report = engine.report();
validateLabelIds( nodeRecord, actualLabels, report );
validateLabelIds( nodeRecord, storeLabels, report );
}
}
else
Expand All @@ -82,42 +84,51 @@ public void checkReference( RECORD record, NodeRecord nodeRecord,
}
}

private void validateLabelIds( NodeRecord nodeRecord, long[] actualLabels, REPORT report )
private void validateLabelIds( NodeRecord nodeRecord, long[] storeLabels, REPORT report )
{
actualLabels = sortAndDeduplicate( actualLabels );
storeLabels = sortAndDeduplicate( storeLabels );

int expectedIndex = 0;
int actualIndex = 0;
int indexLabelsCursor = 0;
int storeLabelsCursor = 0;

do
{
long expectedLabel = expectedLabels[expectedIndex];
long actualLabel = actualLabels[actualIndex];
if ( expectedLabel < actualLabel )
long indexLabel = indexLabels[indexLabelsCursor];
long storeLabel = storeLabels[storeLabelsCursor];
if ( indexLabel < storeLabel )
{ // node store has a label which isn't in label scan store
report.nodeLabelNotInIndex( nodeRecord, expectedLabel );
expectedIndex++;
report.nodeDoesNotHaveExpectedLabel( nodeRecord, indexLabel );
indexLabelsCursor++;
}
else if ( expectedLabel > actualLabel )
else if ( indexLabel > storeLabel )
{ // label scan store has a label which isn't in node store
report.nodeDoesNotHaveExpectedLabel( nodeRecord, actualLabel );
actualIndex++;
reportNodeLabelNotInIndex( report, nodeRecord, storeLabel );
storeLabelsCursor++;
}
else
{ // both match
expectedIndex++;
actualIndex++;
indexLabelsCursor++;
storeLabelsCursor++;
}
}
while ( expectedIndex < expectedLabels.length && actualIndex < actualLabels.length );
while ( indexLabelsCursor < indexLabels.length && storeLabelsCursor < storeLabels.length );

while ( expectedIndex < expectedLabels.length )
while ( indexLabelsCursor < indexLabels.length )
{
report.nodeLabelNotInIndex( nodeRecord, expectedLabels[expectedIndex++] );
report.nodeDoesNotHaveExpectedLabel( nodeRecord, indexLabels[indexLabelsCursor++] );
}
while ( actualIndex < actualLabels.length )
while ( storeLabelsCursor < storeLabels.length )
{
report.nodeDoesNotHaveExpectedLabel( nodeRecord, actualLabels[actualIndex++] );
reportNodeLabelNotInIndex( report, nodeRecord, storeLabels[storeLabelsCursor] );
storeLabelsCursor++;
}
}

private void reportNodeLabelNotInIndex( REPORT report, NodeRecord nodeRecord, long storeLabel )
{
if ( checkStoreToIndex )
{
report.nodeLabelNotInIndex( nodeRecord, storeLabel );
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...sistency-check/src/main/java/org/neo4j/consistency/checking/labelscan/LabelScanCheck.java
View file
Expand Up @@ -37,7 +37,7 @@ public void check( LabelScanDocument record, CheckerEngine<LabelScanDocument,
for ( long nodeId : range.nodes() )
{
engine.comparativeCheck( records.node( nodeId ),
new NodeInUseWithCorrectLabelsCheck<>( record.getNodeLabelRange().labels( nodeId ) ) );
new NodeInUseWithCorrectLabelsCheck<>( record.getNodeLabelRange().labels( nodeId ), true ) );
}
}
}
76 changes: 60 additions & 16 deletions ...rc/test/java/org/neo4j/consistency/checking/full/NodeInUseWithCorrectLabelsCheckTest.java
View file
Expand Up @@ -58,54 +58,98 @@ public void shouldReportNodeNotInUse() throws Exception
NodeRecord node = notInUse( new NodeRecord( nodeId, false, 0, 0 ) );

// when
checker( new long[]{} ).checkReference( null, node, engineFor( report ), null );
checker( new long[]{}, true ).checkReference( null, node, engineFor( report ), null );

// then
verify( report ).nodeNotInUse( node );
}

@Test
public void shouldReportNodeWithoutExpectedLabelWhenLabelsAreInline() throws Exception
public void shouldReportNodeWithoutExpectedLabelWhenLabelsAreInlineBothDirections() throws Exception
{
// given
int nodeId = 42;
int labelId1 = 7;
int labelId2 = 9;
long[] storeLabelIds = new long[] {7, 9};
long[] indexLabelIds = new long[] { 9, 10};

NodeRecord node = inUse( withInlineLabels( new NodeRecord( nodeId, false, 0, 0 ), labelId1 ) );
NodeRecord node = inUse( withInlineLabels( new NodeRecord( nodeId, false, 0, 0 ), storeLabelIds ) );

ConsistencyReport.LabelScanConsistencyReport report =
mock( ConsistencyReport.LabelScanConsistencyReport.class );

// when
checker(new long[] {labelId1, labelId2}).checkReference( null, node, engineFor( report ), null );
checker( indexLabelIds, true ).checkReference( null, node, engineFor( report ), null );

// then
verify( report ).nodeLabelNotInIndex( node, labelId2 );
verify( report ).nodeDoesNotHaveExpectedLabel( node, 10 );
}

@Test
public void shouldReportNodeWithoutExpectedLabelWhenLabelsAreDynamic() throws Exception
public void shouldReportNodeWithoutExpectedLabelWhenLabelsAreInlineIndexToStore() throws Exception
{
// given
int nodeId = 42;
long[] expectedLabelIds = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
long[] presentLabelIds = {1, 2, 3, 4, 5, 6, 8, 9, 10};
long[] storeLabelIds = new long[] {7, 9};
long[] indexLabelIds = new long[] { 9, 10};

NodeRecord node = inUse( withInlineLabels( new NodeRecord( nodeId, false, 0, 0 ), storeLabelIds ) );

ConsistencyReport.LabelScanConsistencyReport report =
mock( ConsistencyReport.LabelScanConsistencyReport.class );

// when
checker( indexLabelIds, false ).checkReference( null, node, engineFor( report ), null );

// then
verify( report ).nodeDoesNotHaveExpectedLabel( node, 10 );
}

@Test
public void shouldReportNodeWithoutExpectedLabelWhenLabelsAreDynamicBothDirections() throws Exception
{
// given
int nodeId = 42;
long[] indexLabelIds = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
long[] storeLabelIds = {1, 2, 3, 4, 5, 6, 8, 9, 10, 11};

RecordAccessStub recordAccess = new RecordAccessStub();
NodeRecord node = inUse( withDynamicLabels( recordAccess, new NodeRecord( nodeId, false, 0, 0 ), storeLabelIds ) );

ConsistencyReport.LabelScanConsistencyReport report =
mock( ConsistencyReport.LabelScanConsistencyReport.class );

// when
CheckerEngine<LabelScanDocument, ConsistencyReport.LabelScanConsistencyReport> engine = recordAccess.engine( null, report );
checker( indexLabelIds, true ).checkReference( null, node, engine, recordAccess );
recordAccess.checkDeferred();

// then
verify( report ).nodeDoesNotHaveExpectedLabel( node, 7 );
verify( report ).nodeLabelNotInIndex( node, 11 );
}

@Test
public void shouldReportNodeWithoutExpectedLabelWhenLabelsAreDynamicIndexToStore() throws Exception
{
// given
int nodeId = 42;
long[] indexLabelIds = { 3, 7, 9, 10};
long[] storeLabelIds = {1, 2, 3, 4, 5, 6, 8, 9, 10};
long missingLabelId = 7;

RecordAccessStub recordAccess = new RecordAccessStub();
NodeRecord node = inUse( withDynamicLabels( recordAccess, new NodeRecord( nodeId, false, 0, 0 ), presentLabelIds ) );
NodeRecord node = inUse( withDynamicLabels( recordAccess, new NodeRecord( nodeId, false, 0, 0 ), storeLabelIds ) );

ConsistencyReport.LabelScanConsistencyReport report =
mock( ConsistencyReport.LabelScanConsistencyReport.class );

// when
CheckerEngine<LabelScanDocument, ConsistencyReport.LabelScanConsistencyReport> engine = recordAccess.engine( null, report );
checker( expectedLabelIds ).checkReference( null, node, engine, recordAccess );
checker( indexLabelIds, true ).checkReference( null, node, engine, recordAccess );
recordAccess.checkDeferred();

// then
verify( report ).nodeLabelNotInIndex( node, missingLabelId );
verify( report ).nodeDoesNotHaveExpectedLabel( node, missingLabelId );
}

@Test
Expand All @@ -121,7 +165,7 @@ public void shouldRemainSilentWhenEverythingIsInOrder() throws Exception
mock( ConsistencyReport.LabelScanConsistencyReport.class );

// when
checker( new long[]{labelId} ).checkReference( null, node, engineFor( report ), null );
checker( new long[]{labelId}, true ).checkReference( null, node, engineFor( report ), null );

// then
verifyNoMoreInteractions( report );
Expand Down Expand Up @@ -159,9 +203,9 @@ private Engine engineFor( ConsistencyReport.LabelScanConsistencyReport report )
return engine;
}

private NodeInUseWithCorrectLabelsCheck<LabelScanDocument, ConsistencyReport.LabelScanConsistencyReport> checker( long[] expectedLabels )
private NodeInUseWithCorrectLabelsCheck<LabelScanDocument, ConsistencyReport.LabelScanConsistencyReport> checker( long[] expectedLabels, boolean checkStoreToIndex )
{
return new NodeInUseWithCorrectLabelsCheck<>( expectedLabels );
return new NodeInUseWithCorrectLabelsCheck<>( expectedLabels, checkStoreToIndex );
}

interface Engine extends CheckerEngine<LabelScanDocument, ConsistencyReport.LabelScanConsistencyReport>
Expand Down
3 changes: 2 additions & 1 deletion community/kernel/src/test/java/org/neo4j/graphdb/LabelScanStoreStartupIT.java
View file
Expand Up @@ -146,9 +146,10 @@ private NeoStoreDataSource getDataSource()

private void checkLabelScanStoreAccessible( LabelScanStore labelScanStore ) throws IOException
{
int labelId = 1;
try ( LabelScanWriter labelScanWriter = labelScanStore.newWriter() )
{
labelScanWriter.write( NodeLabelUpdate.labelChanges( 1, new long[]{}, new long[]{1} ) );
labelScanWriter.write( NodeLabelUpdate.labelChanges( 1, new long[]{}, new long[]{labelId} ) );
}
try ( LabelScanReader labelScanReader = labelScanStore.newReader() )
{
Expand Down

0 comments on commit 70c478e

Please sign in to comment.