Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Integer overflow for large number of dense nodes #6253

Merged
merged 2 commits into from Jan 26, 2016
Merged

Fix Integer overflow for large number of dense nodes #6253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
edf3ebd
Change AtomicInteger to AtomicLong
spacecowboy Jan 18, 2016
98a6571
Add test for large number of dense nodes
spacecowboy Jan 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 26 additions & 7 deletions ...y/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/NodeRelationshipCache.java
View file
Open in desktop
Expand Up @@ -19,7 +19,7 @@
*/
package org.neo4j.unsafe.impl.batchimport.cache;

import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;

import org.neo4j.graphdb.Direction;

Expand All @@ -34,13 +34,20 @@ public class NodeRelationshipCache implements MemoryStatsVisitor.Visitable
private LongArray array;
private final int denseNodeThreshold;
private final RelGroupCache relGroupCache;
private final long base;

public NodeRelationshipCache( NumberArrayFactory arrayFactory, int denseNodeThreshold )
{
this( arrayFactory, denseNodeThreshold, 0 );
}

NodeRelationshipCache( NumberArrayFactory arrayFactory, int denseNodeThreshold, long base )
{
int chunkSize = 1_000_000;
this.array = arrayFactory.newDynamicLongArray( chunkSize, IdFieldManipulator.emptyField() );
this.denseNodeThreshold = denseNodeThreshold;
this.relGroupCache = new RelGroupCache( arrayFactory, chunkSize );
this.base = base;
this.relGroupCache = new RelGroupCache( arrayFactory, chunkSize, base );
}

/**
Expand Down Expand Up @@ -190,13 +197,17 @@ private static class RelGroupCache implements AutoCloseable, MemoryStatsVisitor.
private static final int INDEX_IN = 2;
private static final int INDEX_LOOP = 3;

// Used for testing high id values. Should always be zero in production
private long base;
private LongArray array;
private final AtomicInteger nextFreeId = new AtomicInteger();
private final AtomicLong nextFreeId;

RelGroupCache( NumberArrayFactory arrayFactory, long chunkSize )
RelGroupCache( NumberArrayFactory arrayFactory, long chunkSize, long base )
{
this.base = base;
assert chunkSize > 0;
this.array = arrayFactory.newDynamicLongArray( chunkSize, -1 );
this.nextFreeId = new AtomicLong( base );
}

private void clearRelationships()
Expand All @@ -213,10 +224,18 @@ private void clearRelationships()
private void clearRelationshipId( long relGroupIndex, int fieldIndex )
{
long index = index( relGroupIndex, fieldIndex );
array.set( index, IdFieldManipulator.cleanId( array.get( index ) ) );
array.set( rebase( index ), IdFieldManipulator.cleanId( array.get( index ) ) );
}

/**
* Compensate for test value of index (to avoid allocating all your RAM)
*/
private long rebase( long index )
{
return index - base;
}

private int nextFreeId()
private long nextFreeId()
{
return nextFreeId.getAndIncrement();
}
Expand Down Expand Up @@ -268,7 +287,7 @@ private int directionIndex( Direction direction )

private long index( long relGroupIndex, int fieldIndex )
{
return relGroupIndex*ENTRY_SIZE + fieldIndex;
return rebase( relGroupIndex ) * ENTRY_SIZE + fieldIndex;
}

public long allocate( int type, Direction direction, long relId, boolean incrementCount )
Expand Down
35 changes: 26 additions & 9 deletions ...rnel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/NodeRelationshipCacheTest.java
View file
Open in desktop
Expand Up @@ -21,8 +21,12 @@

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.mockito.InOrder;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Random;

import org.neo4j.graphdb.Direction;
Expand All @@ -39,13 +43,26 @@
import static java.lang.Math.max;
import static java.lang.System.currentTimeMillis;

