From 4d3b5cd40762b26828c47787e6547cc1ea90238b Mon Sep 17 00:00:00 2001 From: MishaDemianenko Date: Thu, 25 May 2017 18:45:49 +0200 Subject: [PATCH] Revert "Merge pull request #4 from davidegrohmann/3.3-refactor-node-cursor" This reverts commit f9f03c28f4b333c0d454c73ff387dd8fe0ea21c4, reversing changes made to c6f73a21bb5e63382c41bc430d245795d59ec1f9. --- .../full/FullCheckIntegrationTest.java | 15 +- .../org/neo4j/kernel/api/ReadOperations.java | 2 - .../neo4j/kernel/api/StatementConstants.java | 1 - .../ConstraintEnforcingEntityOperations.java | 6 - .../impl/api/GuardingStatementOperations.java | 7 - .../kernel/impl/api/OperationsFacade.java | 7 - .../api/StateHandlingStatementOperations.java | 7 - .../api/operations/EntityReadOperations.java | 2 - .../impl/api/store/AllNodeProgression.java | 49 -- .../kernel/impl/api/store/Progression.java | 33 - .../impl/api/store/SingleNodeProgression.java | 51 -- .../kernel/impl/api/store/StorageLayer.java | 7 - ...Cursor.java => StoreSingleNodeCursor.java} | 133 ++-- .../kernel/impl/api/store/StoreStatement.java | 18 +- .../impl/util/diffsets/SuperDiffSets.java | 5 + .../storageengine/api/StorageStatement.java | 2 - .../storageengine/api/StoreReadLayer.java | 3 - .../api/txstate/ReadableTransactionState.java | 2 +- .../impl/api/integrationtest/NodeIT.java | 142 ---- .../kernel/impl/api/store/NodeCursorTest.java | 605 ------------------ .../StorageLayerRelTypesAndDegreeTest.java | 27 +- .../api/store/StoreSingleNodeCursorTest.java | 64 ++ .../state/NodeLabelsFieldTest.java | 2 +- 23 files changed, 175 insertions(+), 1015 deletions(-) delete mode 100644 community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllNodeProgression.java delete mode 100644 community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/Progression.java delete mode 100644 community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SingleNodeProgression.java rename community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/{NodeCursor.java => StoreSingleNodeCursor.java} (66%) delete mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/api/integrationtest/NodeIT.java delete mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/NodeCursorTest.java create mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StoreSingleNodeCursorTest.java diff --git a/community/consistency-check/src/test/java/org/neo4j/consistency/checking/full/FullCheckIntegrationTest.java b/community/consistency-check/src/test/java/org/neo4j/consistency/checking/full/FullCheckIntegrationTest.java index 93d0aaf818d10..d11116d4bdea4 100644 --- a/community/consistency-check/src/test/java/org/neo4j/consistency/checking/full/FullCheckIntegrationTest.java +++ b/community/consistency-check/src/test/java/org/neo4j/consistency/checking/full/FullCheckIntegrationTest.java @@ -101,6 +101,7 @@ import org.neo4j.kernel.impl.store.record.RelationshipGroupRecord; import org.neo4j.kernel.impl.store.record.RelationshipRecord; import org.neo4j.kernel.impl.store.record.RelationshipTypeTokenRecord; +import org.neo4j.kernel.impl.util.Bits; import org.neo4j.kernel.internal.GraphDatabaseAPI; import org.neo4j.logging.FormattedLog; import org.neo4j.storageengine.api.schema.SchemaRule; @@ -141,7 +142,7 @@ import static org.neo4j.kernel.impl.store.record.Record.NO_NEXT_RELATIONSHIP; import static org.neo4j.kernel.impl.store.record.Record.NO_PREV_RELATIONSHIP; import static org.neo4j.kernel.impl.store.record.RecordLoad.FORCE; -import static org.neo4j.kernel.impl.transaction.state.NodeLabelsFieldTest.inlinedLabelsLongRepresentation; +import static org.neo4j.kernel.impl.util.Bits.bits; import static org.neo4j.test.Property.property; import static org.neo4j.test.Property.set; @@ -693,6 +694,18 @@ protected void transactionData( GraphStoreFixture.TransactionDataBuilder tx, .andThatsAllFolks(); } + private long inlinedLabelsLongRepresentation( long... labelIds ) + { + long header = (long) labelIds.length << 36; + byte bitsPerLabel = (byte) (36 / labelIds.length); + Bits bits = bits( 5 ); + for ( long labelId : labelIds ) + { + bits.put( labelId, bitsPerLabel ); + } + return header | bits.getLongs()[0]; + } + @Test public void shouldReportCyclesInDynamicRecordsWithLabels() throws Exception { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/ReadOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/api/ReadOperations.java index e3feedf698daa..278b6fe69b161 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/ReadOperations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/ReadOperations.java @@ -211,8 +211,6 @@ void relationshipVisit( long relId, RelationshipVi //== CURSOR ACCESS OPERATIONS =============== //=========================================== - Cursor nodeGetAllCursor(); - Cursor nodeCursorById( long nodeId ) throws EntityNotFoundException; Cursor relationshipCursorById( long relId ) throws EntityNotFoundException; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/StatementConstants.java b/community/kernel/src/main/java/org/neo4j/kernel/api/StatementConstants.java index 080f0b9fb6d1f..d58899c36d862 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/StatementConstants.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/StatementConstants.java @@ -26,7 +26,6 @@ public final class StatementConstants public static final int NO_SUCH_PROPERTY_KEY = -1; public static final long NO_SUCH_NODE = -1; public static final long NO_SUCH_RELATIONSHIP = -1; - public static final long NO_SUCH_PROPERTY = -1; private StatementConstants() { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java index 542ab94131185..6031765e17ce9 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java @@ -473,12 +473,6 @@ public void relationshipVisit( KernelStatement sta entityReadOperations.relationshipVisit( statement, relId, visitor ); } - @Override - public Cursor nodeGetAllCursor( KernelStatement statement ) - { - return entityReadOperations.nodeGetAllCursor( statement ); - } - @Override public Cursor nodeCursorById( KernelStatement statement, long nodeId ) throws EntityNotFoundException { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/GuardingStatementOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/GuardingStatementOperations.java index ac1b70418cb39..6f636860784e1 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/GuardingStatementOperations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/GuardingStatementOperations.java @@ -243,13 +243,6 @@ public void relationshipVisit( KernelStatement sta entityReadDelegate.relationshipVisit( statement, relId, visitor ); } - @Override - public Cursor nodeGetAllCursor( KernelStatement statement ) - { - guard.check( statement ); - return entityReadDelegate.nodeGetAllCursor( statement ); - } - @Override public Cursor nodeCursorById( KernelStatement statement, long nodeId ) throws EntityNotFoundException { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java index f4776e59f94f2..5148218d745eb 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java @@ -549,13 +549,6 @@ public long nodesCountIndexed( IndexDescriptor index, long nodeId, Object value // // - @Override - public Cursor nodeGetAllCursor() - { - statement.assertOpen(); - return dataRead().nodeGetAllCursor( statement ); - } - @Override public Cursor nodeCursorById( long nodeId ) throws EntityNotFoundException { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java index 3c5f101869fef..58beeed5c234b 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java @@ -144,13 +144,6 @@ public StateHandlingStatementOperations( StoreReadLayer storeLayer, AutoIndexing // - @Override - public Cursor nodeGetAllCursor( KernelStatement statement ) - { - TransactionState state = statement.hasTxStateWithChanges() ? statement.txState() : null; - return storeLayer.nodeGetAllCursor( statement.getStoreStatement(), state ); - } - @Override public Cursor nodeCursorById( KernelStatement statement, long nodeId ) throws EntityNotFoundException { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/operations/EntityReadOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/operations/EntityReadOperations.java index 3c577245bfd22..97771b8b576ff 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/operations/EntityReadOperations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/operations/EntityReadOperations.java @@ -86,8 +86,6 @@ long nodesCountIndexed( KernelStatement statement, IndexDescriptor index, long n void relationshipVisit( KernelStatement statement, long relId, RelationshipVisitor visitor ) throws EntityNotFoundException, EXCEPTION; - Cursor nodeGetAllCursor( KernelStatement statement ); - Cursor nodeCursorById( KernelStatement statement, long nodeId ) throws EntityNotFoundException; Cursor relationshipCursorById( KernelStatement statement, long relId ) 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 deleted file mode 100644 index bc25cafd17a95..0000000000000 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllNodeProgression.java +++ /dev/null @@ -1,49 +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.neo4j.kernel.api.StatementConstants; -import org.neo4j.kernel.impl.store.NodeStore; - -public class AllNodeProgression implements Progression -{ - private final AllIdIterator allIdIterator; - - AllNodeProgression( NodeStore nodeStore ) - { - allIdIterator = new AllIdIterator( nodeStore ); - } - - @Override - public long nextId() - { - if ( allIdIterator.hasNext() ) - { - return allIdIterator.next(); - } - return StatementConstants.NO_SUCH_NODE; - } - - @Override - public Mode mode() - { - return Mode.APPEND; - } -} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/Progression.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/Progression.java deleted file mode 100644 index a78f84668e34a..0000000000000 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/Progression.java +++ /dev/null @@ -1,33 +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; - -public interface Progression -{ - enum Mode - { - APPEND, - FETCH - } - - long nextId(); - - Mode 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 deleted file mode 100644 index 6156a44c8d043..0000000000000 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SingleNodeProgression.java +++ /dev/null @@ -1,51 +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.neo4j.kernel.api.StatementConstants; - -public class SingleNodeProgression implements Progression -{ - private long nodeId; - - SingleNodeProgression( long nodeId ) - { - this.nodeId = nodeId; - } - - @Override - public long nextId() - { - try - { - return nodeId; - } - finally - { - nodeId = StatementConstants.NO_SUCH_NODE; - } - } - - @Override - public Mode mode() - { - return Mode.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 289b644f13673..3e955c1bde5a1 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 @@ -43,7 +43,6 @@ import org.neo4j.kernel.api.schema.SchemaDescriptor; import org.neo4j.kernel.api.schema.constaints.ConstraintDescriptor; import org.neo4j.kernel.api.schema.index.IndexDescriptor; -import org.neo4j.kernel.api.txstate.TransactionState; import org.neo4j.kernel.impl.api.DegreeVisitor; import org.neo4j.kernel.impl.api.RelationshipVisitor; import org.neo4j.kernel.impl.api.index.IndexingService; @@ -419,12 +418,6 @@ public RelationshipIterator relationshipsGetAll() return new AllRelationshipIterator( relationshipStore ); } - @Override - public Cursor nodeGetAllCursor( StorageStatement statement, TransactionState state ) - { - return statement.acquireNodeCursor( state ); - } - @Override public Cursor nodeCursor( StorageStatement statement, long nodeId, ReadableTransactionState state ) { 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/StoreSingleNodeCursor.java similarity index 66% rename from community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/NodeCursor.java rename to community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSingleNodeCursor.java index cd22b32fbc258..a1aeb0d93408b 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/StoreSingleNodeCursor.java @@ -19,134 +19,147 @@ */ package org.neo4j.kernel.impl.api.store; -import java.util.Iterator; import java.util.function.Consumer; import org.neo4j.collection.primitive.PrimitiveIntSet; import org.neo4j.cursor.Cursor; +import org.neo4j.kernel.api.StatementConstants; import org.neo4j.kernel.impl.locking.Lock; import org.neo4j.kernel.impl.locking.LockService; import org.neo4j.kernel.impl.store.NodeLabelsField; import org.neo4j.kernel.impl.store.RecordCursors; import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.kernel.impl.store.record.Record; -import org.neo4j.kernel.impl.store.record.RecordLoad; 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.Primitive.intSet; import static org.neo4j.collection.primitive.PrimitiveIntCollections.asSet; -import static org.neo4j.kernel.impl.api.store.Progression.Mode.APPEND; -import static org.neo4j.kernel.impl.api.store.Progression.Mode.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.Record.NO_NEXT_PROPERTY; +import static org.neo4j.kernel.impl.store.record.Record.NO_NEXT_RELATIONSHIP; import static org.neo4j.kernel.impl.store.record.RecordLoad.CHECK; import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.safeCastLongToInt; -public class NodeCursor implements NodeItem, Cursor +/** + * Base cursor for nodes. + */ +public class StoreSingleNodeCursor implements Cursor, NodeItem { private final NodeRecord nodeRecord; - private final Consumer instanceCache; - private final RecordCursors recordCursors; + private final Consumer instanceCache; + private final LockService lockService; + private final RecordCursors recordCursors; - private Progression progression; + private long nodeId = StatementConstants.NO_SUCH_NODE; private ReadableTransactionState state; - private boolean fetched; + private long[] labels; - private Iterator added; + private boolean fetched; + private NodeState nodeState; - NodeCursor( NodeRecord nodeRecord, Consumer instanceCache, RecordCursors recordCursors, - LockService lockService ) + StoreSingleNodeCursor( NodeRecord nodeRecord, Consumer instanceCache, + RecordCursors recordCursors, LockService lockService ) { this.nodeRecord = nodeRecord; - this.instanceCache = instanceCache; this.recordCursors = recordCursors; this.lockService = lockService; + this.instanceCache = instanceCache; } - public Cursor init( Progression progression, ReadableTransactionState state ) + public StoreSingleNodeCursor init( long nodeId, ReadableTransactionState state ) { - this.progression = progression; this.state = state; - this.added = state != null && progression.mode() == APPEND - ? state.addedAndRemovedNodes().getAdded().iterator() - : null; + this.nodeId = nodeId; return this; } @Override - public boolean next() + public NodeItem get() { - return fetched = fetchNext(); + return this; } - private boolean fetchNext() + @Override + public boolean next() { - labels = null; - long id; - while ( (id = progression.nextId()) >= 0 ) + clearCurrentNodeState(); + if ( nodeId == StatementConstants.NO_SUCH_NODE ) { - if ( (state == null || !state.nodeIsDeletedInThisTx( id )) && - recordCursors.node().next( id, nodeRecord, RecordLoad.CHECK ) ) - { - return true; - } - - if ( state != null && progression.mode() == FETCH && state.nodeIsAddedInThisTx( id ) ) - { - recordFromTxState( id ); - return true; - } + return false; } - if ( added != null && added.hasNext() ) + if ( hasNext() ) { - recordFromTxState( added.next() ); + nodeState = state != null ? state.getNodeState( nodeId ) : null; return true; } + nodeId = StatementConstants.NO_SUCH_NODE; return false; } - private void recordFromTxState( long id ) + private boolean hasNext() { - nodeRecord.clear(); - nodeRecord.setId( id ); + // fetched makes sure we read the node from disk/tx state only once and we do not loop forever + if ( fetched ) + { + return false; + } + + try + { + if ( state != null && state.nodeIsDeletedInThisTx( nodeId ) ) + { + return false; + } + return recordCursors.node().next( nodeId, nodeRecord, CHECK ) || + state != null && state.nodeIsAddedInThisTx( nodeId ); + } + finally + { + fetched = true; + } } @Override public void close() { - labels = null; - added = null; state = null; + clearCurrentNodeState(); + fetched = false; + nodeRecord.clear(); instanceCache.accept( this ); } - @Override - public NodeItem get() + private void clearCurrentNodeState() { - if ( fetched ) - { - return this; - } + labels = null; + nodeState = null; + } - throw new IllegalStateException( "Nothing available" ); + @Override + public long id() + { + return nodeRecord.getId(); } @Override public PrimitiveIntSet labels() { - PrimitiveIntSet labels = asSet( loadedLabels(), IoPrimitiveUtils::safeCastLongToInt ); - return state != null ? state.augmentLabels( labels, state.getNodeState( id() ) ) : labels; + PrimitiveIntSet baseLabels = state != null && state.nodeIsAddedInThisTx( nodeId ) + ? intSet() + : asSet( loadedLabels(), IoPrimitiveUtils::safeCastLongToInt ); + return state != null ? state.augmentLabels( baseLabels, nodeState ) : baseLabels; } @Override public boolean hasLabel( int labelId ) { - NodeState nodeState = state == null ? null : state.getNodeState( id() ); if ( state != null && nodeState.labelDiffSets().getRemoved().contains( labelId ) ) { return false; @@ -176,16 +189,10 @@ private long[] loadedLabels() return labels; } - @Override - public long id() - { - return nodeRecord.getId(); - } - @Override public boolean isDense() { - return nodeRecord.isDense(); + return state != null && state.nodeIsAddedInThisTx( nodeId ) ? false : nodeRecord.isDense(); } @Override @@ -198,19 +205,21 @@ public long nextGroupId() @Override public long nextRelationshipId() { - return nodeRecord.getNextRel(); + return state != null && state.nodeIsAddedInThisTx( nodeId ) ? NO_NEXT_RELATIONSHIP.longValue() + : nodeRecord.getNextRel(); } @Override public long nextPropertyId() { - return nodeRecord.getNextProp(); + return state != null && state.nodeIsAddedInThisTx( nodeId ) ? NO_NEXT_PROPERTY.longValue() + : nodeRecord.getNextProp(); } @Override public Lock lock() { - return state != null && state.nodeIsAddedInThisTx( id() ) ? NO_LOCK : acquireLock(); + return state != null && state.nodeIsAddedInThisTx( nodeId ) ? NO_LOCK : acquireLock(); } private Lock acquireLock() 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 45bfe62013f09..5733d8afa35be 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 @@ -21,6 +21,7 @@ import java.util.function.Supplier; +import org.neo4j.collection.primitive.PrimitiveIntSet; import org.neo4j.cursor.Cursor; import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException; import org.neo4j.kernel.api.schema.index.IndexDescriptor; @@ -53,7 +54,7 @@ */ public class StoreStatement implements StorageStatement { - private final InstanceCache nodeCursor; + private final InstanceCache singleNodeCursor; private final InstanceCache singleRelationshipCursor; private final InstanceCache iteratorRelationshipCursor; private final InstanceCache nodeRelationshipsCursor; @@ -84,12 +85,12 @@ public StoreStatement( NeoStores neoStores, Supplier indexRe this.relationshipGroupStore = neoStores.getRelationshipGroupStore(); this.recordCursors = new RecordCursors( neoStores ); - nodeCursor = new InstanceCache() + singleNodeCursor = new InstanceCache() { @Override - protected NodeCursor create() + protected StoreSingleNodeCursor create() { - return new NodeCursor( nodeStore.newRecord(), this, recordCursors, lockService ); + return new StoreSingleNodeCursor( nodeStore.newRecord(), this, recordCursors, lockService ); } }; singleRelationshipCursor = new InstanceCache() @@ -145,18 +146,11 @@ public void acquire() this.acquired = true; } - @Override - public Cursor acquireNodeCursor( ReadableTransactionState state ) - { - neoStores.assertOpen(); - return nodeCursor.get().init( new AllNodeProgression( nodeStore ), state ); - } - @Override public Cursor acquireSingleNodeCursor( long nodeId, ReadableTransactionState state ) { neoStores.assertOpen(); - return nodeCursor.get().init( new SingleNodeProgression( nodeId ), state ); + return singleNodeCursor.get().init( nodeId, state ); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/diffsets/SuperDiffSets.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/diffsets/SuperDiffSets.java index e99a5d9880650..639193abb511c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/diffsets/SuperDiffSets.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/diffsets/SuperDiffSets.java @@ -46,6 +46,11 @@ abstract class SuperDiffSets private Set removedElements; private Predicate filter; + SuperDiffSets() + { + this( null, null ); + } + SuperDiffSets( Set addedElements, Set removedElements ) { this.addedElements = addedElements; 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 a1cb172768ec1..9e7ff3a430593 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 @@ -65,8 +65,6 @@ public interface StorageStatement extends AutoCloseable @Override void close(); - 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 diff --git a/community/kernel/src/main/java/org/neo4j/storageengine/api/StoreReadLayer.java b/community/kernel/src/main/java/org/neo4j/storageengine/api/StoreReadLayer.java index 8c1a8fb3a8abf..c2093882af952 100644 --- a/community/kernel/src/main/java/org/neo4j/storageengine/api/StoreReadLayer.java +++ b/community/kernel/src/main/java/org/neo4j/storageengine/api/StoreReadLayer.java @@ -38,7 +38,6 @@ import org.neo4j.kernel.api.schema.SchemaDescriptor; import org.neo4j.kernel.api.schema.constaints.ConstraintDescriptor; import org.neo4j.kernel.api.schema.index.IndexDescriptor; -import org.neo4j.kernel.api.txstate.TransactionState; import org.neo4j.kernel.impl.api.DegreeVisitor; import org.neo4j.kernel.impl.api.RelationshipVisitor; import org.neo4j.kernel.impl.api.store.RelationshipIterator; @@ -278,8 +277,6 @@ void relationshipVisit( long relationshipId, */ RelationshipIterator relationshipsGetAll(); - Cursor nodeGetAllCursor( StorageStatement storeStatement, TransactionState state ); - Cursor nodeCursor( StorageStatement storeStatement, long nodeId, ReadableTransactionState state ); Cursor relationshipCursor( StorageStatement storeStatement, long relationshipId, 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 aae655e7078a9..d41b4a561aa00 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 @@ -140,7 +140,7 @@ ReadableDiffSets indexUpdatesForRangeSeekByString( IndexDescriptor index, RelationshipState getRelationshipState( long id ); - PrimitiveIntSet augmentLabels( PrimitiveIntSet labels, NodeState nodeState ); + PrimitiveIntSet augmentLabels( PrimitiveIntSet cursor, NodeState nodeState ); /** * The way tokens are created is that the first time a token is needed it gets created in its own little diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/integrationtest/NodeIT.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/integrationtest/NodeIT.java deleted file mode 100644 index c2b8133f4c4d3..0000000000000 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/integrationtest/NodeIT.java +++ /dev/null @@ -1,142 +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.integrationtest; - -import org.junit.Test; - -import org.neo4j.collection.primitive.Primitive; -import org.neo4j.collection.primitive.PrimitiveLongCollections; -import org.neo4j.collection.primitive.PrimitiveLongSet; -import org.neo4j.kernel.api.DataWriteOperations; -import org.neo4j.kernel.api.ReadOperations; -import org.neo4j.kernel.api.Statement; -import org.neo4j.kernel.api.exceptions.InvalidTransactionTypeKernelException; -import org.neo4j.kernel.api.exceptions.KernelException; -import org.neo4j.kernel.api.security.AnonymousContext; - -import static org.junit.Assert.assertEquals; - -public class NodeIT extends KernelIntegrationTest -{ - @Test - public void nodeScanShouldBehaveTheSameUsingIteratorsAndCursors() throws Throwable - { - createNodesAndCommit( 100 ); - - ReadOperations read = readOperationsInNewTransaction(); - - PrimitiveLongSet cursorIds = Primitive.longSet(); - read.nodeGetAllCursor().forAll( n -> cursorIds.add( n.id() ) ); - - PrimitiveLongSet iteratorIds = PrimitiveLongCollections.asSet( read.nodesGetAll() ); - - commit(); - - assertEquals( iteratorIds, cursorIds ); - } - - @Test - public void nodeScanShouldBehaveTheSameUsingIteratorsAndCursorsWithAddedNodesInTxState() throws Throwable - { - createNodesAndCommit( 100 ); - Statement statement = statementInNewTransaction( AnonymousContext.write() ); - createNodes( statement.dataWriteOperations(), 20 ); - - PrimitiveLongSet cursorIds = Primitive.longSet(); - statement.readOperations().nodeGetAllCursor().forAll( n -> cursorIds.add( n.id() ) ); - - PrimitiveLongSet iteratorIds = PrimitiveLongCollections.asSet( statement.readOperations().nodesGetAll() ); - - commit(); - - assertEquals( iteratorIds, cursorIds ); - } - - @Test - public void nodeScanShouldBehaveTheSameUsingIteratorsAndCursorsWithAddedAndDeletedNodesInTxState() throws Throwable - { - createNodesAndCommit( 100 ); - deleteNodesAndCommit( 24, 56 ); - Statement statement = statementInNewTransaction( AnonymousContext.write() ); - createNodes( statement.dataWriteOperations(), 20 ); - deleteNodes( statement.dataWriteOperations(), 102, 33, 44 ); - - PrimitiveLongSet cursorIds = Primitive.longSet(); - statement.readOperations().nodeGetAllCursor().forAll( n -> cursorIds.add( n.id() ) ); - - PrimitiveLongSet iteratorIds = PrimitiveLongCollections.asSet( statement.readOperations().nodesGetAll() ); - - commit(); - - assertEquals( iteratorIds, cursorIds ); - } - - @Test - public void nodeCursorShouldBeStable() throws Throwable - { - createNodesAndCommit( 100 ); - deleteNodesAndCommit( 24, 56 ); - Statement statement = statementInNewTransaction( AnonymousContext.write() ); - createNodes( statement.dataWriteOperations(), 20 ); - deleteNodes( statement.dataWriteOperations(), 102, 33, 44 ); - - statement.readOperations().nodeGetAllCursor().forAll( n -> - { - try - { - statement.dataWriteOperations().nodeCreate(); - } - catch ( InvalidTransactionTypeKernelException e ) - { - // ignore for this test - } - } ); - - commit(); - } - - private void deleteNodesAndCommit( long... nodeIds ) throws KernelException - { - deleteNodes( dataWriteOperationsInNewTransaction(), nodeIds ); - commit(); - } - - private void deleteNodes( DataWriteOperations write, long... nodeIds ) throws KernelException - { - for ( long nodeId : nodeIds ) - { - write.nodeDelete( nodeId ); - } - } - - private void createNodesAndCommit( int nodes ) throws KernelException - { - createNodes( dataWriteOperationsInNewTransaction(), nodes ); - commit(); - } - - private void createNodes( DataWriteOperations write, int nodes ) throws KernelException - { - for ( int i = 0; i < nodes; i++ ) - { - write.nodeCreate(); - } - } -} 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 deleted file mode 100644 index 2a53dad8d1d56..0000000000000 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/NodeCursorTest.java +++ /dev/null @@ -1,605 +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.apache.commons.lang3.mutable.MutableBoolean; -import org.junit.Rule; -import org.junit.Test; -import org.junit.experimental.theories.DataPoints; -import org.junit.experimental.theories.Theories; -import org.junit.experimental.theories.Theory; -import org.junit.runner.RunWith; - -import java.util.Arrays; -import java.util.Set; -import java.util.function.LongFunction; - -import org.neo4j.collection.primitive.PrimitiveIntIterator; -import org.neo4j.collection.primitive.PrimitiveIntSet; -import org.neo4j.cursor.Cursor; -import org.neo4j.kernel.api.StatementConstants; -import org.neo4j.kernel.impl.api.state.TxState; -import org.neo4j.kernel.impl.api.store.Progression.Mode; -import org.neo4j.kernel.impl.locking.Lock; -import org.neo4j.kernel.impl.store.RecordCursor; -import org.neo4j.kernel.impl.store.RecordCursors; -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.ReadableTransactionState; -import org.neo4j.test.rule.RepeatRule; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.mockito.Mockito.mock; -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.Progression.Mode.APPEND; -import static org.neo4j.kernel.impl.api.store.Progression.Mode.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; - -@RunWith( Theories.class ) -public class NodeCursorTest -{ - /* - * each test is gonna run twice to make sure we reuse cursor correctly - */ - @Rule - public RepeatRule repeatRule = new RepeatRule(); - - private static final int NON_EXISTING_LABEL = Integer.MAX_VALUE; - - private final NodeRecord nodeRecord = new NodeRecord( -1 ); - private final RecordCursors recordCursors = mock( RecordCursors.class ); - @SuppressWarnings( "unchecked" ) - private final RecordCursor recordCursor = mock( RecordCursor.class ); - // cursor is shared since it is designed to be reusable - private final NodeCursor reusableCursor = new NodeCursor( nodeRecord, i -> {}, recordCursors, - NO_LOCK_SERVICE ); - - { - when( recordCursors.node() ).thenReturn( recordCursor ); - } - - @SuppressWarnings( "unchecked" ) - @DataPoints - public static LongFunction[] data() - { - return new LongFunction[]{NodeOnDisk::new, AddedNode::new, DeletedNode::new, ModifiedNode::new}; - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void nothingInAppendMode() throws Throwable - { - // given - TestRun test = new TestRun( APPEND, new Operation[0] ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void nothingInFetchMode() throws Throwable - { - // given - TestRun test = new TestRun( FETCH, new Operation[0] ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void anElementInAppendMode( LongFunction op0 ) throws Throwable - { - // given - TestRun test = new TestRun( APPEND, new Operation[]{op0.apply( 0 )} ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void anElementInFetchMode( LongFunction op0 ) throws Throwable - { - // given - TestRun test = new TestRun( FETCH, new Operation[]{op0.apply( 0 )} ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void twoElementsInAppendMode( LongFunction op0, LongFunction op1 ) throws Throwable - { - // given - TestRun test = new TestRun( APPEND, new Operation[]{op0.apply( 0 ), op1.apply( 1 )} ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void twoElementsInFetchMode( LongFunction op0, LongFunction op1 ) throws Throwable - { - // given - TestRun test = new TestRun( FETCH, new Operation[]{op0.apply( 0 ), op1.apply( 1 )} ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void threeElementsInAppendMode( LongFunction op0, LongFunction op1, - LongFunction op2 ) throws Throwable - { - // given - TestRun test = new TestRun( APPEND, new Operation[]{op0.apply( 0 ), op1.apply( 1 ), op2.apply( 2 )} ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void threeElementsInFetchMode( LongFunction op0, LongFunction op1, - LongFunction op2 ) throws Throwable - { - // given - TestRun test = new TestRun( FETCH, new Operation[]{op0.apply( 0 ), op1.apply( 1 ), op2.apply( 2 )} ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void fourElementsInAppendMode( LongFunction op0, LongFunction op1, - LongFunction op2, LongFunction op3 ) throws Throwable - { - // given - TestRun test = - new TestRun( APPEND, new Operation[]{op0.apply( 0 ), op1.apply( 1 ), op2.apply( 2 ), op3.apply( 3 )} ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Theory - @RepeatRule.Repeat( times = 2 ) - public void fourElementsInFetchMode( LongFunction op0, LongFunction op1, - LongFunction op2, LongFunction op3 ) throws Throwable - { - // given - TestRun test = - new TestRun( FETCH, new Operation[]{op0.apply( 0 ), op1.apply( 1 ), op2.apply( 2 ), op3.apply( 3 )} ); - Cursor cursor = test.initialize( reusableCursor, recordCursor, nodeRecord ); - - // when/then - test.runAndVerify( cursor ); - } - - @Test - public void shouldCallTheConsumerOnClose() - { - MutableBoolean called = new MutableBoolean(); - NodeCursor cursor = new NodeCursor( nodeRecord, c -> called.setTrue(), recordCursors, NO_LOCK_SERVICE ); - cursor.init( mock( Progression.class ), mock( ReadableTransactionState.class ) ); - assertFalse( called.booleanValue() ); - - cursor.close(); - assertTrue( called.booleanValue() ); - } - - private static class ModifiedNode extends NodeOnDisk - { - private ModifiedNode( long id ) - { - super( id ); - } - - @Override - public TxState prepare( RecordCursor recordCursor, NodeRecord nodeRecord, TxState state ) - { - state = state == null ? new TxState() : state; - state.nodeDoAddLabel( 6 + (int) id, id ); - state.nodeDoRemoveLabel( 5 + (int) id, id ); - return super.prepare( recordCursor, nodeRecord, state ); - } - - @Override - public String toString() - { - return String.format( "ModifiedNode[%d]", id ); - } - } - - private static class AddedNode implements Operation - { - private final long id; - - private TxState state; - - private AddedNode( long id ) - { - this.id = id; - } - - @Override - public long id() - { - return id; - } - - @Override - public TxState prepare( RecordCursor recordCursor, NodeRecord nodeRecord, TxState state ) - { - this.state = state = state == null ? new TxState() : state; - state.nodeDoCreate( id ); - state.nodeDoAddLabel( 20 + (int) id, id ); - return state; - } - - @Override - public boolean fromDisk() - { - return false; - } - - @Override - public void check( NodeItem node ) - { - Set added = state.addedAndRemovedNodes().getAdded(); - assertTrue( added.contains( node.id() ) ); - state.getNodeState( node.id() ).labelDiffSets().getAdded(); - PrimitiveIntSet expectedLabels = - asSet( asArray( state.getNodeState( node.id() ).labelDiffSets().getAdded() ) ); - assertEquals( expectedLabels, node.labels() ); - assertEquals( StatementConstants.NO_SUCH_PROPERTY, node.nextPropertyId() ); - assertFalse( node.isDense() ); - assertEquals( StatementConstants.NO_SUCH_RELATIONSHIP, node.nextRelationshipId() ); - PrimitiveIntIterator iterator = expectedLabels.iterator(); - while ( iterator.hasNext() ) - { - assertTrue( node.hasLabel( iterator.next() ) ); - assertFalse( node.hasLabel( NON_EXISTING_LABEL ) ); - } - } - - @Override - public String toString() - { - return String.format( "AddedNode[%d]", id ); - } - } - - private static class DeletedNode implements Operation - { - private final long id; - - private DeletedNode( long id ) - { - this.id = id; - } - - @Override - public long id() - { - return id; - } - - @Override - public TxState prepare( RecordCursor recordCursor, NodeRecord nodeRecord, TxState state ) - { - state = state == null ? new TxState() : state; - state.nodeDoDelete( id ); - record( id, recordCursor, nodeRecord, state ); - return state; - } - - @Override - public boolean fromDisk() - { - return true; - } - - @Override - public void check( NodeItem nodeItem ) - { - throw new UnsupportedOperationException( "no check" ); - } - - @Override - public String toString() - { - return String.format( "DeletedNode[%d]", id ); - } - } - - private static class NodeOnDisk implements Operation - { - protected final long id; - private NodeItem expected; - - private NodeOnDisk( long id ) - { - this.id = id; - } - - @Override - public long id() - { - return id; - } - - @Override - public TxState prepare( RecordCursor recordCursor, NodeRecord nodeRecord, TxState state ) - { - expected = record( id, recordCursor, nodeRecord, state ); - return state; - } - - @Override - public boolean fromDisk() - { - return true; - } - - @Override - public void check( NodeItem nodeItem ) - { - assertEquals( expected.id(), nodeItem.id() ); - assertEquals( expected.nextPropertyId(), nodeItem.nextPropertyId() ); - assertEquals( expected.isDense(), nodeItem.isDense() ); - assertEquals( expected.labels(), nodeItem.labels() ); - if ( nodeItem.isDense() ) - { - assertEquals( expected.nextGroupId(), nodeItem.nextGroupId() ); - } - else - { - assertEquals( expected.nextRelationshipId(), nodeItem.nextRelationshipId() ); - } - - PrimitiveIntIterator iterator = expected.labels().iterator(); - while ( iterator.hasNext() ) - { - assertTrue( nodeItem.hasLabel( iterator.next() ) ); - assertFalse( nodeItem.hasLabel( NON_EXISTING_LABEL ) ); - } - } - - @Override - public String toString() - { - return String.format( "NodeOnDisk[%d]", id ); - } - } - - private static NodeItem record( long id, RecordCursor recordCursor, NodeRecord nodeRecord, - ReadableTransactionState state ) - { - boolean dense = id % 2 == 0; - int nextProp = 42 + (int) id; - long nextRel = 43 + id; - long[] labelIds = new long[]{4 + id, 5 + id}; - when( recordCursor.next( id, nodeRecord, CHECK ) ).thenAnswer( invocationOnMock -> - { - nodeRecord.setId( id ); - nodeRecord.initialize( true, nextProp, dense, nextRel, inlinedLabelsLongRepresentation( labelIds ) ); - return true; - } ); - return new NodeItem() - { - @Override - public long id() - { - return id; - } - - @Override - public PrimitiveIntSet labels() - { - PrimitiveIntSet labels = asSet( labelIds, IoPrimitiveUtils::safeCastLongToInt ); - return state == null ? labels : state.augmentLabels( labels, state.getNodeState( id ) ); - } - - @Override - public boolean isDense() - { - return dense; - } - - @Override - public boolean hasLabel( int labelId ) - { - throw new UnsupportedOperationException( "don't call this" ); - } - - @Override - public long nextGroupId() - { - assert isDense(); - return nextRelationshipId(); - } - - @Override - public long nextRelationshipId() - { - return nextRel; - } - - @Override - public long nextPropertyId() - { - return nextProp; - } - - @Override - public Lock lock() - { - throw new UnsupportedOperationException( "don't call this" ); - } - }; - } - - private interface Operation - { - long id(); - - TxState prepare( RecordCursor recordCursor, NodeRecord nodeRecord, TxState state ); - - boolean fromDisk(); - - void check( NodeItem nodeItem ); - } - - private static class TestRun - { - private final Mode mode; - private final Operation[] ops; - - private TxState state; - - private TestRun( Mode mode, Operation[] ops ) - { - this.mode = mode; - this.ops = ops; - } - - Cursor initialize( NodeCursor cursor, RecordCursor recordCursor, NodeRecord nodeRecord ) - { - for ( Operation op : ops ) - { - state = op.prepare( recordCursor, nodeRecord, state ); - } - return cursor.init( createProgression( ops, mode ), state ); - } - - private Progression createProgression( Operation[] ops, Mode mode ) - { - return new Progression() - { - private int i; - - @Override - public long nextId() - { - while ( i < ops.length ) - { - Operation op = ops[i++]; - if ( op.fromDisk() || mode == FETCH ) - { - return op.id(); - } - } - return -1L; - } - - @Override - public Mode mode() - { - return mode; - } - }; - } - - void runAndVerify( Cursor cursor ) - { - Operation[] operations = Arrays.copyOf( ops, ops.length ); - if ( mode == APPEND ) - { - // push the AddedNodes at the end of the array during the check phase since if added in the cursor they - // are at the end of the stream - Operation tmp; - for ( int i = operations.length - 1; i >= 0; i-- ) - { - if ( operations[i] instanceof AddedNode ) - { - for ( int j = i; j < operations.length - 1 && !(operations[j + 1] instanceof AddedNode); j++ ) - { - tmp = operations[j]; - operations[j] = operations[j + 1]; - operations[j + 1] = tmp; - } - } - } - } - - state = state == null ? new TxState() : state; - - int injectedId = 100_000; - for ( Operation op : operations ) - { - // simulates tx changes after the cursor has been initialized, the cursor should no return this ids! - state.nodeDoCreate( injectedId++ ); - if ( !(op instanceof DeletedNode) ) - { - assertTrue( cursor.next() ); - op.check( cursor.get() ); - } - } - - assertNoMoreElements( cursor ); - cursor.close(); - assertNoMoreElements( cursor ); - } - - private static void assertNoMoreElements( Cursor cursor ) - { - assertFalse( cursor.next() ); - try - { - cursor.get(); - fail( "should have thrown" ); - } - catch ( IllegalStateException ex ) - { - // good - } - } - - @Override - public String toString() - { - return String.format( "{mode=%s, ops=%s}", mode, Arrays.toString( ops ) ); - } - } -} 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 2906fd6e9a200..a512e8ef2fd68 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 @@ -185,7 +185,7 @@ private void testDegreeByDirectionForDenseNodeWithPartiallyDeletedRelGroupChain( int loopRelCount = randomRelCount(); long nodeId = createNode( inRelCount, outRelCount, loopRelCount ); - NodeCursor cursor = newCursor( nodeId ); + StoreSingleNodeCursor cursor = newCursor( nodeId ); for ( TestRelType type : typesToDelete ) { @@ -219,7 +219,7 @@ private void testDegreeByDirectionForDenseNodeWithPartiallyDeletedRelChains( boo int loopRelCount = randomRelCount(); long nodeId = createNode( inRelCount, outRelCount, loopRelCount ); - NodeCursor cursor = newCursor( nodeId ); + StoreSingleNodeCursor cursor = newCursor( nodeId ); if ( modifyInChain ) { @@ -239,13 +239,12 @@ private void testDegreeByDirectionForDenseNodeWithPartiallyDeletedRelChains( boo assertEquals( inRelCount + outRelCount + loopRelCount, degreeForDirection( cursor, BOTH ) ); } - private int degreeForDirection( NodeCursor cursor, Direction direction ) + private int degreeForDirection( StoreSingleNodeCursor cursor, Direction direction ) { return disk .degreeRelationshipsInGroup( disk.newStatement(), cursor.id(), cursor.nextGroupId(), direction, null ); } - - private int degreeForDirectionAndType( NodeCursor cursor, Direction direction, int relType ) + private int degreeForDirectionAndType( StoreSingleNodeCursor cursor, Direction direction, int relType ) { return disk.degreeRelationshipsInGroup( disk.newStatement(), cursor.id(), cursor.nextGroupId(), direction, relType ); @@ -259,7 +258,7 @@ private void testDegreeByDirectionAndTypeForDenseNodeWithPartiallyDeletedRelGrou int loopRelCount = randomRelCount(); long nodeId = createNode( inRelCount, outRelCount, loopRelCount ); - NodeCursor cursor = newCursor( nodeId ); + StoreSingleNodeCursor cursor = newCursor( nodeId ); for ( TestRelType type : typesToDelete ) { @@ -301,7 +300,7 @@ private void testDegreeByDirectionAndTypeForDenseNodeWithPartiallyDeletedRelChai int loopRelCount = randomRelCount(); long nodeId = createNode( inRelCount, outRelCount, loopRelCount ); - NodeCursor cursor = newCursor( nodeId ); + StoreSingleNodeCursor cursor = newCursor( nodeId ); if ( modifyInChain ) { @@ -370,12 +369,12 @@ private void testRelationshipTypesForDenseNode( LongConsumer nodeChanger, Set relTypes( NodeCursor cursor ) + private Set relTypes( StoreSingleNodeCursor cursor ) { return mapToSet( disk.relationshipTypes( disk.newStatement(), cursor.get() ).iterator(), this::relTypeForId ); } @@ -388,7 +387,7 @@ private void testDegreesForDenseNodeWithPartiallyDeletedRelGroupChain( TestRelTy int loopRelCount = randomRelCount(); long nodeId = createNode( inRelCount, outRelCount, loopRelCount ); - NodeCursor cursor = newCursor( nodeId ); + StoreSingleNodeCursor cursor = newCursor( nodeId ); for ( TestRelType type : typesToDelete ) { @@ -436,7 +435,7 @@ private void testDegreesForDenseNodeWithPartiallyDeletedRelChains( boolean modif int loopRelCount = randomRelCount(); long nodeId = createNode( inRelCount, outRelCount, loopRelCount ); - NodeCursor cursor = newCursor( nodeId ); + StoreSingleNodeCursor cursor = newCursor( nodeId ); if ( modifyInChain ) { @@ -470,11 +469,11 @@ private Set degrees( NodeItem nodeItem ) } @SuppressWarnings( "unchecked" ) - private NodeCursor newCursor( long nodeId ) + private StoreSingleNodeCursor newCursor( long nodeId ) { - NodeCursor cursor = new NodeCursor( new NodeRecord( -1 ), mock( Consumer.class ), + StoreSingleNodeCursor cursor = new StoreSingleNodeCursor( new NodeRecord( -1 ), mock( Consumer.class ), new RecordCursors( resolveNeoStores() ), NO_LOCK_SERVICE ); - cursor.init( new SingleNodeProgression( nodeId ), null ); + cursor.init( nodeId, null ); assertTrue( cursor.next() ); return cursor; } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StoreSingleNodeCursorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StoreSingleNodeCursorTest.java new file mode 100644 index 0000000000000..24fe4982848d0 --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StoreSingleNodeCursorTest.java @@ -0,0 +1,64 @@ +/* + * 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.store.RecordCursor; +import org.neo4j.kernel.impl.store.RecordCursors; +import org.neo4j.kernel.impl.store.record.NodeRecord; +import org.neo4j.kernel.impl.store.record.RecordLoad; +import org.neo4j.storageengine.api.txstate.ReadableTransactionState; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.neo4j.kernel.impl.locking.LockService.NO_LOCK_SERVICE; + +public class StoreSingleNodeCursorTest +{ + private final ReadableTransactionState state = mock( ReadableTransactionState.class ); + private final RecordCursors recordCursors = mock( RecordCursors.class ); + private final StoreSingleNodeCursor cursor = + new StoreSingleNodeCursor( new NodeRecord(-1), i -> {}, recordCursors, NO_LOCK_SERVICE ); + + @Test + public void shouldNotLoopForeverWhenNodesAreAddedToTheTxState() throws Exception + { + // given + int nodeId = 42; + when( state.nodeIsDeletedInThisTx( nodeId ) ).thenReturn( false ); + when( state.nodeIsAddedInThisTx( nodeId ) ).thenReturn( true ); + @SuppressWarnings( "unchecked" ) + RecordCursor recordCursor = mock( RecordCursor.class ); + when( recordCursor.next( anyLong(), any( NodeRecord.class ), any( RecordLoad.class ) ) ).thenReturn( false ); + when( recordCursors.node() ).thenReturn( recordCursor ); + + // when + cursor.init( nodeId, state ); + + // then + assertTrue( cursor.next() ); + assertFalse( cursor.next() ); + } +} diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/NodeLabelsFieldTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/NodeLabelsFieldTest.java index 48dceead0e57d..5258fc0277c0e 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/NodeLabelsFieldTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/NodeLabelsFieldTest.java @@ -494,7 +494,7 @@ private long dynamicLabelsLongRepresentation( Iterable records ) return 0x8000000000L | Iterables.first( records ).getId(); } - public static long inlinedLabelsLongRepresentation( long... labelIds ) + private long inlinedLabelsLongRepresentation( long... labelIds ) { long header = (long) labelIds.length << 36; byte bitsPerLabel = (byte) (36 / labelIds.length);