Skip to content

Commit

Permalink
Merge pull request #9373 from davidegrohmann/3.3-small-improvements
Browse files Browse the repository at this point in the history 
Small improvements
  • Loading branch information
@tinwelint
tinwelint committed May 22, 2017
2 parents bd34af2 + ae1cf32 commit febeb00
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 45 deletions.
15 changes: 0 additions & 15 deletions community/common/src/main/java/org/neo4j/function/Predicates.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,19 +271,4 @@ public static <T> Predicate<T> in( final Iterable<T> allowed )
}

public static IntPredicate ALWAYS_TRUE_INT = v -> true;

public static IntPredicate any( int[] values )
{
return v ->
{
for ( int value : values )
{
if ( v == value )
{
return true;
}
}
return false;
};
}
}
50 changes: 31 additions & 19 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeRelationshipCursor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.neo4j.kernel.impl.api.store;

import java.util.function.Consumer;
import java.util.function.IntPredicate;

import org.neo4j.collection.primitive.PrimitiveLongIterator;
import org.neo4j.io.pagecache.PageCursor;
Expand All @@ -34,8 +33,6 @@
import org.neo4j.storageengine.api.Direction;
import org.neo4j.storageengine.api.txstate.ReadableTransactionState;

import static org.neo4j.function.Predicates.ALWAYS_TRUE_INT;
import static org.neo4j.function.Predicates.any;
import static org.neo4j.kernel.impl.store.record.Record.NO_NEXT_RELATIONSHIP;
import static org.neo4j.kernel.impl.store.record.Record.NULL_REFERENCE;
import static org.neo4j.kernel.impl.store.record.RecordLoad.FORCE;
Expand All @@ -56,7 +53,7 @@ public class NodeRelationshipCursor extends AbstractIteratorRelationshipCursor
private long relationshipId;
private long fromNodeId;
private Direction direction;
private IntPredicate allowedTypes;
private int[] allowedTypes;
private int groupChainIndex;
private boolean end;

Expand All @@ -76,19 +73,19 @@ public NodeRelationshipCursor init( boolean isDense, long firstRelId, long fromN
ReadableTransactionState state )
{
PrimitiveLongIterator addedNodeRelationships = addedNodeRelationships( fromNodeId, direction, null, state );
return init( isDense, firstRelId, fromNodeId, direction, ALWAYS_TRUE_INT, state, addedNodeRelationships );
return init( isDense, firstRelId, fromNodeId, direction, null, state, addedNodeRelationships );
}

public NodeRelationshipCursor init( boolean isDense, long firstRelId, long fromNodeId, Direction direction,
int[] allowedTypes, ReadableTransactionState state )
{
PrimitiveLongIterator addedNodeRelationships =
addedNodeRelationships( fromNodeId, direction, allowedTypes, state );
return init( isDense, firstRelId, fromNodeId, direction, any( allowedTypes ), state, addedNodeRelationships );
return init( isDense, firstRelId, fromNodeId, direction, allowedTypes, state, addedNodeRelationships );
}

