Skip to content

Commit

Permalink
Use PageCursors for loading relationships in RelationshipCursors
Browse files Browse the repository at this point in the history
  • Loading branch information
davidegrohmann committed May 8, 2017
1 parent 4b462d0 commit 468edb6
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 195 deletions.
Expand Up @@ -21,8 +21,7 @@

import org.neo4j.collection.primitive.PrimitiveLongIterator;
import org.neo4j.kernel.impl.locking.LockService;
import org.neo4j.kernel.impl.store.RecordCursors;
import org.neo4j.kernel.impl.store.record.RelationshipRecord;
import org.neo4j.kernel.impl.store.RelationshipStore;
import org.neo4j.storageengine.api.txstate.ReadableTransactionState;

import static org.neo4j.kernel.api.StatementConstants.NO_SUCH_NODE;
Expand All @@ -35,10 +34,9 @@ public abstract class StoreAbstractIteratorRelationshipCursor extends StoreAbstr
private PrimitiveLongIterator addedRelationshipIterator;
private boolean fromStore;

StoreAbstractIteratorRelationshipCursor( RelationshipRecord relationshipRecord, RecordCursors cursors,
LockService lockService )
StoreAbstractIteratorRelationshipCursor( RelationshipStore relationshipStore, LockService lockService )
{
super( relationshipRecord, cursors, lockService );
super( relationshipStore, lockService );
}

void internalInitTxState( ReadableTransactionState state, PrimitiveLongIterator addedRelationshipIterator )
Expand Down
Expand Up @@ -19,14 +19,18 @@
*/
package org.neo4j.kernel.impl.api.store;

import java.io.IOException;

import org.neo4j.cursor.Cursor;
import org.neo4j.function.Disposable;
import org.neo4j.io.pagecache.PageCursor;
import org.neo4j.kernel.impl.api.RelationshipVisitor;
import org.neo4j.kernel.impl.locking.Lock;
import org.neo4j.kernel.impl.locking.LockService;
import org.neo4j.kernel.impl.store.RecordCursor;
import org.neo4j.kernel.impl.store.RecordCursors;
import org.neo4j.kernel.impl.store.RelationshipStore;
import org.neo4j.kernel.impl.store.UnderlyingStorageException;
import org.neo4j.kernel.impl.store.record.Record;
import org.neo4j.kernel.impl.store.record.RecordLoad;
import org.neo4j.kernel.impl.store.record.RelationshipRecord;
import org.neo4j.storageengine.api.RelationshipItem;

Expand All @@ -40,16 +44,17 @@
public abstract class StoreAbstractRelationshipCursor
implements RelationshipVisitor<RuntimeException>, RelationshipItem, Cursor<RelationshipItem>, Disposable
{
protected final RelationshipRecord relationshipRecord;
final RecordCursor<RelationshipRecord> relationshipRecordCursor;
private final RelationshipRecord relationshipRecord;
private final PageCursor cursor;
private final RelationshipStore relationshipStore;
private final LockService lockService;
protected boolean fetched;

StoreAbstractRelationshipCursor( RelationshipRecord relationshipRecord, RecordCursors cursors,
LockService lockService )
StoreAbstractRelationshipCursor( RelationshipStore relationshipStore, LockService lockService )
{
this.relationshipRecordCursor = cursors.relationship();
this.relationshipRecord = relationshipRecord;
this.relationshipStore = relationshipStore;
this.relationshipRecord = relationshipStore.newRecord();
this.cursor = relationshipStore.newPageCursor();
this.lockService = lockService;
}

Expand Down Expand Up @@ -118,6 +123,20 @@ public final long nextPropertyId()
return relationshipRecord.getNextProp();
}

RelationshipRecord readRecord( long id, RecordLoad mode )
{
try
{
relationshipRecord.clear();
relationshipStore.readIntoRecord( id, relationshipRecord, mode, cursor );
return relationshipRecord;
}
catch ( IOException e )
{
throw new UnderlyingStorageException( e );
}
}

