From c60ea27eea3522e59fc22e93fdeb393c5bc64e4c Mon Sep 17 00:00:00 2001 From: MishaDemianenko Date: Thu, 25 May 2017 17:16:50 +0200 Subject: [PATCH] Revert "Merge pull request #12 from davidegrohmann/3.3-parallel-node-scan" This reverts commit 015750512d67e14606085ced41d12de7959bbf84, reversing changes made to 9608c179bc86dc8a27fd1f9ad4823fdb4b495087. --- .../kernel/impl/api/state/NodeStateImpl.java | 124 ++++++++-- .../neo4j/kernel/impl/api/state/TxState.java | 12 + .../impl/api/store/AllNodeProgression.java | 60 +---- .../kernel/impl/api/store/NodeCursor.java | 34 +-- .../impl/api/store/NodeProgression.java | 47 +--- .../impl/api/store/SingleNodeProgression.java | 46 +--- .../kernel/impl/api/store/StorageLayer.java | 4 +- .../kernel/impl/api/store/StoreStatement.java | 17 +- .../store/TransactionStateAccessMode.java} | 27 +-- .../storageengine/api/StorageStatement.java | 8 +- .../storageengine/api/txstate/NodeState.java | 120 ---------- .../api/txstate/ReadableTransactionState.java | 2 + .../api/state/SchemaTransactionStateTest.java | 36 ++- .../api/store/AllNodeProgressionTest.java | 162 ------------- .../kernel/impl/api/store/NodeCursorTest.java | 58 ++--- .../impl/api/store/StorageLayerLabelTest.java | 2 +- .../api/store/StorageLayerNodeAndRelTest.java | 2 +- .../api/store/StorageLayerPropertyTest.java | 2 +- .../StorageLayerRelTypesAndDegreeTest.java | 2 +- .../TxStateTransactionDataViewTest.java | 4 +- .../kernel/impl/store/NeoStoresTest.java | 17 +- .../api/store/EnterpriseStoreStatement.java | 30 --- .../api/store/ParallelAllNodeProgression.java | 117 ---------- .../enterprise/PropertyExistenceEnforcer.java | 4 +- .../store/EnterpriseStoreStatementTest.java | 221 ------------------ .../store/ParallelAllNodeProgressionTest.java | 205 ---------------- 26 files changed, 238 insertions(+), 1125 deletions(-) rename community/kernel/src/{test/java/org/neo4j/kernel/impl/api/store/SingleNodeProgressionTest.java => main/java/org/neo4j/kernel/impl/api/store/TransactionStateAccessMode.java} (51%) delete mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/AllNodeProgressionTest.java delete mode 100644 enterprise/kernel/src/main/java/org/neo4j/kernel/impl/api/store/ParallelAllNodeProgression.java delete mode 100644 enterprise/kernel/src/test/java/org/neo4j/kernel/impl/api/store/EnterpriseStoreStatementTest.java delete mode 100644 enterprise/kernel/src/test/java/org/neo4j/kernel/impl/api/store/ParallelAllNodeProgressionTest.java diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/NodeStateImpl.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/NodeStateImpl.java index 11aa065eaa7bc..51d27315d420c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/NodeStateImpl.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/NodeStateImpl.java @@ -38,6 +38,7 @@ import org.neo4j.storageengine.api.txstate.PropertyContainerState; import org.neo4j.storageengine.api.txstate.ReadableDiffSets; +import static java.util.Collections.emptyIterator; import static org.neo4j.collection.primitive.Primitive.intSet; public class NodeStateImpl extends PropertyContainerStateImpl implements NodeState @@ -118,18 +119,6 @@ public void clear() } } - @Override - public PrimitiveIntSet augmentLabels( PrimitiveIntSet labels ) - { - ReadableDiffSets labelDiffSets = labelDiffSets(); - if ( !labelDiffSets.isEmpty() ) - { - labelDiffSets.getRemoved().forEach( labels::remove ); - labelDiffSets.getAdded().forEach( labels::add ); - } - return labels; - } - @Override public int augmentDegree( Direction direction, int degree ) { @@ -254,7 +243,116 @@ final NodeStateImpl createValue( Long id, TxState state ) @Override final NodeState defaultValue() { - return NodeState.EMPTY; + return DEFAULT; } + + private static final NodeState DEFAULT = new NodeState() + { + @Override + public Iterator addedProperties() + { + return emptyIterator(); + } + + @Override + public Iterator changedProperties() + { + return emptyIterator(); + } + + @Override + public Iterator removedProperties() + { + return emptyIterator(); + } + + @Override + public Iterator addedAndChangedProperties() + { + return emptyIterator(); + } + + @Override + public Iterator augmentProperties( Iterator iterator ) + { + return iterator; + } + + @Override + public void accept( PropertyContainerState.Visitor visitor ) throws ConstraintValidationException + { + } + + @Override + public ReadableDiffSets labelDiffSets() + { + return ReadableDiffSets.Empty.instance(); + } + + @Override + public int augmentDegree( Direction direction, int degree ) + { + return degree; + } + + @Override + public int augmentDegree( Direction direction, int degree, int typeId ) + { + return degree; + } + + @Override + public void accept( NodeState.Visitor visitor ) + { + } + + @Override + public PrimitiveIntSet relationshipTypes() + { + return Primitive.intSet(); + } + + @Override + public long getId() + { + throw new UnsupportedOperationException( "id not defined" ); + } + + @Override + public boolean hasChanges() + { + return false; + } + + @Override + public StorageProperty getChangedProperty( int propertyKeyId ) + { + return null; + } + + @Override + public StorageProperty getAddedProperty( int propertyKeyId ) + { + return null; + } + + @Override + public boolean isPropertyRemoved( int propertyKeyId ) + { + return false; + } + + @Override + public PrimitiveLongIterator getAddedRelationships( Direction direction ) + { + return null; + } + + @Override + public PrimitiveLongIterator getAddedRelationships( Direction direction, int[] relTypes ) + { + return null; + } + }; } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java index 200043ef91ce5..64e04721c1a3e 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java @@ -642,6 +642,18 @@ public RelationshipState getRelationshipState( long id ) return RELATIONSHIP_STATE.get( this, id ); } + @Override + public PrimitiveIntSet augmentLabels( PrimitiveIntSet labels, NodeState nodeState ) + { + ReadableDiffSets labelDiffSets = nodeState.labelDiffSets(); + if ( !labelDiffSets.isEmpty() ) + { + labelDiffSets.getRemoved().forEach( labels::remove ); + labelDiffSets.getAdded().forEach( labels::add ); + } + return labels; + } + @Override public ReadableDiffSets nodesWithLabelChanged( int labelId ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllNodeProgression.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllNodeProgression.java index 083d10ee43acc..b1a278127f8b6 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllNodeProgression.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllNodeProgression.java @@ -19,71 +19,31 @@ */ package org.neo4j.kernel.impl.api.store; -import java.util.Iterator; - +import org.neo4j.kernel.api.StatementConstants; import org.neo4j.kernel.impl.store.NodeStore; -import org.neo4j.storageengine.api.txstate.NodeState; -import org.neo4j.storageengine.api.txstate.ReadableTransactionState; public class AllNodeProgression implements NodeProgression { - private final NodeStore nodeStore; - private final ReadableTransactionState state; - - private long start; - private boolean done; + private final AllIdIterator allIdIterator; - AllNodeProgression( NodeStore nodeStore, ReadableTransactionState state ) + AllNodeProgression( NodeStore nodeStore ) { - this.nodeStore = nodeStore; - this.state = state; - this.start = nodeStore.getNumberOfReservedLowIds(); + allIdIterator = new AllIdIterator( nodeStore ); } @Override - public boolean nextBatch( Batch batch ) + public long nextId() { - while ( true ) + if ( allIdIterator.hasNext() ) { - if ( done ) - { - batch.nothing(); - return false; - } - - long highId = nodeStore.getHighestPossibleIdInUse(); - if ( start <= highId ) - { - batch.init( start, highId ); - start = highId + 1; - return true; - } - - done = true; + return allIdIterator.next(); } + return StatementConstants.NO_SUCH_NODE; } @Override - public Iterator addedNodes() - { - return state == null ? null : state.addedAndRemovedNodes().getAdded().iterator(); - } - - @Override - public boolean fetchFromTxState( long id ) - { - return false; - } - - @Override - public boolean fetchFromDisk( long id ) - { - return state == null || !state.nodeIsDeletedInThisTx( id ); - } - - @Override - public NodeState nodeState( long id ) + public TransactionStateAccessMode mode() { - return state == null ? NodeState.EMPTY : state.getNodeState( id ); + return TransactionStateAccessMode.APPEND; } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeCursor.java index 48f62b9f5059f..8c0b5a5f51e89 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeCursor.java @@ -35,8 +35,11 @@ import org.neo4j.kernel.impl.util.IoPrimitiveUtils; import org.neo4j.storageengine.api.NodeItem; import org.neo4j.storageengine.api.txstate.NodeState; +import org.neo4j.storageengine.api.txstate.ReadableTransactionState; import static org.neo4j.collection.primitive.PrimitiveIntCollections.asSet; +import static org.neo4j.kernel.impl.api.store.TransactionStateAccessMode.APPEND; +import static org.neo4j.kernel.impl.api.store.TransactionStateAccessMode.FETCH; import static org.neo4j.kernel.impl.locking.LockService.NO_LOCK; import static org.neo4j.kernel.impl.locking.LockService.NO_LOCK_SERVICE; import static org.neo4j.kernel.impl.store.record.RecordLoad.CHECK; @@ -44,7 +47,6 @@ public class NodeCursor implements NodeItem, Cursor, Disposable { - private final NodeProgression.Batch batch = new NodeProgression.Batch(); private final NodeRecord nodeRecord; private final Consumer instanceCache; private final NodeStore nodeStore; @@ -52,6 +54,7 @@ public class NodeCursor implements NodeItem, Cursor, Disposable private final PageCursor pageCursor; private NodeProgression progression; + private ReadableTransactionState state; private boolean fetched; private long[] labels; private Iterator added; @@ -65,10 +68,13 @@ public class NodeCursor implements NodeItem, Cursor, Disposable this.lockService = lockService; } - public Cursor init( NodeProgression progression ) + public Cursor init( NodeProgression progression, ReadableTransactionState state ) { this.progression = progression; - this.added = progression.addedNodes(); + this.state = state; + this.added = state != null && progression.mode() == APPEND + ? state.addedAndRemovedNodes().getAdded().iterator() + : null; return this; } @@ -81,16 +87,18 @@ public boolean next() private boolean fetchNext() { labels = null; - while ( progression != null && (batch.hasNext() || progression.nextBatch( batch ) ) ) + long id; + while ( progression != null && (id = progression.nextId()) >= 0 ) { - long id = batch.next(); - if ( progression.fetchFromTxState( id ) ) + + if ( state != null && progression.mode() == FETCH && state.nodeIsAddedInThisTx( id ) ) { recordFromTxState( id ); return true; } - if ( progression.fetchFromDisk( id ) && nodeStore.readRecord( id, nodeRecord, CHECK, pageCursor ).inUse() ) + if ( (state == null || !state.nodeIsDeletedInThisTx( id )) && + nodeStore.readRecord( id, nodeRecord, CHECK, pageCursor ).inUse() ) { return true; } @@ -117,8 +125,8 @@ public void close() fetched = false; labels = null; added = null; + state = null; progression = null; - batch.nothing(); instanceCache.accept( this ); } @@ -143,19 +151,19 @@ public NodeItem get() public PrimitiveIntSet labels() { PrimitiveIntSet labels = asSet( loadedLabels(), IoPrimitiveUtils::safeCastLongToInt ); - return progression.nodeState( id() ).augmentLabels( labels ); + return state != null ? state.augmentLabels( labels, state.getNodeState( id() ) ) : labels; } @Override public boolean hasLabel( int labelId ) { - NodeState nodeState = progression.nodeState( id() ); - if ( nodeState.labelDiffSets().getRemoved().contains( labelId ) ) + NodeState nodeState = state == null ? null : state.getNodeState( id() ); + if ( state != null && nodeState.labelDiffSets().getRemoved().contains( labelId ) ) { return false; } - if ( nodeState.labelDiffSets().getAdded().contains( labelId ) ) + if ( state != null && nodeState.labelDiffSets().getAdded().contains( labelId ) ) { return true; } @@ -213,7 +221,7 @@ public long nextPropertyId() @Override public Lock lock() { - return progression.fetchFromTxState( id() ) ? NO_LOCK : acquireLock(); + return state != null && state.nodeIsAddedInThisTx( id() ) ? NO_LOCK : acquireLock(); } private Lock acquireLock() diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeProgression.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeProgression.java index 993b14d56becb..f6bacd0e5a127 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeProgression.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeProgression.java @@ -19,53 +19,10 @@ */ package org.neo4j.kernel.impl.api.store; -import java.util.Iterator; - -import org.neo4j.collection.primitive.PrimitiveLongIterator; -import org.neo4j.storageengine.api.txstate.NodeState; - public interface NodeProgression { - boolean nextBatch( Batch batch ); - - Iterator addedNodes(); - - boolean fetchFromTxState( long id ); - - boolean fetchFromDisk( long id ); - - NodeState nodeState( long id ); - - class Batch implements PrimitiveLongIterator - { - private long first; - private long last; - - { - nothing(); - } - - public void init( long first, long last ) - { - this.first = first; - this.last = last; - } - - public void nothing() - { - init( -1, -2 ); - } - @Override - public boolean hasNext() - { - return first <= last; - } + long nextId(); - @Override - public long next() - { - return first++; - } - } + TransactionStateAccessMode mode(); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SingleNodeProgression.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SingleNodeProgression.java index 91a09664271cf..a26ad983b9d29 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SingleNodeProgression.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SingleNodeProgression.java @@ -19,61 +19,33 @@ */ package org.neo4j.kernel.impl.api.store; -import java.util.Iterator; - -import org.neo4j.storageengine.api.txstate.NodeState; -import org.neo4j.storageengine.api.txstate.ReadableTransactionState; - -import static org.neo4j.kernel.api.StatementConstants.NO_SUCH_NODE; +import org.neo4j.kernel.api.StatementConstants; public class SingleNodeProgression implements NodeProgression { - private final ReadableTransactionState state; private long nodeId; - public SingleNodeProgression( long nodeId, ReadableTransactionState state ) + SingleNodeProgression( long nodeId ) { - this.state = state; this.nodeId = nodeId; } @Override - public boolean nextBatch( Batch batch ) + public long nextId() { - if ( nodeId != NO_SUCH_NODE ) + try { - batch.init( nodeId, nodeId ); - nodeId = NO_SUCH_NODE; - return true; + return nodeId; } - else + finally { - batch.nothing(); - return false; + nodeId = StatementConstants.NO_SUCH_NODE; } } @Override - public Iterator addedNodes() - { - return null; - } - - @Override - public boolean fetchFromTxState( long id ) - { - return state != null && state.nodeIsAddedInThisTx( id ); - } - - @Override - public boolean fetchFromDisk( long id ) - { - return state == null || !state.nodeIsDeletedInThisTx( id ); - } - - @Override - public NodeState nodeState( long id ) + public TransactionStateAccessMode mode() { - return state == null ? NodeState.EMPTY : state.getNodeState( id ); + return TransactionStateAccessMode.FETCH; } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorageLayer.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorageLayer.java index 9cfb6b8e00c72..afb00a75fe0de 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorageLayer.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorageLayer.java @@ -396,13 +396,13 @@ public RelationshipIterator relationshipsGetAll() @Override public Cursor nodeGetAllCursor( StorageStatement statement, TransactionState state ) { - return statement.acquireNodeCursor( new AllNodeProgression( nodeStore, state ) ); + return statement.acquireNodeCursor( state ); } @Override public Cursor nodeCursor( StorageStatement statement, long nodeId, ReadableTransactionState state ) { - return statement.acquireNodeCursor( new SingleNodeProgression( nodeId, state ) ); + return statement.acquireSingleNodeCursor( nodeId, state ); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreStatement.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreStatement.java index 468be8c09dad3..4dcb14f31a5fe 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreStatement.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreStatement.java @@ -49,7 +49,7 @@ */ public class StoreStatement implements StorageStatement { - protected final InstanceCache nodeCursor; + private final InstanceCache nodeCursor; private final InstanceCache singleRelationshipCursor; private final InstanceCache iteratorRelationshipCursor; private final InstanceCache nodeRelationshipsCursor; @@ -151,22 +151,17 @@ public void acquire() } @Override - public NodeProgression parallelNodeScanProgression( ReadableTransactionState state ) + public Cursor acquireNodeCursor( ReadableTransactionState state ) { - throw unsupportedOperation(); - } - - private UnsupportedOperationException unsupportedOperation() - { - return new UnsupportedOperationException( "This operation is not supported in community edition but only in " + - "enterprise edition" ); + neoStores.assertOpen(); + return nodeCursor.get().init( new AllNodeProgression( neoStores.getNodeStore() ), state ); } @Override - public Cursor acquireNodeCursor( NodeProgression nodeProgression ) + public Cursor acquireSingleNodeCursor( long nodeId, ReadableTransactionState state ) { neoStores.assertOpen(); - return nodeCursor.get().init( nodeProgression ); + return nodeCursor.get().init( new SingleNodeProgression( nodeId ), state ); } @Override diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/SingleNodeProgressionTest.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/TransactionStateAccessMode.java similarity index 51% rename from community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/SingleNodeProgressionTest.java rename to community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/TransactionStateAccessMode.java index 62bfa10ffe653..035df7c23d890 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/SingleNodeProgressionTest.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/TransactionStateAccessMode.java @@ -19,29 +19,8 @@ */ package org.neo4j.kernel.impl.api.store; -import org.junit.Test; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -public class SingleNodeProgressionTest +public enum TransactionStateAccessMode { - @Test - public void shouldReturnOnlyTheGivenNodeId() throws Throwable - { - // given - long nodeId = 42L; - SingleNodeProgression progression = new SingleNodeProgression( nodeId, null ); - NodeProgression.Batch batch = new NodeProgression.Batch(); - - // when / then - assertTrue( progression.nextBatch( batch ) ); - assertTrue( batch.hasNext() ); - assertEquals( nodeId, batch.next() ); - - assertFalse( batch.hasNext() ); - assertFalse( progression.nextBatch( batch ) ); - assertFalse( batch.hasNext() ); - } + APPEND, + FETCH } diff --git a/community/kernel/src/main/java/org/neo4j/storageengine/api/StorageStatement.java b/community/kernel/src/main/java/org/neo4j/storageengine/api/StorageStatement.java index 8720fd5e953e2..6b0ebfef15ca4 100644 --- a/community/kernel/src/main/java/org/neo4j/storageengine/api/StorageStatement.java +++ b/community/kernel/src/main/java/org/neo4j/storageengine/api/StorageStatement.java @@ -23,7 +23,6 @@ import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException; import org.neo4j.kernel.api.schema.index.IndexDescriptor; import org.neo4j.kernel.impl.api.store.NodeDegreeCounter; -import org.neo4j.kernel.impl.api.store.NodeProgression; import org.neo4j.kernel.impl.locking.Lock; import org.neo4j.storageengine.api.schema.IndexReader; import org.neo4j.storageengine.api.schema.LabelScanReader; @@ -66,17 +65,18 @@ public interface StorageStatement extends AutoCloseable @Override void close(); - NodeProgression parallelNodeScanProgression( ReadableTransactionState state ); + Cursor acquireNodeCursor( ReadableTransactionState state ); /** * Acquires {@link Cursor} capable of {@link Cursor#get() serving} {@link NodeItem} for selected nodes. * No node is selected when this method returns, a call to {@link Cursor#next()} will have to be made * to place the cursor over the first item and then more calls to move the cursor through the selection. * - * @param nodeProgression the progression of the selected nodes to be fetched + * @param nodeId id of node to get cursor for. + * @param state the transaction state or null if there are no changes. * @return a {@link Cursor} over {@link NodeItem} for the given {@code nodeId}. */ - Cursor acquireNodeCursor( NodeProgression nodeProgression ); + Cursor acquireSingleNodeCursor( long nodeId, ReadableTransactionState state ); /** * Acquires {@link Cursor} capable of {@link Cursor#get() serving} {@link RelationshipItem} for selected diff --git a/community/kernel/src/main/java/org/neo4j/storageengine/api/txstate/NodeState.java b/community/kernel/src/main/java/org/neo4j/storageengine/api/txstate/NodeState.java index b380353e8db07..5f045309d082d 100644 --- a/community/kernel/src/main/java/org/neo4j/storageengine/api/txstate/NodeState.java +++ b/community/kernel/src/main/java/org/neo4j/storageengine/api/txstate/NodeState.java @@ -19,15 +19,12 @@ */ package org.neo4j.storageengine.api.txstate; -import java.util.Iterator; import java.util.Set; -import org.neo4j.collection.primitive.Primitive; import org.neo4j.collection.primitive.PrimitiveIntSet; import org.neo4j.collection.primitive.PrimitiveLongIterator; import org.neo4j.kernel.api.exceptions.schema.ConstraintValidationException; import org.neo4j.storageengine.api.Direction; -import org.neo4j.storageengine.api.StorageProperty; import static java.util.Collections.emptyIterator; @@ -50,8 +47,6 @@ void visitLabelChanges( long nodeId, Set added, Set removed ) ReadableDiffSets labelDiffSets(); - PrimitiveIntSet augmentLabels( PrimitiveIntSet labels ); - int augmentDegree( Direction direction, int degree ); int augmentDegree( Direction direction, int degree, int typeId ); @@ -66,119 +61,4 @@ void visitLabelChanges( long nodeId, Set added, Set removed ) PrimitiveLongIterator getAddedRelationships( Direction direction, int[] relTypes ); - NodeState EMPTY = new NodeState() - { - @Override - public Iterator addedProperties() - { - return emptyIterator(); - } - - @Override - public Iterator changedProperties() - { - return emptyIterator(); - } - - @Override - public Iterator removedProperties() - { - return emptyIterator(); - } - - @Override - public Iterator addedAndChangedProperties() - { - return emptyIterator(); - } - - @Override - public Iterator augmentProperties( Iterator iterator ) - { - return iterator; - } - - @Override - public void accept( PropertyContainerState.Visitor visitor ) throws ConstraintValidationException - { - } - - @Override - public ReadableDiffSets labelDiffSets() - { - return ReadableDiffSets.Empty.instance(); - } - - @Override - public PrimitiveIntSet augmentLabels( PrimitiveIntSet labels ) - { - return labels; - } - - @Override - public int augmentDegree( Direction direction, int degree ) - { - return degree; - } - - @Override - public int augmentDegree( Direction direction, int degree, int typeId ) - { - return degree; - } - - @Override - public void accept( NodeState.Visitor visitor ) - { - } - - @Override - public PrimitiveIntSet relationshipTypes() - { - return Primitive.intSet(); - } - - @Override - public long getId() - { - throw new UnsupportedOperationException( "id not defined" ); - } - - @Override - public boolean hasChanges() - { - return false; - } - - @Override - public StorageProperty getChangedProperty( int propertyKeyId ) - { - return null; - } - - @Override - public StorageProperty getAddedProperty( int propertyKeyId ) - { - return null; - } - - @Override - public boolean isPropertyRemoved( int propertyKeyId ) - { - return false; - } - - @Override - public PrimitiveLongIterator getAddedRelationships( Direction direction ) - { - return null; - } - - @Override - public PrimitiveLongIterator getAddedRelationships( Direction direction, int[] relTypes ) - { - return null; - } - }; - } diff --git a/community/kernel/src/main/java/org/neo4j/storageengine/api/txstate/ReadableTransactionState.java b/community/kernel/src/main/java/org/neo4j/storageengine/api/txstate/ReadableTransactionState.java index f693deaa7773e..34d05d9a650d6 100644 --- a/community/kernel/src/main/java/org/neo4j/storageengine/api/txstate/ReadableTransactionState.java +++ b/community/kernel/src/main/java/org/neo4j/storageengine/api/txstate/ReadableTransactionState.java @@ -130,6 +130,8 @@ ReadableDiffSets indexUpdatesForRangeSeekByString( IndexDescriptor index, RelationshipState getRelationshipState( long id ); + PrimitiveIntSet augmentLabels( PrimitiveIntSet labels, NodeState nodeState ); + /** * The way tokens are created is that the first time a token is needed it gets created in its own little * token mini-transaction, separate from the surrounding transaction that creates or modifies data that need it. diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/SchemaTransactionStateTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/SchemaTransactionStateTest.java index 49d340620ea4b..254a6fb936738 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/SchemaTransactionStateTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/SchemaTransactionStateTest.java @@ -21,9 +21,12 @@ import org.junit.Before; import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -39,12 +42,10 @@ import org.neo4j.kernel.impl.api.StateHandlingStatementOperations; import org.neo4j.kernel.impl.api.StatementOperationsTestHelper; import org.neo4j.kernel.impl.api.legacyindex.InternalAutoIndexing; -import org.neo4j.kernel.impl.api.store.SingleNodeProgression; import org.neo4j.kernel.impl.api.store.StoreStatement; import org.neo4j.kernel.impl.index.LegacyIndexStore; import org.neo4j.storageengine.api.StoreReadLayer; -import static java.util.Collections.emptyIterator; import static java.util.Collections.emptySet; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -242,17 +243,29 @@ public void before() throws Exception state = StatementOperationsTestHelper.mockedState( txState ); store = mock( StoreReadLayer.class ); - when( store.indexesGetForLabel( labelId1 ) ).then( invocation -> emptyIterator() ); - when( store.indexesGetForLabel( labelId2 ) ).then( invocation -> emptyIterator() ); - when( store.indexesGetAll() ).then( invocation -> emptyIterator() ); + when( store.indexesGetForLabel( labelId1 ) ).then( asAnswer( Collections.emptyList() ) ); + when( store.indexesGetForLabel( labelId2 ) ).then( asAnswer( Collections.emptyList() ) ); + when( store.indexesGetAll() ).then( asAnswer( Collections.emptyList() ) ); txContext = new StateHandlingStatementOperations( store, mock( InternalAutoIndexing.class ), mock( ConstraintIndexCreator.class ), mock( LegacyIndexStore.class ) ); - storeStatement = mock( StoreStatement.class ); + storeStatement = mock(StoreStatement.class); when( state.getStoreStatement() ).thenReturn( storeStatement ); } + private static Answer> asAnswer( final Iterable values ) + { + return new Answer>() + { + @Override + public Iterator answer( InvocationOnMock invocation ) throws Throwable + { + return values.iterator(); + } + }; + } + private static class Labels { private final long nodeId; @@ -275,13 +288,18 @@ private void commitLabels( Labels... labels ) throws Exception Map> allLabels = new HashMap<>(); for ( Labels nodeLabels : labels ) { - when( storeStatement.acquireNodeCursor( new SingleNodeProgression( nodeLabels.nodeId, null ) ) ) + when( storeStatement.acquireSingleNodeCursor( nodeLabels.nodeId, null ) ) .thenReturn( asNodeCursor( nodeLabels.nodeId, StubCursors.labels( nodeLabels.labelIds ) ) ); for ( int label : nodeLabels.labelIds ) { - Collection nodes = allLabels.computeIfAbsent( label, k -> new ArrayList<>() ); + Collection nodes = allLabels.get( label ); + if ( nodes == null ) + { + nodes = new ArrayList<>(); + allLabels.put( label, nodes ); + } nodes.add( nodeLabels.nodeId ); } } @@ -289,7 +307,7 @@ private void commitLabels( Labels... labels ) throws Exception for ( Map.Entry> entry : allLabels.entrySet() ) { when( store.nodesGetForLabel( state.getStoreStatement(), entry.getKey() ) ) - .then( invocation -> entry.getValue().iterator() ); + .then( asAnswer( entry.getValue() ) ); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/AllNodeProgressionTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/AllNodeProgressionTest.java deleted file mode 100644 index cadf339d5bff3..0000000000000 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/AllNodeProgressionTest.java +++ /dev/null @@ -1,162 +0,0 @@ -/* - * Copyright (c) 2002-2017 "Neo Technology," - * Network Engine for Objects in Lund AB [http://neotechnology.com] - * - * This file is part of Neo4j. - * - * Neo4j is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ -package org.neo4j.kernel.impl.api.store; - -import org.junit.Test; - -import org.neo4j.kernel.impl.api.state.TxState; -import org.neo4j.kernel.impl.store.NodeStore; - -import static java.util.Collections.singletonList; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.neo4j.helpers.collection.Iterators.asList; - -public class AllNodeProgressionTest -{ - private final int start = 1; - private final long end = 42L; - private final NodeStore nodeStore = mock( NodeStore.class ); - private final NodeProgression.Batch batch = new NodeProgression.Batch(); - - { - when( nodeStore.getNumberOfReservedLowIds() ).thenReturn( start ); - when( nodeStore.getHighestPossibleIdInUse() ).thenReturn( end ); - batch.nothing(); - } - - @Test - public void shouldReturnABatchFromLowReservedIdsToHighIdPossibleInUse() throws Throwable - { - // given - AllNodeProgression progression = new AllNodeProgression( nodeStore, null ); - - // when - boolean hasNext = progression.nextBatch( batch ); - - // then - assertTrue( hasNext ); - checkBatch( start, end, progression ); - - assertNoMoreValueInBatchAndProgression( progression ); - } - - @Test - public void shouldCheckIfTheHighIdHasChangedAndIssueAnExtraBatchWithTheRemainingElements() throws Throwable - { - // given - AllNodeProgression progression = new AllNodeProgression( nodeStore, null ); - assertTrue( progression.nextBatch( batch ) ); - checkBatch( start, end, progression ); - - // when / then - long movedEnd = end + 10; - when( nodeStore.getHighestPossibleIdInUse() ).thenReturn( movedEnd ); - assertTrue( progression.nextBatch( batch ) ); - checkBatch( end + 1, movedEnd, progression ); - - assertNoMoreValueInBatchAndProgression( progression ); - } - - @Test - public void shouldNeverReturnNewBatchesIfTheProgressionHasReturnFalseToSignalTermination() throws Throwable - { - // given - AllNodeProgression progression = new AllNodeProgression( nodeStore, null ); - assertTrue( progression.nextBatch( batch ) ); - checkBatch( start, end, progression ); - - assertNoMoreValueInBatchAndProgression( progression ); - - // when / then - long movedEnd = end + 10; - when( nodeStore.getHighestPossibleIdInUse() ).thenReturn( movedEnd ); - assertNoMoreValueInBatchAndProgression( progression ); - } - - @Test - public void shouldReturnNoAddedNodesIfNoTransactionStateIsGiven() throws Throwable - { - AllNodeProgression progression = new AllNodeProgression( nodeStore, null ); - assertNull( progression.addedNodes() ); - } - - @Test - public void shouldReturnAddedNodesFromTheTxState() throws Throwable - { - TxState txState = new TxState(); - long id = 42; - txState.nodeDoCreate( id ); - AllNodeProgression progression = new AllNodeProgression( nodeStore, txState ); - assertEquals( singletonList( id ), asList( progression.addedNodes() ) ); - } - - @Test - public void shouldMarkTheDeletedNodesAsNonFetchableFromDisk() throws Throwable - { - TxState txState = new TxState(); - for ( long i = start; i <= end; i++ ) - { - if ( i % 3 == 0 ) - { - txState.nodeDoDelete( i ); - } - } - - // given - AllNodeProgression progression = new AllNodeProgression( nodeStore, txState ); - - // when - boolean hasNext = progression.nextBatch( batch ); - - // then - assertTrue( hasNext ); - - for ( long i = start; i <= end; i++ ) - { - assertTrue( batch.hasNext() ); - assertEquals( i, batch.next() ); - assertEquals( i % 3 != 0, progression.fetchFromDisk( i ) ); - } - - assertNoMoreValueInBatchAndProgression( progression ); - } - - private void checkBatch( long start, long end, NodeProgression progression ) - { - for ( long i = start; i <= end; i++ ) - { - assertTrue( batch.hasNext() ); - assertEquals( i, batch.next() ); - assertTrue( progression.fetchFromDisk( i ) ); - } - } - - private void assertNoMoreValueInBatchAndProgression( AllNodeProgression progression ) - { - assertFalse( batch.hasNext() ); - assertFalse( progression.nextBatch( batch ) ); - assertFalse( batch.hasNext() ); - } -} diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/NodeCursorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/NodeCursorTest.java index 583d70a275e05..c5b0039827d0a 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/NodeCursorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/NodeCursorTest.java @@ -27,8 +27,8 @@ import org.junit.experimental.theories.Theory; import org.junit.runner.RunWith; +import java.io.IOException; import java.util.Arrays; -import java.util.Iterator; import java.util.Set; import java.util.function.LongFunction; @@ -43,7 +43,6 @@ import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.kernel.impl.util.IoPrimitiveUtils; import org.neo4j.storageengine.api.NodeItem; -import org.neo4j.storageengine.api.txstate.NodeState; import org.neo4j.storageengine.api.txstate.ReadableTransactionState; import org.neo4j.test.rule.RepeatRule; @@ -51,13 +50,14 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.neo4j.collection.primitive.PrimitiveIntCollections.asArray; import static org.neo4j.collection.primitive.PrimitiveIntCollections.asSet; -import static org.neo4j.kernel.impl.api.store.NodeCursorTest.Mode.APPEND; -import static org.neo4j.kernel.impl.api.store.NodeCursorTest.Mode.FETCH; +import static org.neo4j.kernel.impl.api.store.TransactionStateAccessMode.APPEND; +import static org.neo4j.kernel.impl.api.store.TransactionStateAccessMode.FETCH; import static org.neo4j.kernel.impl.locking.LockService.NO_LOCK_SERVICE; import static org.neo4j.kernel.impl.store.record.RecordLoad.CHECK; import static org.neo4j.kernel.impl.transaction.state.NodeLabelsFieldTest.inlinedLabelsLongRepresentation; @@ -65,12 +65,6 @@ @RunWith( Theories.class ) public class NodeCursorTest { - enum Mode - { - APPEND, - FETCH - } - /* * each test is gonna run twice to make sure we reuse cursor correctly */ @@ -228,7 +222,7 @@ public void shouldCallTheConsumerOnClose() { MutableBoolean called = new MutableBoolean(); NodeCursor cursor = new NodeCursor( nodeStore, c -> called.setTrue(), NO_LOCK_SERVICE ); - cursor.init( mock( NodeProgression.class ) ); + cursor.init( mock( NodeProgression.class ), mock( ReadableTransactionState.class ) ); assertFalse( called.booleanValue() ); cursor.close(); @@ -239,7 +233,7 @@ public void shouldCallTheConsumerOnClose() public void shouldCloseThePageCursorWhenDisposed() { NodeCursor cursor = new NodeCursor( nodeStore, c -> {}, NO_LOCK_SERVICE ); - cursor.init( mock( NodeProgression.class ) ); + cursor.init( mock( NodeProgression.class ), mock( ReadableTransactionState.class ) ); cursor.close(); cursor.dispose(); @@ -457,7 +451,7 @@ public long id() public PrimitiveIntSet labels() { PrimitiveIntSet labels = asSet( labelIds, IoPrimitiveUtils::safeCastLongToInt ); - return state == null ? labels : state.getNodeState( id ).augmentLabels( labels ); + return state == null ? labels : state.augmentLabels( labels, state.getNodeState( id ) ); } @Override @@ -512,12 +506,12 @@ private interface Operation private static class TestRun { - private final Mode mode; + private final TransactionStateAccessMode mode; private final Operation[] ops; private TxState state; - private TestRun( Mode mode, Operation[] ops ) + private TestRun( TransactionStateAccessMode mode, Operation[] ops ) { this.mode = mode; this.ops = ops; @@ -530,53 +524,33 @@ Cursor initialize( NodeCursor cursor, NodeStore nodeStore, PageCursor { state = op.prepare( nodeStore, pageCursor, nodeRecord, state ); } - return cursor.init( createProgression( ops, mode, state ) ); + return cursor.init( createProgression( ops, mode ), state ); } - private NodeProgression createProgression( Operation[] ops, Mode mode, TxState state ) + private NodeProgression createProgression( Operation[] ops, TransactionStateAccessMode mode ) { return new NodeProgression() { private int i; @Override - public boolean nextBatch( Batch batch ) + public long nextId() { while ( i < ops.length ) { Operation op = ops[i++]; if ( op.fromDisk() || mode == FETCH ) { - batch.init( op.id(), op.id() ); - return true; + return op.id(); } } - batch.nothing(); - return false; - } - - @Override - public Iterator addedNodes() - { - return mode == APPEND && state != null ? state.addedAndRemovedNodes().getAdded().iterator() : null; - } - - @Override - public boolean fetchFromTxState( long id ) - { - return mode == FETCH && state != null && state.nodeIsAddedInThisTx( id ); - } - - @Override - public boolean fetchFromDisk( long id ) - { - return state == null || !state.nodeIsDeletedInThisTx( id ); + return -1L; } @Override - public NodeState nodeState( long id ) + public TransactionStateAccessMode mode() { - return state == null ? NodeState.EMPTY : state.getNodeState( id ); + return mode; } }; } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerLabelTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerLabelTest.java index f5f787ee7f691..c09a8b8e9d26b 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerLabelTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerLabelTest.java @@ -57,7 +57,7 @@ public void should_be_able_to_list_labels_for_node() throws Exception } // THEN - disk.newStatement().acquireNodeCursor( new SingleNodeProgression( nodeId, null ) ).forAll( + disk.newStatement().acquireSingleNodeCursor( nodeId, null ).forAll( node -> assertEquals( PrimitiveIntCollections.asSet( new int[]{labelId1, labelId2} ), node.labels() ) ); } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerNodeAndRelTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerNodeAndRelTest.java index d91e882f0e99e..a36d2c2ca92e1 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerNodeAndRelTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerNodeAndRelTest.java @@ -91,7 +91,7 @@ private boolean nodeExists( long id ) { try ( StorageStatement statement = disk.newStatement() ) { - try ( Cursor node = statement.acquireNodeCursor( new SingleNodeProgression( id, null ) ) ) + try ( Cursor node = statement.acquireSingleNodeCursor( id, null ) ) { return node.next(); } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerPropertyTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerPropertyTest.java index 8bb5ca54a39c1..7320dae94447b 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerPropertyTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerPropertyTest.java @@ -96,7 +96,7 @@ public void should_get_all_node_properties() throws Exception long nodeId = createLabeledNode( db, singletonMap( "prop", value ), label1 ).getId(); // when - try ( Cursor node = statement.acquireNodeCursor( new SingleNodeProgression( nodeId, null ) ) ) + try ( Cursor node = statement.acquireSingleNodeCursor( nodeId, null ) ) { node.next(); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerRelTypesAndDegreeTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerRelTypesAndDegreeTest.java index 9bb56671e14da..197ff0e73cc18 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerRelTypesAndDegreeTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorageLayerRelTypesAndDegreeTest.java @@ -471,7 +471,7 @@ private NodeCursor newCursor( long nodeId ) { NodeCursor cursor = new NodeCursor( resolveNeoStores().getNodeStore(), mock( Consumer.class ), NO_LOCK_SERVICE ); - cursor.init( new SingleNodeProgression( nodeId, null ) ); + cursor.init( new SingleNodeProgression( nodeId ), null ); assertTrue( cursor.next() ); return cursor; } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java index 81840cca6636f..4aa7321440600 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java @@ -40,7 +40,6 @@ import org.neo4j.kernel.api.txstate.TransactionState; import org.neo4j.kernel.impl.api.KernelTransactionImplementation; import org.neo4j.kernel.impl.api.state.TxState; -import org.neo4j.kernel.impl.api.store.NodeProgression; import org.neo4j.kernel.impl.api.store.StoreStatement; import org.neo4j.kernel.impl.core.NodeProxy; import org.neo4j.kernel.impl.core.RelationshipProxy; @@ -297,7 +296,8 @@ public void shouldListAddedLabels() throws Exception // Given state.nodeDoAddLabel( 2, 1L ); when( ops.labelGetName( 2 ) ).thenReturn( "theLabel" ); - when( storeStatement.acquireNodeCursor( any( NodeProgression.class ) ) ).thenReturn( asNodeCursor( 1 ) ); + when( storeStatement.acquireSingleNodeCursor( eq( 1 ), any( ReadableTransactionState.class ) ) ) + .thenReturn( asNodeCursor( 1 ) ); // When Iterable labelEntries = snapshot().assignedLabels(); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java index 8f8fc2c8ddf74..6d054504ea91e 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java @@ -58,7 +58,6 @@ import org.neo4j.kernel.impl.api.KernelStatement; import org.neo4j.kernel.impl.api.KernelTransactionImplementation; import org.neo4j.kernel.impl.api.RelationshipVisitor; -import org.neo4j.kernel.impl.api.store.SingleNodeProgression; import org.neo4j.kernel.impl.core.RelationshipTypeToken; import org.neo4j.kernel.impl.storageengine.impl.recordstorage.RecordStorageEngine; import org.neo4j.kernel.impl.store.MetaDataStore.Position; @@ -296,7 +295,7 @@ private DefinedProperty nodeAddProperty( long nodeId, int key, Object value ) DefinedProperty property = Property.property( key, value ); DefinedProperty oldProperty = NO_SUCH_PROPERTY; try ( StorageStatement statement = storeLayer.newStatement(); - Cursor cursor = statement.acquireNodeCursor( new SingleNodeProgression( nodeId, null ) ) ) + Cursor cursor = statement.acquireSingleNodeCursor( nodeId, null ) ) { if ( cursor.next() ) { @@ -1021,8 +1020,7 @@ private int validateAndCountRelationships( long node, long rel1, long rel2, int { int count = 0; try ( KernelStatement statement = (KernelStatement) tx.acquireStatement(); - Cursor nodeCursor = statement.getStoreStatement() - .acquireNodeCursor( new SingleNodeProgression( node, null ) ) ) + Cursor nodeCursor = statement.getStoreStatement().acquireSingleNodeCursor( node, null ) ) { nodeCursor.next(); @@ -1106,8 +1104,7 @@ else if ( data.propertyKeyId() == prop3.propertyKeyId() ) count = 0; try ( KernelStatement statement = (KernelStatement) tx.acquireStatement(); - Cursor nodeCursor = statement.getStoreStatement() - .acquireNodeCursor( new SingleNodeProgression( node, null ) ) ) + Cursor nodeCursor = statement.getStoreStatement().acquireSingleNodeCursor( node, null ) ) { nodeCursor.next(); NodeItem nodeItem = nodeCursor.get(); @@ -1143,7 +1140,7 @@ private boolean nodeExists( long nodeId ) { try ( StorageStatement statement = storeLayer.newStatement() ) { - try ( Cursor node = statement.acquireNodeCursor( new SingleNodeProgression( nodeId, null ) ) ) + try ( Cursor node = statement.acquireSingleNodeCursor( nodeId, null ) ) { return node.next(); } @@ -1400,8 +1397,7 @@ else if ( data.propertyKeyId() == prop3.propertyKeyId() ) private void assertHasRelationships( long node ) { try ( KernelStatement statement = (KernelStatement) tx.acquireStatement(); - Cursor nodeCursor = statement.getStoreStatement() - .acquireNodeCursor( new SingleNodeProgression( node, null ) ) ) + Cursor nodeCursor = statement.getStoreStatement().acquireSingleNodeCursor( node, null ) ) { nodeCursor.next(); NodeItem nodeItem = nodeCursor.get(); @@ -1524,8 +1520,7 @@ private void testGetRels( long[] relIds ) private void deleteRelationships( long nodeId ) throws Exception { try ( KernelStatement statement = (KernelStatement) tx.acquireStatement(); - Cursor nodeCursor = statement.getStoreStatement() - .acquireNodeCursor( new SingleNodeProgression( nodeId, null ) ) ) + Cursor nodeCursor = statement.getStoreStatement().acquireSingleNodeCursor( nodeId, null ) ) { assertTrue( nodeCursor.next() ); NodeItem nodeItem = nodeCursor.get(); diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/api/store/EnterpriseStoreStatement.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/api/store/EnterpriseStoreStatement.java index 71c34ffeef677..9c200e9cb9cf8 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/api/store/EnterpriseStoreStatement.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/api/store/EnterpriseStoreStatement.java @@ -1,22 +1,3 @@ -/* - * Copyright (c) 2002-2017 "Neo Technology," - * Network Engine for Objects in Lund AB [http://neotechnology.com] - * - * This file is part of Neo4j. - * - * Neo4j is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ package org.neo4j.kernel.impl.api.store; import java.util.function.Supplier; @@ -24,24 +5,13 @@ import org.neo4j.kernel.impl.api.IndexReaderFactory; import org.neo4j.kernel.impl.locking.LockService; import org.neo4j.kernel.impl.store.NeoStores; -import org.neo4j.kernel.impl.store.NodeStore; import org.neo4j.storageengine.api.schema.LabelScanReader; -import org.neo4j.storageengine.api.txstate.ReadableTransactionState; public class EnterpriseStoreStatement extends StoreStatement { - private final NodeStore nodeStore; - public EnterpriseStoreStatement( NeoStores neoStores, Supplier indexReaderFactory, Supplier labelScanReaderSupplier, LockService lockService ) { super( neoStores, indexReaderFactory, labelScanReaderSupplier, lockService ); - this.nodeStore = neoStores.getNodeStore(); - } - - @Override - public NodeProgression parallelNodeScanProgression( ReadableTransactionState state ) - { - return new ParallelAllNodeProgression( nodeStore, state ); } } diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/api/store/ParallelAllNodeProgression.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/api/store/ParallelAllNodeProgression.java deleted file mode 100644 index c18793e63eed7..0000000000000 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/api/store/ParallelAllNodeProgression.java +++ /dev/null @@ -1,117 +0,0 @@ -/* - * Copyright (c) 2002-2017 "Neo Technology," - * Network Engine for Objects in Lund AB [http://neotechnology.com] - * - * This file is part of Neo4j. - * - * Neo4j is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ -package org.neo4j.kernel.impl.api.store; - -import java.util.Iterator; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicLong; - -import org.neo4j.kernel.impl.store.NodeStore; -import org.neo4j.storageengine.api.txstate.NodeState; -import org.neo4j.storageengine.api.txstate.ReadableTransactionState; - -import static java.lang.Math.max; - -class ParallelAllNodeProgression implements NodeProgression -{ - private final NodeStore nodeStore; - private final ReadableTransactionState state; - private final int recordsPerPage; - private final int numberOfReservedLowIds; - private final long lastPageId; - - private final AtomicLong nextPageId = new AtomicLong(); - private final AtomicBoolean done = new AtomicBoolean(); - private final AtomicBoolean append = new AtomicBoolean( true ); - - ParallelAllNodeProgression( NodeStore nodeStore, ReadableTransactionState state ) - { - this.nodeStore = nodeStore; - this.state = state; - recordsPerPage = nodeStore.getRecordsPerPage(); - numberOfReservedLowIds = nodeStore.getNumberOfReservedLowIds(); - // last page to process is the one containing the highest id in use - lastPageId = nodeStore.getHighestPossibleIdInUse() / recordsPerPage; - // start from the page containing the first non reserved id - nextPageId.set( numberOfReservedLowIds / recordsPerPage ); - } - - @Override - public boolean nextBatch( Batch batch ) - { - while ( true ) - { - if ( done.get() ) - { - batch.nothing(); - return false; - } - - long pageId = nextPageId.getAndIncrement(); - if ( pageId < lastPageId ) - { - long first = firstIdOnPage( pageId ); - long last = firstIdOnPage( pageId + 1 ) - 1; - batch.init( first, last ); - return true; - } - else if ( !done.get() && done.compareAndSet( false, true ) ) - { - long first = firstIdOnPage( lastPageId ); - long last = nodeStore.getHighestPossibleIdInUse(); - batch.init( first, last ); - return true; - } - } - } - - private long firstIdOnPage( long pageId ) - { - return max( numberOfReservedLowIds, pageId * recordsPerPage ); - } - - @Override - public Iterator addedNodes() - { - if ( state != null && append.get() && append.compareAndSet( true, false ) ) - { - return state.addedAndRemovedNodes().getAdded().iterator(); - } - return null; - } - - @Override - public boolean fetchFromTxState( long id ) - { - return false; - } - - @Override - public boolean fetchFromDisk( long id ) - { - return state == null || !state.nodeIsDeletedInThisTx( id ); - } - - @Override - public NodeState nodeState( long id ) - { - return state == null ? NodeState.EMPTY : state.getNodeState( id ); - } -} diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/PropertyExistenceEnforcer.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/PropertyExistenceEnforcer.java index a45f5bec75613..b1a2b1f094552 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/PropertyExistenceEnforcer.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/PropertyExistenceEnforcer.java @@ -38,7 +38,6 @@ import org.neo4j.kernel.api.schema.RelationTypeSchemaDescriptor; import org.neo4j.kernel.api.schema.SchemaProcessor; import org.neo4j.kernel.api.schema.constaints.ConstraintDescriptor; -import org.neo4j.kernel.impl.api.store.SingleNodeProgression; import org.neo4j.kernel.impl.locking.Lock; import org.neo4j.storageengine.api.NodeItem; import org.neo4j.storageengine.api.PropertyItem; @@ -212,8 +211,7 @@ private void validateNode( long nodeId ) throws NodePropertyExistenceException } PrimitiveIntSet labelIds; - try ( Cursor node = storeStatement() - .acquireNodeCursor( new SingleNodeProgression( nodeId, txState ) ) ) + try ( Cursor node = storeStatement().acquireSingleNodeCursor( nodeId, txState ) ) { if ( node.next() ) { diff --git a/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/api/store/EnterpriseStoreStatementTest.java b/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/api/store/EnterpriseStoreStatementTest.java deleted file mode 100644 index 936ad628fbc98..0000000000000 --- a/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/api/store/EnterpriseStoreStatementTest.java +++ /dev/null @@ -1,221 +0,0 @@ -/* - * Copyright (c) 2002-2017 "Neo Technology," - * Network Engine for Objects in Lund AB [http://neotechnology.com] - * - * This file is part of Neo4j. - * - * Neo4j is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ -package org.neo4j.kernel.impl.api.store; - -import org.junit.Rule; -import org.junit.Test; - -import java.util.HashSet; -import java.util.Set; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; - -import org.neo4j.cursor.Cursor; -import org.neo4j.graphdb.Transaction; -import org.neo4j.helpers.Exceptions; -import org.neo4j.kernel.impl.api.state.TxState; -import org.neo4j.kernel.impl.storageengine.impl.recordstorage.RecordStorageEngine; -import org.neo4j.kernel.impl.store.NeoStores; -import org.neo4j.kernel.impl.store.NodeStore; -import org.neo4j.kernel.internal.GraphDatabaseAPI; -import org.neo4j.storageengine.api.NodeItem; -import org.neo4j.storageengine.api.txstate.ReadableTransactionState; -import org.neo4j.test.rule.EnterpriseDatabaseRule; -import org.neo4j.test.rule.RandomRule; - -import static java.util.Collections.disjoint; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.neo4j.kernel.impl.locking.LockService.NO_LOCK_SERVICE; - -public class EnterpriseStoreStatementTest -{ - @Rule - public final EnterpriseDatabaseRule databaseRule = new EnterpriseDatabaseRule(); - @Rule - public final RandomRule random = new RandomRule(); - - @Test - public void parallelScanShouldProvideTheSameResultAsANormalScan() throws Throwable - { - GraphDatabaseAPI db = databaseRule.getGraphDatabaseAPI(); - NeoStores neoStores = - db.getDependencyResolver().resolveDependency( RecordStorageEngine.class ).testAccessNeoStores(); - int nodes = randomNodes( neoStores.getNodeStore() ); - createNodes( db, nodes ); - - Set expected = singleThreadExecution( neoStores, null ); - - int threads = random.nextInt( 2, 6 ); - ExecutorService executorService = Executors.newCachedThreadPool(); - try - { - Set parallelResult = parallelExecution( neoStores, executorService, threads, null ); - assertEquals( expected, parallelResult ); - } - finally - { - executorService.shutdown(); - } - } - - @Test - public void parallelScanWithTxStateChangesShouldProvideTheSameResultAsANormalScanWithTheSameChanges() - throws Throwable - { - GraphDatabaseAPI db = databaseRule.getGraphDatabaseAPI(); - NeoStores neoStores = - db.getDependencyResolver().resolveDependency( RecordStorageEngine.class ).testAccessNeoStores(); - int nodes = randomNodes( neoStores.getNodeStore() ); - long lastNodeId = createNodes( db, nodes ); - - TxState txState = crateTxStateWithRandomAddedAndDeletedNodes( nodes, lastNodeId ); - - Set expected = singleThreadExecution( neoStores, txState ); - - ExecutorService executorService = Executors.newCachedThreadPool(); - try - { - int threads = random.nextInt( 2, 6 ); - Set parallelResult = parallelExecution( neoStores, executorService, threads, txState ); - assertEquals( expected, parallelResult ); - } - finally - { - executorService.shutdown(); - } - } - - private Set parallelExecution( NeoStores neoStores, ExecutorService executorService, int threads, - ReadableTransactionState state ) throws Throwable - { - EnterpriseStoreStatement[] localStatements = new EnterpriseStoreStatement[threads]; - for ( int i = 0; i < threads; i++ ) - { - localStatements[i] = new EnterpriseStoreStatement( neoStores, null, null, NO_LOCK_SERVICE ); - } - // use any of the local statements to build the shared progression - NodeProgression progression = localStatements[0].parallelNodeScanProgression( state ); - - @SuppressWarnings( "unchecked" ) - Future>[] futures = new Future[threads]; - for ( int i = 0; i < threads; i++ ) - { - int id = i; - futures[i] = executorService.submit( () -> - { - HashSet ids = new HashSet<>(); - try ( Cursor cursor = localStatements[id].acquireNodeCursor( progression ) ) - { - while ( cursor.next() ) - { - long nodeId = cursor.get().id(); - assertTrue( ids.add( nodeId ) ); - } - } - return ids; - } ); - } - - Throwable t = null; - @SuppressWarnings( "unchecked" ) - Set parallelResult = new HashSet<>(); - for ( int i = 0; i < threads; i++ ) - { - try - { - Set set = futures[i].get(); - assertTrue( disjoint( parallelResult, set ) ); - parallelResult.addAll( set ); - } - catch ( Throwable current ) - { - t = Exceptions.chain( t, current ); - } - } - - if ( t != null ) - { - throw t; - } - - return parallelResult; - } - - private Set singleThreadExecution( NeoStores neoStores, ReadableTransactionState state ) - { - Set expected = new HashSet<>(); - EnterpriseStoreStatement statement = new EnterpriseStoreStatement( neoStores, null, null, NO_LOCK_SERVICE ); - try ( Cursor cursor = statement - .acquireNodeCursor( new AllNodeProgression( neoStores.getNodeStore(), state ) ) ) - { - while ( cursor.next() ) - { - long nodeId = cursor.get().id(); - assertTrue( expected.add( nodeId ) ); - - } - } - return expected; - } - - private int randomNodes( NodeStore nodeStore ) - { - int recordsPerPage = nodeStore.getRecordsPerPage(); - int pages = random.nextInt( 40, 1000 ); - int nonAlignedRecords = random.nextInt( 0, recordsPerPage - 1 ); - return pages * recordsPerPage + nonAlignedRecords; - } - - private TxState crateTxStateWithRandomAddedAndDeletedNodes( int nodes, long lastNodeId ) - { - TxState txState = new TxState(); - for ( long i = lastNodeId + 1; i <= lastNodeId + 100; i++ ) - { - txState.nodeDoCreate( i ); - } - - for ( int i = 0; i < 100; i++ ) - { - long id = random.nextLong( 0, nodes ); - txState.nodeDoDelete( id ); - } - return txState; - } - - private long createNodes( GraphDatabaseAPI db, int nodes ) - { - try ( Transaction tx = db.beginTx() ) - { - long nodeId = -1; - for ( int j = 0; j < nodes; j++ ) - { - nodeId = db.createNode().getId(); - } - tx.success(); - return nodeId; - } - } - - private void noCache( NodeCursor c ) - { - } -} diff --git a/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/api/store/ParallelAllNodeProgressionTest.java b/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/api/store/ParallelAllNodeProgressionTest.java deleted file mode 100644 index 8dee40a4dbfbc..0000000000000 --- a/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/api/store/ParallelAllNodeProgressionTest.java +++ /dev/null @@ -1,205 +0,0 @@ -/* - * Copyright (c) 2002-2017 "Neo Technology," - * Network Engine for Objects in Lund AB [http://neotechnology.com] - * - * This file is part of Neo4j. - * - * Neo4j is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ -package org.neo4j.kernel.impl.api.store; - -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; - -import org.neo4j.helpers.collection.Iterables; -import org.neo4j.helpers.collection.Iterators; -import org.neo4j.kernel.impl.api.state.TxState; -import org.neo4j.kernel.impl.store.NodeStore; -import org.neo4j.test.rule.RandomRule; - -import static java.util.Collections.disjoint; -import static java.util.stream.Collectors.toList; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.neo4j.helpers.collection.Iterables.single; - -public class ParallelAllNodeProgressionTest -{ - @Rule - public RandomRule random = new RandomRule(); - - private final NodeStore nodeStore = mock( NodeStore.class ); - - private int start; - private int end; - private int threads; - - @Before - public void setup() - { - start = random.nextInt( 0, 20 ); - end = random.nextInt( start + 40, start + 100 ); - threads = random.nextInt( 2, 6 ); - when( nodeStore.getNumberOfReservedLowIds() ).thenReturn( start ); - when( nodeStore.getRecordsPerPage() ).thenReturn( end / random.nextInt( 2, 5 ) ); - } - - @Test - public void shouldServeDisjointBatchesToDifferentThreads() throws Throwable - { - // given - when( nodeStore.getHighestPossibleIdInUse() ).thenReturn( (long) end ); - ParallelAllNodeProgression progression = new ParallelAllNodeProgression( nodeStore, null ); - ExecutorService service = Executors.newFixedThreadPool( threads ); - try - { - Future>[] futures = runInParallel( threads, progression, service ); - Set mergedResults = mergeResultsAndAssertDisjoint( futures ); - Set expected = expected( start, end ); - assertEquals( message( expected, mergedResults ), expected, mergedResults ); - } - finally - { - service.shutdown(); - } - } - - @Test - public void shouldConsiderHighIdChanges() throws Throwable - { - // given - when( nodeStore.getHighestPossibleIdInUse() ).thenReturn( (long) end, end + 1L, end + 2L ); - ParallelAllNodeProgression progression = new ParallelAllNodeProgression( nodeStore, null ); - ExecutorService service = Executors.newFixedThreadPool( threads ); - try - { - Future>[] futures = runInParallel( threads, progression, service ); - Set mergedResults = mergeResultsAndAssertDisjoint( futures ); - Set expected = expected( start, end + 1 ); - assertEquals( message( expected, mergedResults ), expected, mergedResults ); - } - finally - { - service.shutdown(); - } - } - - @Test - public void onlyOneShouldRetrieveTheAddedNodes() throws Throwable - { - TxState txState = new TxState(); - Set expected = new HashSet<>(); - for ( long i = 0; i < 10; i++ ) - { - expected.add( end + i ); - txState.nodeDoCreate( end + i ); - } - ParallelAllNodeProgression progression = new ParallelAllNodeProgression( nodeStore, txState ); - ExecutorService service = Executors.newFixedThreadPool( threads ); - try - { - @SuppressWarnings( "unchecked" ) - Future>[] futures = new Future[threads]; - for ( int i = 0; i < threads; i++ ) - { - futures[i] = service.submit( progression::addedNodes ); - } - ArrayList> results = new ArrayList<>(); - for ( int i = 0; i < threads; i++ ) - { - results.add( futures[i].get() ); - } - - List> nonNullResults = results.stream().filter( Objects::nonNull ).collect( toList() ); - assertEquals( 1, nonNullResults.size() ); - assertEquals( expected, Iterators.asSet( single( nonNullResults ) ) ); - } - finally - { - service.shutdown(); - } - } - - private Set mergeResultsAndAssertDisjoint( Future>[] futures ) - throws InterruptedException, java.util.concurrent.ExecutionException - { - Set mergedResults = new HashSet<>(); - for ( int i = 0; i < threads; i++ ) - { - Set result = futures[i].get(); - assertTrue( message( result, mergedResults ), disjoint( mergedResults, result ) ); - mergedResults.addAll( result ); - } - return mergedResults; - } - - private Future>[] runInParallel( int threads, ParallelAllNodeProgression progression, - ExecutorService service ) - { - @SuppressWarnings( "unchecked" ) - Future>[] futures = new Future[this.threads]; - for ( int i = 0; i < threads; i++ ) - { - futures[i] = service.submit( () -> - { - Set result = new HashSet<>(); - NodeProgression.Batch batch = new NodeProgression.Batch(); - while ( progression.nextBatch( batch ) ) - { - while ( batch.hasNext() ) - { - assertTrue( result.add( batch.next() ) ); - } - } - return result; - } ); - } - return futures; - } - - private Set expected( int start, int end ) - { - Set expected = new HashSet<>(); - for ( long i = start; i <= end; i++ ) - { - expected.add( i ); - } - return expected; - } - - private String message( Set expected, Set mergedResults ) - { - return sort( expected ) + "\n" + sort( mergedResults ); - } - - private List sort( Collection expected ) - { - return expected.stream().sorted().collect( toList() ); - } -}