private NodeRelationshipCursor init( boolean isDense, long firstRelId, long fromNodeId, Direction direction,
IntPredicate allowedTypes, ReadableTransactionState state, PrimitiveLongIterator addedNodeRelationships )
int[] allowedTypes, ReadableTransactionState state, PrimitiveLongIterator addedNodeRelationships )
{
internalInitTxState( state, addedNodeRelationships );
this.isDense = isDense;
Expand All @@ -114,11 +111,6 @@ private NodeRelationshipCursor init( boolean isDense, long firstRelId, long from
private PrimitiveLongIterator addedNodeRelationships( long fromNodeId, Direction direction,
int[] allowedTypes, ReadableTransactionState state )
{
if ( state == null )
{
return null;
}

return allowedTypes == null ? state.getNodeState( fromNodeId ).getAddedRelationships( direction )
: state.getNodeState( fromNodeId ).getAddedRelationships( direction, allowedTypes );
}
Expand All @@ -137,13 +129,17 @@ protected boolean doFetchNext()
// to chase a used record down the line.
try
{
// Direction check
if ( record.inUse() )
{
if ( direction != Direction.BOTH )
// direction is checked while reading the group chain, no need to check it here again
if ( !isDense )
{
// Direction check
switch ( direction )
{
case BOTH:
break;

case INCOMING:
{
if ( record.getSecondNode() != fromNodeId )
Expand All @@ -167,12 +163,11 @@ protected boolean doFetchNext()
}
}

// Type check
if ( !allowedTypes.test( record.getType() ) )
// Type check, for dense nodes it is checked while traversing the group records
if ( isDense || checkType( record.getType() ) )
{
continue;
return true;
}
return true;
}
}
finally
Expand Down Expand Up @@ -207,6 +202,23 @@ else if ( record.getSecondNode() == fromNodeId )
return false;
}

private boolean checkType( int type )
{
if ( allowedTypes == null )
{
return true;
}

for ( int allowedType : allowedTypes )
{
if ( type == allowedType )
{
return true;
}
}
return false;
}

@Override
public void close()
{
Expand All @@ -220,7 +232,7 @@ private long nextChainStart()
{
// We check inUse flag here since we can actually follow pointers in unused records
// to guard for and overcome concurrent deletes in the relationship group chain
if ( groupRecord.inUse() && allowedTypes.test( groupRecord.getType() ) )
if ( groupRecord.inUse() && checkType( groupRecord.getType() ) )
{
// Go to the next chain (direction) within this group
while ( groupChainIndex < GROUP_CHAINS.length )
Expand Down
4 changes: 2 additions & 2 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SinglePropertyCursor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ protected boolean loadNextFromDisk()
}

// go to disk if the state does not contain the keyId we are looking for
return state.getAddedProperty( propertyKeyId ) == null;
return !state.hasChanges() || state.getAddedProperty( propertyKeyId ) == null;
}

@Override
protected DefinedProperty nextAdded()
{
return !fetched ? (DefinedProperty) state.getAddedProperty( propertyKeyId ) : null;
return fetched ? null : (DefinedProperty) state.getAddedProperty( propertyKeyId );
}

@Override
Expand Down
34 changes: 25 additions & 9 deletions community/kernel/src/test/java/org/neo4j/kernel/impl/store/counts/CountsTrackerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.time.Clock;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeoutException;
import java.util.function.Predicate;

import org.neo4j.function.IOFunction;
Expand Down Expand Up @@ -374,16 +375,31 @@ public void shouldNotEndUpInBrokenStateAfterRotationFailure() throws Exception
{
tx.incrementNodeCount( labelId, 1 ); // now at 2
}
clock.forward( Config.empty().get( GraphDatabaseSettings.counts_store_rotation_timeout ) * 2, MILLISECONDS );
try
{
rotation.get();
fail( "Should've failed rotation due to timeout" );
}
catch ( ExecutionException e )

while ( true )
{
// good
assertTrue( e.getCause() instanceof RotationTimeoutException );
/*
* There is a race between forwarding the clock and the thread calling rotate, in case the call to forward
* the clock happens before the thread calls rotate, then this test hangs. In order to solve this issue,
* a timeout is set on the future and if the timeout is trigger the clock is forwarded again and we try
* again.
*/
clock.forward( Config.empty().get( GraphDatabaseSettings.counts_store_rotation_timeout ) * 2, MILLISECONDS );
try
{
rotation.get( 100, MILLISECONDS );
fail( "Should've failed rotation due to timeout" );
}
catch ( TimeoutException e )
{
// ignore try again
}
catch ( ExecutionException e )
{
// good
assertTrue( e.getCause() instanceof RotationTimeoutException );
break;
}
}

// THEN
Expand Down

0 comments on commit febeb00

Please sign in to comment.