@Override
public final Lock lock()
{
Expand All @@ -127,15 +146,16 @@ public final Lock lock()
boolean success = false;
try
{
RelationshipRecord record = readRecord( relationshipRecord.getId(), FORCE );
// It's safer to re-read the relationship record here, specifically nextProp, after acquiring the lock
if ( !relationshipRecordCursor.next( relationshipRecord.getId(), relationshipRecord, FORCE ) )
if ( !record.inUse() )
{
// So it looks like the node has been deleted. The current behavior of RelationshipStore#fillRecord
// w/ FORCE is to only set the inUse field on loading an unused record. This should (and will)
// change to be more of a centralized behavior by the stores. Anyway, setting this pointer
// to the primitive equivalent of null the property cursor will just look empty from the
// outside and the releasing of the lock will be done as usual.
relationshipRecord.setNextProp( Record.NO_NEXT_PROPERTY.intValue() );
// So it looks like the relationship has been deleted. The current behavior of
// RelationshipStore#fillRecord w/ FORCE is to only set the inUse field on loading an unused record.
// This should (and will) change to be more of a centralized behavior by the stores. Anyway,
// setting this pointer to the primitive equivalent of null the property cursor will just look
// empty from the outside and the releasing of the lock will be done as usual.
record.setNextProp( Record.NO_NEXT_PROPERTY.intValue() );
}
success = true;
}
Expand Down
Expand Up @@ -21,7 +21,7 @@

import org.neo4j.collection.primitive.PrimitiveLongIterator;
import org.neo4j.kernel.impl.locking.LockService;
import org.neo4j.kernel.impl.store.RecordCursors;
import org.neo4j.kernel.impl.store.RelationshipStore;
import org.neo4j.kernel.impl.store.record.RelationshipRecord;
import org.neo4j.kernel.impl.util.InstanceCache;
import org.neo4j.storageengine.api.txstate.ReadableTransactionState;
Expand All @@ -37,11 +37,11 @@ public class StoreIteratorRelationshipCursor extends StoreAbstractIteratorRelati
private PrimitiveLongIterator iterator;
private final InstanceCache<StoreIteratorRelationshipCursor> instanceCache;

StoreIteratorRelationshipCursor( RelationshipRecord relationshipRecord,
InstanceCache<StoreIteratorRelationshipCursor> instanceCache, RecordCursors cursors,
StoreIteratorRelationshipCursor( RelationshipStore relationshipStore,
InstanceCache<StoreIteratorRelationshipCursor> instanceCache,
LockService lockService )
{
super( relationshipRecord, cursors, lockService );
super( relationshipStore, lockService );
this.instanceCache = instanceCache;
}

Expand All @@ -62,7 +62,8 @@ protected boolean doFetchNext()
{
while ( iterator != null && iterator.hasNext() )
{
if ( relationshipRecordCursor.next( iterator.next(), relationshipRecord, CHECK ) )
RelationshipRecord relationshipRecord = readRecord( iterator.next(), CHECK );
if ( relationshipRecord.inUse() )
{
return true;
}
Expand Down
Expand Up @@ -26,6 +26,7 @@
import org.neo4j.kernel.impl.locking.LockService;
import org.neo4j.kernel.impl.store.InvalidRecordException;
import org.neo4j.kernel.impl.store.RecordCursors;
import org.neo4j.kernel.impl.store.RelationshipStore;
import org.neo4j.kernel.impl.store.record.Record;
import org.neo4j.kernel.impl.store.record.RelationshipGroupRecord;
import org.neo4j.kernel.impl.store.record.RelationshipRecord;
Expand Down Expand Up @@ -56,13 +57,13 @@ public class StoreNodeRelationshipCursor extends StoreAbstractIteratorRelationsh
private boolean end;
private final RecordCursors cursors;

public StoreNodeRelationshipCursor( RelationshipRecord relationshipRecord,
public StoreNodeRelationshipCursor( RelationshipStore relationshipStore,
RelationshipGroupRecord groupRecord,
Consumer<StoreNodeRelationshipCursor> instanceCache,
RecordCursors cursors,
LockService lockService )
{
super( relationshipRecord, cursors, lockService );
super( relationshipStore, lockService );
this.groupRecord = groupRecord;
this.instanceCache = instanceCache;
this.cursors = cursors;
Expand Down Expand Up @@ -124,7 +125,7 @@ protected boolean doFetchNext()
{
while ( relationshipId != NO_NEXT_RELATIONSHIP.intValue() )
{
relationshipRecordCursor.next( relationshipId, relationshipRecord, FORCE );
RelationshipRecord relationshipRecord = readRecord( relationshipId, FORCE );

// If we end up on a relationship record that isn't in use there's a good chance there
// have been a concurrent transaction deleting this record under our feet. Since we don't
Expand Down
Expand Up @@ -21,6 +21,7 @@

import org.neo4j.kernel.impl.locking.LockService;
import org.neo4j.kernel.impl.store.RecordCursors;
import org.neo4j.kernel.impl.store.RelationshipStore;
import org.neo4j.kernel.impl.store.record.RelationshipRecord;
import org.neo4j.kernel.impl.util.InstanceCache;
import org.neo4j.storageengine.api.txstate.ReadableTransactionState;
Expand All @@ -39,10 +40,10 @@ public class StoreSingleRelationshipCursor extends StoreAbstractRelationshipCurs
private long relationshipId = NO_SUCH_RELATIONSHIP;
private ReadableTransactionState state;

StoreSingleRelationshipCursor( RelationshipRecord relationshipRecord,
InstanceCache<StoreSingleRelationshipCursor> instanceCache, RecordCursors cursors, LockService lockService )
StoreSingleRelationshipCursor( RelationshipStore relationshipStore,
InstanceCache<StoreSingleRelationshipCursor> instanceCache, LockService lockService )
{
super( relationshipRecord, cursors, lockService );
super( relationshipStore, lockService );
this.instanceCache = instanceCache;
}

Expand Down Expand Up @@ -74,7 +75,7 @@ private boolean isDeletedInTx()
private boolean loadNextRecord()
{
return relationshipId != NO_SUCH_RELATIONSHIP &&
relationshipRecordCursor.next( relationshipId, relationshipRecord, CHECK );
readRecord( relationshipId, CHECK ).inUse();
}

private boolean fetchFromTxState()
Expand Down
Expand Up @@ -97,25 +97,23 @@ protected NodeCursor create()
@Override
protected StoreSingleRelationshipCursor create()
{
return new StoreSingleRelationshipCursor( relationshipStore.newRecord(), this, recordCursors,
lockService );
return new StoreSingleRelationshipCursor( relationshipStore, this, lockService );
}
};
iteratorRelationshipCursor = new InstanceCache<StoreIteratorRelationshipCursor>()
{
@Override
protected StoreIteratorRelationshipCursor create()
{
return new StoreIteratorRelationshipCursor( relationshipStore.newRecord(), this, recordCursors,
lockService );
return new StoreIteratorRelationshipCursor( relationshipStore, this, lockService );
}
};
nodeRelationshipsCursor = new InstanceCache<StoreNodeRelationshipCursor>()
{
@Override
protected StoreNodeRelationshipCursor create()
{
return new StoreNodeRelationshipCursor( relationshipStore.newRecord(),
return new StoreNodeRelationshipCursor( relationshipStore,
relationshipGroupStore.newRecord(), this, recordCursors, lockService );
}
};
Expand Down
Expand Up @@ -32,10 +32,7 @@
import org.neo4j.kernel.impl.store.RelationshipStore;
import org.neo4j.kernel.impl.store.record.NodeRecord;
import org.neo4j.kernel.impl.store.record.RelationshipGroupRecord;
import org.neo4j.kernel.impl.store.record.RelationshipRecord;
import org.neo4j.storageengine.api.Direction;

import static org.neo4j.function.Predicates.ALWAYS_TRUE_INT;
import static org.neo4j.kernel.impl.locking.LockService.NO_LOCK_SERVICE;
import static org.neo4j.kernel.impl.store.record.RecordLoad.NORMAL;
import static org.neo4j.storageengine.api.Direction.BOTH;
Expand All @@ -48,9 +45,8 @@ abstract class BatchRelationshipIterable<T> implements Iterable<T>
{
RelationshipStore relationshipStore = neoStores.getRelationshipStore();
RecordStore<RelationshipGroupRecord> relationshipGroupStore = neoStores.getRelationshipGroupStore();
RelationshipRecord relationshipRecord = relationshipStore.newRecord();
RelationshipGroupRecord relationshipGroupRecord = relationshipGroupStore.newRecord();
this.relationshipCursor = new StoreNodeRelationshipCursor( relationshipRecord, relationshipGroupRecord,
this.relationshipCursor = new StoreNodeRelationshipCursor( relationshipStore, relationshipGroupRecord,
cursor -> {}, cursors, NO_LOCK_SERVICE );

// TODO There's an opportunity to reuse lots of instances created here, but this isn't a
Expand Down

0 comments on commit 468edb6

Please sign in to comment.