@RunWith( Parameterized.class )
public class NodeRelationshipCacheTest
{
@Parameterized.Parameter( 0 )
public long base;

@Parameterized.Parameters
public static Collection<Object[]> data()
{
Collection<Object[]> data = new ArrayList<>();
data.add( new Object[]{ 0L } );
data.add( new Object[]{ (long) Integer.MAX_VALUE * 2 } );
return data;
}

@Test
public void shouldReportCorrectNumberOfDenseNodes() throws Exception
{
// GIVEN
NodeRelationshipCache cache = new NodeRelationshipCache( NumberArrayFactory.AUTO, 5 );
NodeRelationshipCache cache = new NodeRelationshipCache( NumberArrayFactory.AUTO, 5, base );
increment( cache, 2, 10 );
increment( cache, 5, 2 );
increment( cache, 7, 12 );
Expand All @@ -68,7 +85,7 @@ public void shouldGoThroughThePhases() throws Exception
{
// GIVEN
int nodeCount = 10;
NodeRelationshipCache link = new NodeRelationshipCache( NumberArrayFactory.OFF_HEAP, 20 );
NodeRelationshipCache link = new NodeRelationshipCache( NumberArrayFactory.OFF_HEAP, 20, base );
incrementRandomCounts( link, nodeCount, nodeCount*20 );

// Test sparse node semantics
Expand All @@ -91,7 +108,7 @@ public void shouldAddGroupAfterTheFirst() throws Exception
{
// GIVEN a dense node
long denseNode = 0;
NodeRelationshipCache link = new NodeRelationshipCache( NumberArrayFactory.AUTO, 1 );
NodeRelationshipCache link = new NodeRelationshipCache( NumberArrayFactory.AUTO, 1, base );
link.incrementCount( denseNode );
link.getAndPutRelationship( denseNode, 0, Direction.OUTGOING, 0, true );

Expand All @@ -105,7 +122,7 @@ public void shouldAddGroupAfterTheFirst() throws Exception
GroupVisitor visitor = mock( GroupVisitor.class );
assertEquals( 0L, link.getFirstRel( denseNode, visitor ) );
InOrder order = inOrder( visitor );
order.verify( visitor ).visit( denseNode, 0, 1L, 0L, 2L, -1L );
order.verify( visitor ).visit( denseNode, 0, base + 1L, 0L, 2L, -1L );
order.verify( visitor ).visit( denseNode, 1, -1L, 3L, 1L, -1L );
order.verifyNoMoreInteractions();
}
Expand All @@ -115,7 +132,7 @@ public void shouldAddGroupBeforeTheFirst() throws Exception
{
// GIVEN a dense node
long denseNode = 0;
NodeRelationshipCache link = new NodeRelationshipCache( NumberArrayFactory.AUTO, 1 );
NodeRelationshipCache link = new NodeRelationshipCache( NumberArrayFactory.AUTO, 1, base );
link.incrementCount( denseNode );
link.getAndPutRelationship( denseNode, 1, Direction.INCOMING, 1, true );

Expand All @@ -129,7 +146,7 @@ public void shouldAddGroupBeforeTheFirst() throws Exception
GroupVisitor visitor = mock( GroupVisitor.class );
assertEquals( 0L, link.getFirstRel( denseNode, visitor ) );
InOrder order = inOrder( visitor );
order.verify( visitor ).visit( denseNode, 0, 1L, 0L, 2L, -1L );
order.verify( visitor ).visit( denseNode, 0, base + 1L, 0L, 2L, -1L );
order.verify( visitor ).visit( denseNode, 1, -1L, 3L, 1L, -1L );
order.verifyNoMoreInteractions();
}
Expand All @@ -139,7 +156,7 @@ public void shouldAddGroupInTheMiddleIfTwo() throws Exception
{
// GIVEN a dense node
long denseNode = 0;
NodeRelationshipCache link = new NodeRelationshipCache( NumberArrayFactory.AUTO, 1 );
NodeRelationshipCache link = new NodeRelationshipCache( NumberArrayFactory.AUTO, 1, base );
link.incrementCount( denseNode );
link.getAndPutRelationship( denseNode, 0, Direction.OUTGOING, 0, true );
link.getAndPutRelationship( denseNode, 2, Direction.OUTGOING, 1, true );
Expand All @@ -155,8 +172,8 @@ public void shouldAddGroupInTheMiddleIfTwo() throws Exception
// THEN
GroupVisitor visitor = mock( GroupVisitor.class );
assertEquals( 0L, link.getFirstRel( denseNode, visitor ) );
verify( visitor ).visit( denseNode, 0, 2L, 0L, 3L, -1L );
verify( visitor ).visit( denseNode, 1, 1L, 4L, 2L, 6L );
verify( visitor ).visit( denseNode, 0, base + 2L, 0L, 3L, -1L );
verify( visitor ).visit( denseNode, 1, base + 1L, 4L, 2L, 6L );
verify( visitor ).visit( denseNode, 2, -1L, 1L, 5L, -1L );
verifyNoMoreInteractions( visitor );
}
Expand Down