diff --git a/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java b/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java index 7097348d7cbae..7db5a254b7440 100644 --- a/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java +++ b/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java @@ -71,6 +71,7 @@ import static org.neo4j.kernel.configuration.Settings.min; import static org.neo4j.kernel.configuration.Settings.options; import static org.neo4j.kernel.configuration.Settings.pathSetting; +import static org.neo4j.kernel.configuration.Settings.range; import static org.neo4j.kernel.configuration.Settings.setting; /** @@ -459,8 +460,17 @@ public class GraphDatabaseSettings implements LoadableConfig public static final Setting label_block_size = setting("unsupported.dbms.block_size.labels", INTEGER, "0", min( 0 ) ); - @Description("An identifier that uniquely identifies this graph database instance within this JVM. " + - "Defaults to an auto-generated number depending on how many instance are started in this JVM.") + @Description( "Specifies the size of id batches local to each transaction when committing. " + + "Committing a transaction which contains changes most often results in new data records being created. " + + "For each record a new id needs to be generated from an id generator. " + + "It's more efficient to allocate a batch of ids from the contended id generator, which the transaction " + + "holds and generates ids from while creating these new records. " + + "This setting specifies how big those batches are. " + + "Remaining ids are freed back to id generator on clean shutdown." ) + @Internal + public static final Setting record_id_batch_size = setting( "unsupported.dbms.record_id_batch_size", INTEGER, + "20", range( 1, 1_000 ) ); + @Internal public static final Setting forced_kernel_id = setting("unsupported.dbms.kernel_id", STRING, NO_DEFAULT, illegalValueMessage("has to be a valid kernel identifier", matches("[a-zA-Z0-9]*"))); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java index 8da15226542e8..3bcb329a2d8e5 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactions.java @@ -322,7 +322,7 @@ private void assertCurrentThreadIsNotBlockingNewTransactions() private class KernelTransactionImplementationFactory implements Factory { - private Set transactions; + private final Set transactions; KernelTransactionImplementationFactory( Set transactions ) { @@ -345,7 +345,7 @@ public KernelTransactionImplementation newInstance() private class GlobalKernelTransactionPool extends LinkedQueuePool { - private Set transactions; + private final Set transactions; GlobalKernelTransactionPool( Set transactions, Factory factory ) 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 e6151987d903a..4b11226e65b41 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 @@ -417,7 +417,7 @@ private Cursor relationshipGetPropertyCursor( KernelStatement stat @Override public long nodeCreate( KernelStatement state ) { - long nodeId = storeLayer.reserveNode(); + long nodeId = state.getStoreStatement().reserveNode(); state.txState().nodeDoCreate(nodeId); return nodeId; } @@ -452,7 +452,7 @@ public long relationshipCreate( KernelStatement state, { try ( Cursor endNode = nodeCursorById( state, endNodeId ) ) { - long id = storeLayer.reserveRelationship(); + long id = state.getStoreStatement().reserveRelationship(); state.txState().relationshipDoCreate( id, relationshipTypeId, startNode.get().id(), endNode.get().id() ); return id; } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllIdIterator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllIdIterator.java index 15e0d5042e59f..13c34c8e50cfa 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllIdIterator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/AllIdIterator.java @@ -21,11 +21,11 @@ import org.neo4j.kernel.impl.store.CommonAbstractStore; -class AllIdIterator extends HighIdAwareIterator> +public class AllIdIterator extends HighIdAwareIterator> { private long currentId; - AllIdIterator( CommonAbstractStore store ) + public AllIdIterator( CommonAbstractStore store ) { super( store ); } 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 2fd5b5f383541..727cacedf5489 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 @@ -447,18 +447,6 @@ public Cursor relationshipGetProperty( StorageStatement statement, return statement.acquireSinglePropertyCursor( relationship.nextPropertyId(), propertyKeyId, lock, assertOpen ); } - @Override - public long reserveNode() - { - return nodeStore.nextId(); - } - - @Override - public long reserveRelationship() - { - return relationshipStore.nextId(); - } - @Override public void releaseNode( long id ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSingleNodeCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSingleNodeCursor.java index 2be2c3030cd0c..76b02bcbf4969 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSingleNodeCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSingleNodeCursor.java @@ -52,7 +52,7 @@ public class StoreSingleNodeCursor implements Cursor, NodeItem private long nodeId = StatementConstants.NO_SUCH_NODE; private long[] labels; - StoreSingleNodeCursor( NodeRecord nodeRecord, Consumer instanceCache, + public StoreSingleNodeCursor( NodeRecord nodeRecord, Consumer instanceCache, RecordCursors recordCursors, LockService lockService ) { this.nodeRecord = nodeRecord; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSingleRelationshipCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSingleRelationshipCursor.java index 0b8f77ff91828..d3e1cd0ebcaf9 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSingleRelationshipCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreSingleRelationshipCursor.java @@ -35,7 +35,7 @@ public class StoreSingleRelationshipCursor extends StoreAbstractRelationshipCurs private final InstanceCache instanceCache; private long relationshipId = StatementConstants.NO_SUCH_RELATIONSHIP; - StoreSingleRelationshipCursor( RelationshipRecord relationshipRecord, + public StoreSingleRelationshipCursor( RelationshipRecord relationshipRecord, InstanceCache instanceCache, RecordCursors cursors, LockService lockService ) { super( relationshipRecord, cursors, lockService ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageCommandCreationContext.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageCommandCreationContext.java new file mode 100644 index 0000000000000..7d4c932bd27da --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageCommandCreationContext.java @@ -0,0 +1,92 @@ +/* + * 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.storageengine.impl.recordstorage; + +import org.neo4j.kernel.impl.store.NeoStores; +import org.neo4j.kernel.impl.store.StandardDynamicRecordAllocator; +import org.neo4j.kernel.impl.store.StoreType; +import org.neo4j.kernel.impl.store.id.RenewableBatchIdSequences; +import org.neo4j.kernel.impl.transaction.state.IntegrityValidator; +import org.neo4j.kernel.impl.transaction.state.Loaders; +import org.neo4j.kernel.impl.transaction.state.PropertyCreator; +import org.neo4j.kernel.impl.transaction.state.PropertyDeleter; +import org.neo4j.kernel.impl.transaction.state.PropertyTraverser; +import org.neo4j.kernel.impl.transaction.state.RecordChangeSet; +import org.neo4j.kernel.impl.transaction.state.RelationshipCreator; +import org.neo4j.kernel.impl.transaction.state.RelationshipDeleter; +import org.neo4j.kernel.impl.transaction.state.RelationshipGroupGetter; +import org.neo4j.kernel.impl.transaction.state.TransactionRecordState; +import org.neo4j.storageengine.api.CommandCreationContext; +import org.neo4j.storageengine.api.lock.ResourceLocker; + +/** + * Holds commit data structures for creating records in a {@link NeoStores}. + */ +public class RecordStorageCommandCreationContext implements CommandCreationContext +{ + private final NeoStores neoStores; + private final Loaders loaders; + private final RelationshipCreator relationshipCreator; + private final RelationshipDeleter relationshipDeleter; + private final PropertyCreator propertyCreator; + private final PropertyDeleter propertyDeleter; + private final RenewableBatchIdSequences idBatches; + + RecordStorageCommandCreationContext( NeoStores neoStores, int denseNodeThreshold, int idBatchSize ) + { + this.neoStores = neoStores; + this.idBatches = new RenewableBatchIdSequences( neoStores, idBatchSize ); + + this.loaders = new Loaders( neoStores ); + RelationshipGroupGetter relationshipGroupGetter = + new RelationshipGroupGetter( idBatches.idGenerator( StoreType.RELATIONSHIP_GROUP ) ); + this.relationshipCreator = new RelationshipCreator( relationshipGroupGetter, denseNodeThreshold ); + PropertyTraverser propertyTraverser = new PropertyTraverser(); + this.propertyDeleter = new PropertyDeleter( propertyTraverser ); + this.relationshipDeleter = new RelationshipDeleter( relationshipGroupGetter, propertyDeleter ); + this.propertyCreator = new PropertyCreator( + new StandardDynamicRecordAllocator( idBatches.idGenerator( StoreType.PROPERTY_STRING ), + neoStores.getPropertyStore().getStringStore().getRecordDataSize() ), + new StandardDynamicRecordAllocator( idBatches.idGenerator( StoreType.PROPERTY_ARRAY ), + neoStores.getPropertyStore().getArrayStore().getRecordDataSize() ), + idBatches.idGenerator( StoreType.PROPERTY ), + propertyTraverser ); + } + + public long nextId( StoreType storeType ) + { + return idBatches.nextId( storeType ); + } + + @Override + public void close() + { + this.idBatches.close(); + } + + public TransactionRecordState createTransactionRecordState( IntegrityValidator integrityValidator, long lastTransactionIdWhenStarted, + ResourceLocker locks ) + { + RecordChangeSet recordChangeSet = new RecordChangeSet( loaders ); + return new TransactionRecordState( neoStores, integrityValidator, + recordChangeSet, lastTransactionIdWhenStarted, locks, + relationshipCreator, relationshipDeleter, propertyCreator, propertyDeleter ); + } +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java index 446988c7ead91..42cb8f047591d 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageEngine.java @@ -60,7 +60,6 @@ import org.neo4j.kernel.impl.api.scan.LabelScanStoreProvider; import org.neo4j.kernel.impl.api.store.SchemaCache; import org.neo4j.kernel.impl.api.store.StorageLayer; -import org.neo4j.kernel.impl.api.store.StoreStatement; import org.neo4j.kernel.impl.cache.BridgingCacheAccess; import org.neo4j.kernel.impl.constraints.ConstraintSemantics; import org.neo4j.kernel.impl.core.CacheAccessBackDoor; @@ -87,14 +86,6 @@ import org.neo4j.kernel.impl.transaction.command.NeoStoreBatchTransactionApplier; import org.neo4j.kernel.impl.transaction.state.DefaultSchemaIndexProviderMap; import org.neo4j.kernel.impl.transaction.state.IntegrityValidator; -import org.neo4j.kernel.impl.transaction.state.Loaders; -import org.neo4j.kernel.impl.transaction.state.PropertyCreator; -import org.neo4j.kernel.impl.transaction.state.PropertyDeleter; -import org.neo4j.kernel.impl.transaction.state.PropertyTraverser; -import org.neo4j.kernel.impl.transaction.state.RecordChangeSet; -import org.neo4j.kernel.impl.transaction.state.RelationshipCreator; -import org.neo4j.kernel.impl.transaction.state.RelationshipDeleter; -import org.neo4j.kernel.impl.transaction.state.RelationshipGroupGetter; import org.neo4j.kernel.impl.transaction.state.TransactionRecordState; import org.neo4j.kernel.impl.transaction.state.storeview.DynamicIndexStoreView; import org.neo4j.kernel.impl.transaction.state.storeview.NeoStoreIndexStoreView; @@ -154,13 +145,8 @@ public class RecordStorageEngine implements StorageEngine, Lifecycle private final PropertyPhysicalToLogicalConverter indexUpdatesConverter; private final Supplier storeStatementSupplier; private final IdController idController; - - // Immutable state for creating/applying commands - private final Loaders loaders; - private final RelationshipCreator relationshipCreator; - private final RelationshipDeleter relationshipDeleter; - private final PropertyCreator propertyCreator; - private final PropertyDeleter propertyDeleter; + private final int denseNodeThreshold; + private final int recordIdBatchSize; public RecordStorageEngine( File storeDir, @@ -233,16 +219,8 @@ public RecordStorageEngine( commandReaderFactory = new RecordStorageCommandReaderFactory(); indexUpdatesSync = new WorkSync<>( indexingService ); - // Immutable state for creating/applying commands - loaders = new Loaders( neoStores ); - RelationshipGroupGetter relationshipGroupGetter = - new RelationshipGroupGetter( neoStores.getRelationshipGroupStore() ); - relationshipCreator = new RelationshipCreator( relationshipGroupGetter, - config.get( GraphDatabaseSettings.dense_node_threshold ) ); - PropertyTraverser propertyTraverser = new PropertyTraverser(); - propertyDeleter = new PropertyDeleter( propertyTraverser ); - relationshipDeleter = new RelationshipDeleter( relationshipGroupGetter, propertyDeleter ); - propertyCreator = new PropertyCreator( neoStores.getPropertyStore(), propertyTraverser ); + denseNodeThreshold = config.get( GraphDatabaseSettings.dense_node_threshold ); + recordIdBatchSize = config.get( GraphDatabaseSettings.record_id_batch_size ); } catch ( Throwable failure ) { @@ -256,7 +234,8 @@ private Supplier storeStatementSupplier( NeoStores neoStores ) Supplier indexReaderFactory = () -> new IndexReaderFactory.Caching( indexingService ); LockService lockService = takePropertyReadLocks ? this.lockService : NO_LOCK_SERVICE; - return () -> new StoreStatement( neoStores, indexReaderFactory, labelScanStore::newReader, lockService ); + return () -> new StoreStatement( neoStores, indexReaderFactory, labelScanStore::newReader, lockService, + allocateCommandCreationContext() ); } @Override @@ -265,6 +244,12 @@ public StoreReadLayer storeReadLayer() return storeLayer; } + @Override + public RecordStorageCommandCreationContext allocateCommandCreationContext() + { + return new RecordStorageCommandCreationContext( neoStores, denseNodeThreshold, recordIdBatchSize ); + } + @Override public CommandReaderFactory commandReaderFactory() { @@ -283,10 +268,13 @@ public void createCommands( { if ( txState != null ) { - RecordChangeSet recordChangeSet = new RecordChangeSet( loaders ); - TransactionRecordState recordState = new TransactionRecordState( neoStores, integrityValidator, - recordChangeSet, lastTransactionIdWhenStarted, locks, - relationshipCreator, relationshipDeleter, propertyCreator, propertyDeleter ); + // We can make this cast here because we expected that the storageStatement passed in here comes from + // this storage engine itself, anything else is considered a bug. And we do know the inner workings + // of the storage statements that we create. + RecordStorageCommandCreationContext creationContext = + (RecordStorageCommandCreationContext) storageStatement.getCommandCreationContext(); + TransactionRecordState recordState = + creationContext.createTransactionRecordState( integrityValidator, lastTransactionIdWhenStarted, locks ); // Visit transaction state and populate these record state objects TxStateVisitor txStateVisitor = new TransactionToRecordStateVisitor( recordState, 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/storageengine/impl/recordstorage/StoreStatement.java similarity index 87% rename from community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StoreStatement.java rename to community/kernel/src/main/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/StoreStatement.java index 7204d084ec77f..65dcad104c4bc 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/storageengine/impl/recordstorage/StoreStatement.java @@ -17,7 +17,7 @@ * 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; +package org.neo4j.kernel.impl.storageengine.impl.recordstorage; import java.util.function.IntPredicate; import java.util.function.Supplier; @@ -27,6 +27,13 @@ import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException; import org.neo4j.kernel.api.schema.index.IndexDescriptor; import org.neo4j.kernel.impl.api.IndexReaderFactory; +import org.neo4j.kernel.impl.api.store.AllIdIterator; +import org.neo4j.kernel.impl.api.store.StoreIteratorRelationshipCursor; +import org.neo4j.kernel.impl.api.store.StoreNodeRelationshipCursor; +import org.neo4j.kernel.impl.api.store.StorePropertyCursor; +import org.neo4j.kernel.impl.api.store.StoreSingleNodeCursor; +import org.neo4j.kernel.impl.api.store.StoreSinglePropertyCursor; +import org.neo4j.kernel.impl.api.store.StoreSingleRelationshipCursor; import org.neo4j.kernel.impl.locking.Lock; import org.neo4j.kernel.impl.locking.LockService; import org.neo4j.kernel.impl.store.NeoStores; @@ -34,8 +41,10 @@ import org.neo4j.kernel.impl.store.RecordCursors; import org.neo4j.kernel.impl.store.RecordStore; import org.neo4j.kernel.impl.store.RelationshipStore; +import org.neo4j.kernel.impl.store.StoreType; import org.neo4j.kernel.impl.store.record.RelationshipGroupRecord; import org.neo4j.kernel.impl.util.InstanceCache; +import org.neo4j.storageengine.api.CommandCreationContext; import org.neo4j.storageengine.api.Direction; import org.neo4j.storageengine.api.NodeItem; import org.neo4j.storageengine.api.PropertyItem; @@ -66,6 +75,7 @@ public class StoreStatement implements StorageStatement private final RecordCursors recordCursors; private final Supplier labelScanStore; private final RecordStore relationshipGroupStore; + private final RecordStorageCommandCreationContext commandCreationContext; private IndexReaderFactory indexReaderFactory; private LabelScanReader labelScanReader; @@ -74,11 +84,13 @@ public class StoreStatement implements StorageStatement private boolean closed; public StoreStatement( NeoStores neoStores, Supplier indexReaderFactory, - Supplier labelScanReaderSupplier, LockService lockService ) + Supplier labelScanReaderSupplier, LockService lockService, + RecordStorageCommandCreationContext commandCreationContext ) { this.neoStores = neoStores; this.indexReaderFactorySupplier = indexReaderFactory; this.labelScanStore = labelScanReaderSupplier; + this.commandCreationContext = commandCreationContext; this.nodeStore = neoStores.getNodeStore(); this.relationshipStore = neoStores.getRelationshipStore(); this.relationshipGroupStore = neoStores.getRelationshipGroupStore(); @@ -203,6 +215,7 @@ public void close() assert !closed; closeSchemaResources(); recordCursors.close(); + commandCreationContext.close(); closed = true; } @@ -250,4 +263,22 @@ public RecordCursors recordCursors() { return recordCursors; } + + @Override + public CommandCreationContext getCommandCreationContext() + { + return commandCreationContext; + } + + @Override + public long reserveNode() + { + return commandCreationContext.nextId( StoreType.NODE ); + } + + @Override + public long reserveRelationship() + { + return commandCreationContext.nextId( StoreType.RELATIONSHIP ); + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/AbstractDynamicStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/AbstractDynamicStore.java index d95513051ac26..6ba66c1d0b7ff 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/AbstractDynamicStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/AbstractDynamicStore.java @@ -182,10 +182,7 @@ public static Pair readFullByteArrayFromHeavyRecords( @Override public DynamicRecord nextRecord() { - DynamicRecord record = new DynamicRecord( nextId() ); - record.setCreated(); - record.setInUse( true ); - return record; + return StandardDynamicRecordAllocator.allocateRecord( nextId() ); } public void allocateRecordsFromBytes( Collection target, byte[] src ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordStore.java index 493c91e53d972..3e8ca3f7a9500 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/RecordStore.java @@ -243,6 +243,8 @@ public interface RecordStore extends IdSequen */ void scanAllRecords( Visitor visitor ) throws EXCEPTION; + void freeId( long id ); + Predicate IN_USE = AbstractBaseRecord::inUse; class Delegator implements RecordStore @@ -391,6 +393,12 @@ public void scanAllRecords( Visitor v { actual.scanAllRecords( visitor ); } + + @Override + public void freeId( long id ) + { + actual.freeId( id ); + } } @SuppressWarnings( "unchecked" ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/StandardDynamicRecordAllocator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/StandardDynamicRecordAllocator.java new file mode 100644 index 0000000000000..6cbec9e5b9d61 --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/StandardDynamicRecordAllocator.java @@ -0,0 +1,55 @@ +/* + * 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.store; + +import org.neo4j.kernel.impl.store.id.IdSequence; +import org.neo4j.kernel.impl.store.record.DynamicRecord; + +public class StandardDynamicRecordAllocator implements DynamicRecordAllocator +{ + protected final IdSequence idGenerator; + private final int dataSize; + + public StandardDynamicRecordAllocator( IdSequence idGenerator, int dataSize ) + { + this.idGenerator = idGenerator; + this.dataSize = dataSize; + } + + @Override + public int getRecordDataSize() + { + return dataSize; + } + + @Override + public DynamicRecord nextRecord() + { + return allocateRecord( idGenerator.nextId() ); + } + + public static DynamicRecord allocateRecord( long id ) + { + DynamicRecord record = new DynamicRecord( id ); + record.setCreated(); + record.setInUse( true ); + return record; + } +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/StoreType.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/StoreType.java index 1168cd954f976..183a98ebcc6ee 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/StoreType.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/StoreType.java @@ -31,7 +31,7 @@ public enum StoreType { - NODE_LABEL( StoreFile.NODE_LABEL_STORE ) + NODE_LABEL( StoreFile.NODE_LABEL_STORE, true, false ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -40,7 +40,7 @@ public CommonAbstractStore open( NeoStores neoStores ) GraphDatabaseSettings.label_block_size ); } }, - NODE( StoreFile.NODE_STORE ) + NODE( StoreFile.NODE_STORE, true, false ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -48,7 +48,7 @@ public CommonAbstractStore open( NeoStores neoStores ) return neoStores.createNodeStore( getStoreName() ); } }, - PROPERTY_KEY_TOKEN_NAME( StoreFile.PROPERTY_KEY_TOKEN_NAMES_STORE ) + PROPERTY_KEY_TOKEN_NAME( StoreFile.PROPERTY_KEY_TOKEN_NAMES_STORE, true, true ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -57,7 +57,7 @@ public CommonAbstractStore open( NeoStores neoStores ) TokenStore.NAME_STORE_BLOCK_SIZE ); } }, - PROPERTY_KEY_TOKEN( StoreFile.PROPERTY_KEY_TOKEN_STORE ) + PROPERTY_KEY_TOKEN( StoreFile.PROPERTY_KEY_TOKEN_STORE, true, true ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -65,7 +65,7 @@ public CommonAbstractStore open( NeoStores neoStores ) return neoStores.createPropertyKeyTokenStore( getStoreName() ); } }, - PROPERTY_STRING( StoreFile.PROPERTY_STRING_STORE ) + PROPERTY_STRING( StoreFile.PROPERTY_STRING_STORE, true, false ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -74,7 +74,7 @@ public CommonAbstractStore open( NeoStores neoStores ) GraphDatabaseSettings.string_block_size ); } }, - PROPERTY_ARRAY( StoreFile.PROPERTY_ARRAY_STORE ) + PROPERTY_ARRAY( StoreFile.PROPERTY_ARRAY_STORE, true, false ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -83,7 +83,7 @@ public CommonAbstractStore open( NeoStores neoStores ) GraphDatabaseSettings.array_block_size ); } }, - PROPERTY( StoreFile.PROPERTY_STORE ) + PROPERTY( StoreFile.PROPERTY_STORE, true, false ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -91,7 +91,7 @@ public CommonAbstractStore open( NeoStores neoStores ) return neoStores.createPropertyStore( getStoreName() ); } }, - RELATIONSHIP( StoreFile.RELATIONSHIP_STORE ) + RELATIONSHIP( StoreFile.RELATIONSHIP_STORE, true, false ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -99,7 +99,7 @@ public CommonAbstractStore open( NeoStores neoStores ) return neoStores.createRelationshipStore( getStoreName() ); } }, - RELATIONSHIP_TYPE_TOKEN_NAME( StoreFile.RELATIONSHIP_TYPE_TOKEN_NAMES_STORE ) + RELATIONSHIP_TYPE_TOKEN_NAME( StoreFile.RELATIONSHIP_TYPE_TOKEN_NAMES_STORE, true, true ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -108,7 +108,7 @@ public CommonAbstractStore open( NeoStores neoStores ) TokenStore.NAME_STORE_BLOCK_SIZE ); } }, - RELATIONSHIP_TYPE_TOKEN( StoreFile.RELATIONSHIP_TYPE_TOKEN_STORE ) + RELATIONSHIP_TYPE_TOKEN( StoreFile.RELATIONSHIP_TYPE_TOKEN_STORE, true, true ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -116,7 +116,7 @@ public CommonAbstractStore open( NeoStores neoStores ) return neoStores.createRelationshipTypeTokenStore( getStoreName() ); } }, - LABEL_TOKEN_NAME( StoreFile.LABEL_TOKEN_NAMES_STORE ) + LABEL_TOKEN_NAME( StoreFile.LABEL_TOKEN_NAMES_STORE, true, true ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -125,7 +125,7 @@ public CommonAbstractStore open( NeoStores neoStores ) TokenStore.NAME_STORE_BLOCK_SIZE ); } }, - LABEL_TOKEN( StoreFile.LABEL_TOKEN_STORE ) + LABEL_TOKEN( StoreFile.LABEL_TOKEN_STORE, true, true ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -133,7 +133,7 @@ public CommonAbstractStore open( NeoStores neoStores ) return neoStores.createLabelTokenStore( getStoreName() ); } }, - SCHEMA( StoreFile.SCHEMA_STORE ) + SCHEMA( StoreFile.SCHEMA_STORE, true, true ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -141,7 +141,7 @@ public CommonAbstractStore open( NeoStores neoStores ) return neoStores.createSchemaStore( getStoreName() ); } }, - RELATIONSHIP_GROUP( StoreFile.RELATIONSHIP_GROUP_STORE ) + RELATIONSHIP_GROUP( StoreFile.RELATIONSHIP_GROUP_STORE, true, false ) { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -149,7 +149,7 @@ public CommonAbstractStore open( NeoStores neoStores ) return neoStores.createRelationshipGroupStore( getStoreName() ); } }, - COUNTS( null, false ) + COUNTS( null, false, false ) { @Override public CountsTracker open( final NeoStores neoStores ) @@ -176,7 +176,7 @@ public String getStoreName() return StoreFactory.COUNTS_STORE; } }, - META_DATA( StoreFile.NEO_STORE ) // Make sure this META store is last + META_DATA( StoreFile.NEO_STORE, true, true ) // Make sure this META store is last { @Override public CommonAbstractStore open( NeoStores neoStores ) @@ -186,17 +186,14 @@ public CommonAbstractStore open( NeoStores neoStores ) }; private final boolean recordStore; + private final boolean limitedIdStore; private final StoreFile storeFile; - StoreType( StoreFile storeFile ) - { - this( storeFile, true ); - } - - StoreType( StoreFile storeFile, boolean recordStore ) + StoreType( StoreFile storeFile, boolean recordStore, boolean limitedIdStore ) { this.storeFile = storeFile; this.recordStore = recordStore; + this.limitedIdStore = limitedIdStore; } abstract Object open( NeoStores neoStores ); @@ -206,6 +203,15 @@ public boolean isRecordStore() return recordStore; } + /** + * @return {@code true} to signal that this store has a quite limited id space and is more of a meta data store. + * Originally came about when adding transaction-local id batching, to avoid id generator batching on certain stores. + */ + public boolean isLimitedIdStore() + { + return limitedIdStore; + } + public String getStoreName() { return storeFile.fileNamePart(); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdRange.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdRange.java index f05544ac77675..a184afd67b183 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdRange.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdRange.java @@ -57,6 +57,16 @@ public String toString() Arrays.toString( defragIds ) + "]"; } + public int totalSize() + { + return defragIds.length + rangeLength; + } + + public IdRangeIterator iterator() + { + return new IdRangeIterator( this ); + } + @Override public boolean equals( Object o ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdRangeIterator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdRangeIterator.java index 9b0109f7d9f2e..2dd599663b523 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdRangeIterator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdRangeIterator.java @@ -21,15 +21,19 @@ import java.util.Arrays; +import org.neo4j.kernel.impl.store.id.validation.IdValidator; + +import static java.lang.Integer.min; + import static org.neo4j.collection.primitive.PrimitiveLongCollections.EMPTY_LONG_ARRAY; -public class IdRangeIterator +public class IdRangeIterator implements IdSequence { public static IdRangeIterator EMPTY_ID_RANGE_ITERATOR = new IdRangeIterator( new IdRange( EMPTY_LONG_ARRAY, 0, 0 ) ) { @Override - public long next() + public long nextId() { return VALUE_REPRESENTING_NULL; } @@ -48,7 +52,8 @@ public IdRangeIterator( IdRange idRange ) this.length = idRange.getRangeLength(); } - public long next() + @Override + public long nextId() { try { @@ -58,7 +63,7 @@ public long next() } long candidate = nextRangeCandidate(); - if ( candidate == IdGeneratorImpl.INTEGER_MINUS_ONE ) + if ( IdValidator.isReservedId( candidate ) ) { position++; candidate = nextRangeCandidate(); @@ -71,12 +76,44 @@ public long next() } } + @Override + public IdRange nextIdBatch( int size ) + { + int sizeLeft = size; + long[] rangeDefrag = EMPTY_LONG_ARRAY; + if ( position < defrag.length ) + { + // There are defragged ids to grab + int numberOfDefrags = min( sizeLeft, defrag.length - position ); + rangeDefrag = Arrays.copyOfRange( defrag, position, numberOfDefrags + position ); + position += numberOfDefrags; + sizeLeft -= numberOfDefrags; + } + + long rangeStart = 0; + int rangeLength = 0; + int rangeOffset = currentRangeOffset(); + int rangeAvailable = length - rangeOffset; + if ( sizeLeft > 0 && rangeAvailable > 0 ) + { + rangeStart = start + rangeOffset; + rangeLength = min( rangeAvailable, sizeLeft ); + position += rangeLength; + } + return new IdRange( rangeDefrag, rangeStart, rangeLength ); + } + private long nextRangeCandidate() { - int offset = position - defrag.length; + int offset = currentRangeOffset(); return (offset < length) ? (start + offset) : VALUE_REPRESENTING_NULL; } + private int currentRangeOffset() + { + return position - defrag.length; + } + @Override public String toString() { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/RenewableBatchIdSequence.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/RenewableBatchIdSequence.java new file mode 100644 index 0000000000000..89f203d1f724b --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/RenewableBatchIdSequence.java @@ -0,0 +1,85 @@ +/* + * 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.store.id; + +import java.util.function.LongConsumer; + +import org.neo4j.graphdb.Resource; + +import static org.neo4j.kernel.impl.store.id.IdRangeIterator.VALUE_REPRESENTING_NULL; + +/** + * An {@link IdSequence} which does internal batching by using another {@link IdSequence} as source of batches. + * Meant to be used by a single thread during its life time only. + */ +public class RenewableBatchIdSequence implements IdSequence, Resource +{ + private final IdSequence source; + private final int batchSize; + private final LongConsumer excessIdConsumer; + private IdSequence currentBatch; + private boolean closed; + + public RenewableBatchIdSequence( IdSequence source, int batchSize, LongConsumer excessIdConsumer ) + { + this.source = source; + this.batchSize = batchSize; + this.excessIdConsumer = excessIdConsumer; + } + + /** + * It's dangerous to potentially have multiple concurrent calls to close w/ regards to freeing excessive ids. + * This class isn't designed for concurrent access, but close can guard for it nonetheless. Only the first call + * to close will perform close. + */ + @Override + public synchronized void close() + { + if ( !closed && currentBatch != null ) + { + long id; + while ( (id = currentBatch.nextId()) != VALUE_REPRESENTING_NULL ) + { + excessIdConsumer.accept( id ); + } + currentBatch = null; + } + closed = true; + } + + @Override + public long nextId() + { + assert !closed; + + long id; + while ( currentBatch == null || (id = currentBatch.nextId()) == VALUE_REPRESENTING_NULL ) + { + currentBatch = source.nextIdBatch( batchSize ).iterator(); + } + return id; + } + + @Override + public IdRange nextIdBatch( int size ) + { + throw new UnsupportedOperationException( "Haven't been needed so far" ); + } +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/RenewableBatchIdSequences.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/RenewableBatchIdSequences.java new file mode 100644 index 0000000000000..6ec8bc706bfc9 --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/RenewableBatchIdSequences.java @@ -0,0 +1,75 @@ +/* + * 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.store.id; + +import org.neo4j.graphdb.Resource; +import org.neo4j.kernel.impl.store.NeoStores; +import org.neo4j.kernel.impl.store.RecordStore; +import org.neo4j.kernel.impl.store.StoreType; +import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; + +public class RenewableBatchIdSequences implements Resource +{ + private final IdSequence[] types = new IdSequence[StoreType.values().length]; + + public RenewableBatchIdSequences( NeoStores stores, int batchSize ) + { + for ( StoreType type : StoreType.values() ) + { + if ( type.isRecordStore() ) + { + RecordStore store = stores.getRecordStore( type ); + if ( type.isLimitedIdStore() || batchSize == 1 ) + { + // This is a token store or otherwise meta-data store, so let's not add batching for it + types[type.ordinal()] = store; + } + else + { + // This is a normal record store where id batching is beneficial + types[type.ordinal()] = new RenewableBatchIdSequence( store, batchSize, store::freeId ); + } + } + } + } + + public long nextId( StoreType type ) + { + return idGenerator( type ).nextId(); + } + + public IdSequence idGenerator( StoreType type ) + { + return types[type.ordinal()]; + } + + @Override + public void close() + { + for ( StoreType type : StoreType.values() ) + { + IdSequence generator = idGenerator( type ); + if ( generator instanceof RenewableBatchIdSequence ) + { + ((RenewableBatchIdSequence)generator).close(); + } + } + } +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/RelationshipGroupGetter.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/RelationshipGroupGetter.java index c8d7f851c6fff..ee2a875e687a6 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/RelationshipGroupGetter.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/RelationshipGroupGetter.java @@ -19,7 +19,7 @@ */ package org.neo4j.kernel.impl.transaction.state; -import org.neo4j.kernel.impl.store.RecordStore; +import org.neo4j.kernel.impl.store.id.IdSequence; import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.kernel.impl.store.record.Record; import org.neo4j.kernel.impl.store.record.RelationshipGroupRecord; @@ -27,11 +27,11 @@ public class RelationshipGroupGetter { - private final RecordStore store; + private final IdSequence idGenerator; - public RelationshipGroupGetter( RecordStore store ) + public RelationshipGroupGetter( IdSequence idGenerator ) { - this.store = store; + this.idGenerator = idGenerator; } public RelationshipGroupPosition getRelationshipGroup( NodeRecord node, int type, @@ -69,7 +69,7 @@ public RecordProxy getOrCreateRelationsh if ( change == null ) { assert node.isDense() : "Node " + node + " should have been dense at this point"; - long id = store.nextId(); + long id = idGenerator.nextId(); change = relGroupRecords.create( id, type ); RelationshipGroupRecord record = change.forChangingData(); record.setInUse( true ); diff --git a/community/kernel/src/main/java/org/neo4j/storageengine/api/CommandCreationContext.java b/community/kernel/src/main/java/org/neo4j/storageengine/api/CommandCreationContext.java new file mode 100644 index 0000000000000..6e6d528da71bd --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/storageengine/api/CommandCreationContext.java @@ -0,0 +1,30 @@ +/* + * 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.storageengine.api; + +import org.neo4j.graphdb.Resource; + +/** + * A context which {@link StorageEngine} hands out to clients and which gets passed back in + * to calls about creating commands. + */ +public interface CommandCreationContext extends Resource +{ +} diff --git a/community/kernel/src/main/java/org/neo4j/storageengine/api/StorageEngine.java b/community/kernel/src/main/java/org/neo4j/storageengine/api/StorageEngine.java index 09846d0d03a25..302f189eeb28c 100644 --- a/community/kernel/src/main/java/org/neo4j/storageengine/api/StorageEngine.java +++ b/community/kernel/src/main/java/org/neo4j/storageengine/api/StorageEngine.java @@ -41,6 +41,14 @@ public interface StorageEngine */ StoreReadLayer storeReadLayer(); + /** + * @return a new {@link CommandCreationContext} meant to be kept for multiple calls to + * {@link #createCommands(Collection, ReadableTransactionState, StorageStatement, ResourceLocker, + * long)}. + * Must be {@link CommandCreationContext#close() closed} after used, before being discarded. + */ + CommandCreationContext allocateCommandCreationContext(); + /** * Generates a list of {@link StorageCommand commands} representing the changes in the given transaction state * ({@code state} and {@code legacyIndexTransactionState}. @@ -48,7 +56,6 @@ public interface StorageEngine * storage using {@link #apply(CommandsToApply, TransactionApplicationMode)}. * The reason this is separated like this is that the generated commands can be used for other things * than applying to storage, f.ex replicating to another storage engine. - * * @param target {@link Collection} to put {@link StorageCommand commands} into. * @param state {@link ReadableTransactionState} representing logical store changes to generate commands for. * @param storageStatement {@link StorageStatement} to use for reading store state during creation of commands. @@ -61,6 +68,7 @@ public interface StorageEngine * @param lastTransactionIdWhenStarted transaction id which was seen as last committed when this * transaction started, i.e. before any changes were made and before any data was read. * TODO Transitional (Collection), might be {@link Stream} or whatever. + * * @throws TransactionFailureException if command generation fails or some prerequisite of some command * didn't validate, for example if trying to delete a node that still has relationships. * @throws CreateConstraintFailureException if this transaction was set to create a constraint and that failed. 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 f821d662c3ab4..808fc17ca34b7 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 @@ -157,4 +157,24 @@ Cursor acquireSinglePropertyCursor( long propertyId, int propertyK * @return record cursors */ RecordCursors recordCursors(); + + /** + * Reserves a node id for future use to store a node. The reason for it being exposed here is that + * internal ids of nodes and relationships are publicly accessible all the way out to the user. + * This will likely change in the future though. + * + * @return a reserved node id for future use. + */ + long reserveNode(); + + /** + * Reserves a relationship id for future use to store a relationship. The reason for it being exposed here is that + * internal ids of nodes and relationships are publicly accessible all the way out to the user. + * This will likely change in the future though. + * + * @return a reserved relationship id for future use. + */ + long reserveRelationship(); + + CommandCreationContext getCommandCreationContext(); } 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 7cb2ab5796dad..de6b71df44763 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 @@ -285,25 +285,7 @@ Cursor relationshipGetProperty( StorageStatement statement, Relati int propertyKeyId, AssertOpen assertOpen ); /** - * Reserves a node id for future use to store a node. The reason for it being exposed here is that - * internal ids of nodes and relationships are publicly accessible all the way out to the user. - * This will likely change in the future though. - * - * @return a reserved node id for future use. - */ - long reserveNode(); - - /** - * Reserves a relationship id for future use to store a relationship. The reason for it being exposed here is that - * internal ids of nodes and relationships are publicly accessible all the way out to the user. - * This will likely change in the future though. - * - * @return a reserved relationship id for future use. - */ - long reserveRelationship(); - - /** - * Releases a previously {@link #reserveNode() reserved} node id if it turns out to not actually being used, + * Releases a previously {@link StorageStatement#reserveNode() reserved} node id if it turns out to not actually being used, * for example in the event of a transaction rolling back. * * @param id reserved node id to release. @@ -311,7 +293,7 @@ Cursor relationshipGetProperty( StorageStatement statement, Relati void releaseNode( long id ); /** - * Releases a previously {@link #reserveRelationship() reserved} relationship id if it turns out to not + * Releases a previously {@link StorageStatement#reserveRelationship() reserved} relationship id if it turns out to not * actually being used, for example in the event of a transaction rolling back. * * @param id reserved relationship id to release. diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/PropertyEncoderStep.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/PropertyEncoderStep.java index 2e303ed3730e4..b296e4138fa02 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/PropertyEncoderStep.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/PropertyEncoderStep.java @@ -139,7 +139,7 @@ protected void process( Batch batch, BatchSender sender ) private static IdRangeIterator idRange( int size, IdSequence idSource ) { - return size > 0 ? new IdRangeIterator( idSource.nextIdBatch( size ) ) : IdRangeIterator.EMPTY_ID_RANGE_ITERATOR; + return size > 0 ? idSource.nextIdBatch( size ).iterator() : IdRangeIterator.EMPTY_ID_RANGE_ITERATOR; } private static void reassignPropertyIds( InputEntity input, PrimitiveRecord record, PropertyRecord[] propertyRecords, @@ -165,7 +165,7 @@ private static void reassignPropertyIds( InputEntity input, PrimitiveRecord reco private static long reassignPropertyRecordIds( PrimitiveRecord record, IdRangeIterator ids, PropertyRecord[] propertyRecords ) { - long newId = ids.next(); + long newId = ids.nextId(); long firstId = newId; PropertyRecord prev = null; for ( PropertyRecord propertyRecord : propertyRecords ) @@ -174,7 +174,7 @@ private static long reassignPropertyRecordIds( PrimitiveRecord record, IdRangeIt propertyRecord.setId( newId ); if ( !Record.NO_NEXT_PROPERTY.is( propertyRecord.getNextProp() ) ) { - propertyRecord.setNextProp( newId = ids.next() ); + propertyRecord.setNextProp( newId = ids.nextId() ); } if ( prev != null ) { @@ -214,7 +214,7 @@ private static void reassignDynamicRecordIds( IdRangeIterator stringRecordsIds, private static void reassignDynamicRecordIds( PropertyBlock block, PropertyType type, IdRangeIterator ids ) { Iterator dynamicRecords = block.getValueRecords().iterator(); - long newId = ids.next(); + long newId = ids.nextId(); block.getValueBlocks()[0] = PropertyStore.singleBlockLongValue( block.getKeyIndexId(), type, newId ); while ( dynamicRecords.hasNext() ) { @@ -222,7 +222,7 @@ private static void reassignDynamicRecordIds( PropertyBlock block, PropertyType dynamicRecord.setId( newId ); if ( dynamicRecords.hasNext() ) { - dynamicRecord.setNextBlock( newId = ids.next() ); + dynamicRecord.setNextBlock( newId = ids.nextId() ); } } } diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/RelativeIdRecordAllocator.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/RelativeIdRecordAllocator.java index 8f44de38c2ef2..f205534e7fe96 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/RelativeIdRecordAllocator.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/RelativeIdRecordAllocator.java @@ -20,46 +20,35 @@ package org.neo4j.unsafe.impl.batchimport; import org.neo4j.kernel.impl.store.DynamicRecordAllocator; +import org.neo4j.kernel.impl.store.StandardDynamicRecordAllocator; import org.neo4j.kernel.impl.store.record.DynamicRecord; +import org.neo4j.unsafe.impl.batchimport.store.BatchingIdSequence; /** * {@link DynamicRecordAllocator} that allocates new {@link DynamicRecord dynamic records} which has, * relative ids, {@link #initialize() starting} at 0, ignoring any global id generator. This is because when using * this allocator it is assumed that the ids are re-assigned later anyway. */ -public class RelativeIdRecordAllocator implements DynamicRecordAllocator +public class RelativeIdRecordAllocator extends StandardDynamicRecordAllocator { - private final int dataSize; - private long id; - public RelativeIdRecordAllocator( int dataSize ) { - this.dataSize = dataSize; + super( new BatchingIdSequence(), dataSize ); } public RelativeIdRecordAllocator initialize() { - this.id = 0; + idSequence().reset(); return this; } - @Override - public int getRecordDataSize() + private BatchingIdSequence idSequence() { - return dataSize; + return (BatchingIdSequence) idGenerator; } public long peek() { - return id; - } - - @Override - public DynamicRecord nextRecord() - { - DynamicRecord record = new DynamicRecord( id++ ); - record.setInUse( true ); - record.setCreated(); - return record; + return idSequence().peek(); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionTestBase.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionTestBase.java index 8500eff405e13..855881559ea2b 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionTestBase.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionTestBase.java @@ -32,13 +32,13 @@ import org.neo4j.kernel.api.security.SecurityContext; import org.neo4j.kernel.api.txstate.LegacyIndexTransactionState; import org.neo4j.kernel.configuration.Config; -import org.neo4j.kernel.impl.api.store.StoreStatement; import org.neo4j.kernel.impl.factory.CanWrite; import org.neo4j.kernel.impl.locking.LockTracer; import org.neo4j.kernel.impl.locking.Locks; import org.neo4j.kernel.impl.locking.NoOpClient; import org.neo4j.kernel.impl.locking.SimpleStatementLocks; import org.neo4j.kernel.impl.locking.StatementLocks; +import org.neo4j.kernel.impl.storageengine.impl.recordstorage.StoreStatement; import org.neo4j.kernel.impl.store.MetaDataStore; import org.neo4j.kernel.impl.store.NeoStores; import org.neo4j.kernel.impl.transaction.TransactionHeaderInformationFactory; @@ -95,9 +95,11 @@ public void before() throws Exception when( neoStores.getMetaDataStore() ).thenReturn( metaDataStore ); when( storageEngine.storeReadLayer() ).thenReturn( readLayer ); doAnswer( invocation -> ((Collection) invocation.getArguments()[0]).add( null ) ) - .when( storageEngine ).createCommands( anyCollectionOf( StorageCommand.class ), - any( ReadableTransactionState.class ), any( StorageStatement.class ), - any( ResourceLocker.class ), anyLong() ); + .when( storageEngine ).createCommands( + anyCollectionOf( StorageCommand.class ), + any( ReadableTransactionState.class ), + any( StorageStatement.class ), any( ResourceLocker.class ), + anyLong() ); } public KernelTransactionImplementation newTransaction( long transactionTimeoutMillis ) diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexPopulationJobTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexPopulationJobTest.java index 4a02bc89da31c..4d31e05baac40 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexPopulationJobTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexPopulationJobTest.java @@ -38,6 +38,7 @@ import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.Transaction; +import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.helpers.collection.MapUtil; import org.neo4j.helpers.collection.Pair; import org.neo4j.helpers.collection.Visitor; @@ -119,7 +120,8 @@ public class IndexPopulationJobTest @Before public void before() throws Exception { - db = (GraphDatabaseAPI) new TestGraphDatabaseFactory().newImpermanentDatabase(); + db = (GraphDatabaseAPI) new TestGraphDatabaseFactory().newImpermanentDatabaseBuilder() + .setConfig( GraphDatabaseSettings.record_id_batch_size, "1" ).newGraphDatabase(); kernel = db.getDependencyResolver().resolveDependency( KernelAPI.class ); stateHolder = new KernelSchemaStateStore( NullLogProvider.getInstance() ); indexStoreView = indexStoreView(); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/IndexQueryTransactionStateTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/IndexQueryTransactionStateTest.java index 3da2532b444c7..c68abfefc2909 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/IndexQueryTransactionStateTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/IndexQueryTransactionStateTest.java @@ -41,9 +41,9 @@ import org.neo4j.kernel.impl.api.StatementOperationsTestHelper; import org.neo4j.kernel.impl.api.legacyindex.InternalAutoIndexing; import org.neo4j.kernel.impl.api.operations.EntityOperations; -import org.neo4j.kernel.impl.api.store.StoreStatement; import org.neo4j.kernel.impl.constraints.StandardConstraintSemantics; import org.neo4j.kernel.impl.index.LegacyIndexStore; +import org.neo4j.kernel.impl.storageengine.impl.recordstorage.StoreStatement; import org.neo4j.storageengine.api.NodeItem; import org.neo4j.storageengine.api.StoreReadLayer; import org.neo4j.storageengine.api.schema.IndexReader; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/LabelTransactionStateTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/LabelTransactionStateTest.java index 080527c16ed33..71fc4030f6379 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/LabelTransactionStateTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/LabelTransactionStateTest.java @@ -40,8 +40,8 @@ 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.StoreStatement; import org.neo4j.kernel.impl.index.LegacyIndexStore; +import org.neo4j.kernel.impl.storageengine.impl.recordstorage.StoreStatement; import org.neo4j.storageengine.api.NodeItem; import org.neo4j.storageengine.api.StoreReadLayer; 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 eaf9eb9856f9a..238bc533a1a1d 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 @@ -42,8 +42,8 @@ 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.StoreStatement; import org.neo4j.kernel.impl.index.LegacyIndexStore; +import org.neo4j.kernel.impl.storageengine.impl.recordstorage.StoreStatement; import org.neo4j.storageengine.api.StoreReadLayer; import static org.junit.Assert.assertEquals; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/StateHandlingStatementOperationsTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/StateHandlingStatementOperationsTest.java index a8aedb5efa2f9..b0075c56e6ccc 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/StateHandlingStatementOperationsTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/StateHandlingStatementOperationsTest.java @@ -49,8 +49,8 @@ import org.neo4j.kernel.impl.api.KernelStatement; import org.neo4j.kernel.impl.api.StateHandlingStatementOperations; import org.neo4j.kernel.impl.api.legacyindex.InternalAutoIndexing; -import org.neo4j.kernel.impl.api.store.StoreStatement; import org.neo4j.kernel.impl.index.LegacyIndexStore; +import org.neo4j.kernel.impl.storageengine.impl.recordstorage.StoreStatement; import org.neo4j.kernel.impl.util.diffsets.DiffSets; import org.neo4j.storageengine.api.NodeItem; import org.neo4j.storageengine.api.PropertyItem; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/TestCrashWithRebuildSlow.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/TestCrashWithRebuildSlow.java index 23c9ea231c2c9..1b1c2cb5a56de 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/TestCrashWithRebuildSlow.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/TestCrashWithRebuildSlow.java @@ -56,6 +56,7 @@ import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; + import static org.neo4j.kernel.configuration.Settings.FALSE; import static org.neo4j.test.mockito.matcher.Neo4jMatchers.hasProperty; import static org.neo4j.test.mockito.matcher.Neo4jMatchers.inTx; @@ -76,7 +77,9 @@ public void crashAndRebuildSlowWithDynamicStringDeletions() throws Exception { File storeDir = new File( "dir" ).getAbsoluteFile(); final GraphDatabaseAPI db = (GraphDatabaseAPI) new TestGraphDatabaseFactory() - .setFileSystem( fs.get() ).newImpermanentDatabase( storeDir ); + .setFileSystem( fs.get() ).newImpermanentDatabaseBuilder( storeDir ) + .setConfig( GraphDatabaseSettings.record_id_batch_size, "1" ) + .newGraphDatabase(); List deletedNodeIds = produceNonCleanDefraggedStringStore( db ); Map highIdsBeforeCrash = getHighIds( db ); @@ -158,7 +161,8 @@ private static List produceNonCleanDefraggedStringStore( GraphDatabaseServ nodes.add( node ); if ( previous != null ) { - previous.createRelationshipTo( node, MyRelTypes.TEST ); + Relationship rel = previous.createRelationshipTo( node, MyRelTypes.TEST ); + System.out.println( rel ); } previous = node; } 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 3f521ac8ce71a..419274d75d88d 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 @@ -42,11 +42,11 @@ 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.StoreStatement; import org.neo4j.kernel.impl.core.NodeProxy; import org.neo4j.kernel.impl.core.RelationshipProxy; import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge; import org.neo4j.kernel.impl.locking.Lock; +import org.neo4j.kernel.impl.storageengine.impl.recordstorage.StoreStatement; import org.neo4j.storageengine.api.StoreReadLayer; import static java.util.Arrays.asList; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StoreStatementTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/StoreStatementTest.java similarity index 92% rename from community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StoreStatementTest.java rename to community/kernel/src/test/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/StoreStatementTest.java index 5e622e5264194..f68437002687a 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StoreStatementTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/storageengine/impl/recordstorage/StoreStatementTest.java @@ -17,7 +17,7 @@ * 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; +package org.neo4j.kernel.impl.storageengine.impl.recordstorage; import org.junit.Test; @@ -44,7 +44,7 @@ public void shouldCloseOpenedLabelScanReader() throws Exception when( scanStore.get() ).thenReturn( scanReader ); StoreStatement statement = new StoreStatement( MockedNeoStores.basicMockedNeoStores(), mock( Supplier.class ), - scanStore, LockService.NO_LOCK_SERVICE ); + scanStore, LockService.NO_LOCK_SERVICE, mock( RecordStorageCommandCreationContext.class ) ); statement.acquire(); // when diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestPropertyBlocks.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestPropertyBlocks.java index 05c3b8df0172d..7ffd0418b6f43 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestPropertyBlocks.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestPropertyBlocks.java @@ -590,7 +590,7 @@ private void testYoyoArrayBase( boolean withNewTx ) long recordsInUseAtStart = propertyRecordsInUse(); long valueRecordsInUseAtStart = dynamicArrayRecordsInUse(); - List theYoyoData = new ArrayList(); + List theYoyoData = new ArrayList<>(); for ( int i = 0; i < PropertyType.getPayloadSizeLongs() - 1; i++ ) { theYoyoData.add( 1L << 63 ); @@ -756,14 +756,10 @@ public void deleteNodeWithNewPropertyRecordShouldFreeTheNewRecord() throws Excep node.setProperty( "three", 3 ); node.setProperty( "four", 4 ); newTransaction(); - assertEquals( "Invalid assumption: property record count", propcount + 1, - getIdGenerator( IdType.PROPERTY ).getNumberOfIdsInUse() ); - assertEquals( "Invalid assumption: property record count", propcount + 1, - getIdGenerator( IdType.PROPERTY ).getNumberOfIdsInUse() ); + assertEquals( "Invalid assumption: property record count", propcount + 1, propertyRecordsInUse() ); node.setProperty( "final", 666 ); newTransaction(); - assertEquals( "Invalid assumption: property record count", propcount + 2, - getIdGenerator( IdType.PROPERTY ).getNumberOfIdsInUse() ); + assertEquals( "Invalid assumption: property record count", propcount + 2, propertyRecordsInUse() ); node.delete(); commit(); assertEquals( "All property records should be freed", propcount, propertyRecordsInUse() ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/IdRangeIteratorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/IdRangeIteratorTest.java index 2d454ad352b78..de5638e81b5e4 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/IdRangeIteratorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/IdRangeIteratorTest.java @@ -27,6 +27,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.neo4j.collection.primitive.PrimitiveLongCollections.EMPTY_LONG_ARRAY; +import static org.neo4j.kernel.impl.store.id.IdRangeIterator.VALUE_REPRESENTING_NULL; + public class IdRangeIteratorTest { @Test @@ -34,16 +37,16 @@ public void shouldReturnValueRepresentingNullIfWeExhaustIdRange() throws Excepti { // given int rangeLength = 1024; - IdRangeIterator iterator = new IdRangeIterator( new IdRange( new long[]{}, 0, rangeLength ) ); + IdRangeIterator iterator = new IdRange( new long[]{}, 0, rangeLength ).iterator(); // when for ( int i = 0; i < rangeLength; i++ ) { - iterator.next(); + iterator.nextId(); } // then - assertEquals( IdRangeIterator.VALUE_REPRESENTING_NULL, iterator.next() ); + assertEquals( IdRangeIterator.VALUE_REPRESENTING_NULL, iterator.nextId() ); } @Test @@ -51,13 +54,13 @@ public void shouldNotHaveAnyGaps() throws Exception { // given int rangeLength = 1024; - IdRangeIterator iterator = new IdRangeIterator( new IdRange( new long[]{}, 0, rangeLength ) ); + IdRangeIterator iterator = new IdRange( new long[]{}, 0, rangeLength ).iterator(); // when Set seenIds = new HashSet<>(); for ( int i = 0; i < rangeLength; i++ ) { - seenIds.add( iterator.next() ); + seenIds.add( iterator.nextId() ); if ( i > 0 ) { // then @@ -71,12 +74,129 @@ public void shouldUseDefragIdsFirst() throws Exception { // given int rangeLength = 1024; - IdRangeIterator iterator = new IdRangeIterator( new IdRange( new long[]{7,8,9}, 1024, rangeLength ) ); + IdRangeIterator iterator = new IdRange( new long[] {7, 8, 9}, 1024, rangeLength ).iterator(); + + // then + assertEquals( 7, iterator.nextId() ); + assertEquals( 8, iterator.nextId() ); + assertEquals( 9, iterator.nextId() ); + assertEquals( 1024, iterator.nextId() ); + } + + @Test + public void shouldGetNextIdBatchFromOnlyDefragIds() throws Exception + { + // given + IdRangeIterator iterator = new IdRange( new long[] {1, 2, 3, 4, 5, 6}, 7, 0 ).iterator(); + + // when + IdRangeIterator subRange = iterator.nextIdBatch( 5 ).iterator(); + + // then + assertEquals( 6, iterator.nextId() ); + for ( long i = 0; i < 5; i++ ) + { + assertEquals( 1 + i, subRange.nextId() ); + } + assertEquals( VALUE_REPRESENTING_NULL, subRange.nextId() ); + } + + @Test + public void shouldGetNextIdBatchFromOnlyDefragIdsWhenSomeDefragIdsHaveAlreadyBeenReturned() throws Exception + { + // given + IdRangeIterator iterator = new IdRange( new long[] {1, 2, 3, 4, 5, 6}, 7, 0 ).iterator(); + iterator.nextId(); + iterator.nextId(); + + // when + IdRangeIterator subRange = iterator.nextIdBatch( 3 ).iterator(); + + // then + assertEquals( 6, iterator.nextId() ); + for ( long i = 0; i < 3; i++ ) + { + assertEquals( 3 + i, subRange.nextId() ); + } + assertEquals( VALUE_REPRESENTING_NULL, subRange.nextId() ); + } + + @Test + public void shouldGetNextIdBatchFromSomeDefragAndSomeRangeIds() throws Exception + { + // given + IdRangeIterator iterator = new IdRange( new long[] {1, 2, 3}, 10, 5 ).iterator(); + iterator.nextId(); + + // when + IdRangeIterator subRange = iterator.nextIdBatch( 5 ).iterator(); + + // then + assertEquals( 13, iterator.nextId() ); + assertEquals( 2, subRange.nextId() ); + assertEquals( 3, subRange.nextId() ); + assertEquals( 10, subRange.nextId() ); + assertEquals( 11, subRange.nextId() ); + assertEquals( 12, subRange.nextId() ); + assertEquals( VALUE_REPRESENTING_NULL, subRange.nextId() ); + } + + @Test + public void shouldGetNextIdBatchFromSomeRangeIds() throws Exception + { + // given + IdRangeIterator iterator = new IdRange( EMPTY_LONG_ARRAY, 0, 20 ).iterator(); + iterator.nextId(); + + // when + IdRangeIterator subRange = iterator.nextIdBatch( 5 ).iterator(); + + // then + assertEquals( 6, iterator.nextId() ); + assertEquals( 1, subRange.nextId() ); + assertEquals( 2, subRange.nextId() ); + assertEquals( 3, subRange.nextId() ); + assertEquals( 4, subRange.nextId() ); + assertEquals( 5, subRange.nextId() ); + assertEquals( VALUE_REPRESENTING_NULL, subRange.nextId() ); + + // when + subRange = iterator.nextIdBatch( 2 ).iterator(); + + // then + assertEquals( 9, iterator.nextId() ); + assertEquals( 7, subRange.nextId() ); + assertEquals( 8, subRange.nextId() ); + assertEquals( VALUE_REPRESENTING_NULL, subRange.nextId() ); + } + + @Test + public void shouldGetNextIdBatchFromSomeRangeIdsWhenThereAreUsedDefragIds() throws Exception + { + // given + IdRangeIterator iterator = new IdRange( new long[] {0, 1, 2}, 3, 10 ).iterator(); + iterator.nextId(); + iterator.nextId(); + iterator.nextId(); + + // when + IdRangeIterator subRange = iterator.nextIdBatch( 3 ).iterator(); + + // then + assertEquals( 6, iterator.nextId() ); + assertEquals( 3, subRange.nextId() ); + assertEquals( 4, subRange.nextId() ); + assertEquals( 5, subRange.nextId() ); + assertEquals( VALUE_REPRESENTING_NULL, subRange.nextId() ); + + // when + subRange = iterator.nextIdBatch( 3 ).iterator(); // then - assertEquals(7, iterator.next()); - assertEquals(8, iterator.next()); - assertEquals(9, iterator.next()); - assertEquals(1024, iterator.next()); + assertEquals( 10, iterator.nextId() ); + assertEquals( 7, subRange.nextId() ); + assertEquals( 8, subRange.nextId() ); + assertEquals( 9, subRange.nextId() ); + assertEquals( VALUE_REPRESENTING_NULL, subRange.nextId() ); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/RenewableBatchIdSequenceTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/RenewableBatchIdSequenceTest.java new file mode 100644 index 0000000000000..9dcfe52a95356 --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/RenewableBatchIdSequenceTest.java @@ -0,0 +1,180 @@ +/* + * 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.store.id; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import static java.util.Arrays.asList; + +import static org.neo4j.collection.primitive.PrimitiveLongCollections.EMPTY_LONG_ARRAY; + +public class RenewableBatchIdSequenceTest +{ + public static final int BATCH_SIZE = 5; + + private final IdSource idSource = new IdSource(); + private final List excessIds = new ArrayList<>(); + private final RenewableBatchIdSequence ids = new RenewableBatchIdSequence( idSource, BATCH_SIZE, excessIds::add ); + + @Test + public void shouldRequestIdBatchFromSourceOnFirstCall() throws Exception + { + // given + assertEquals( 0, idSource.calls ); + + // when/then + assertEquals( 0, ids.nextId() ); + assertEquals( 1, idSource.calls ); + for ( int i = 1; i < BATCH_SIZE; i++ ) + { + assertEquals( i, ids.nextId() ); + assertEquals( 1, idSource.calls ); + } + } + + @Test + public void shouldRequestIdBatchFromSourceOnDepletingCurrent() throws Exception + { + // given + assertEquals( 0, idSource.calls ); + for ( int i = 0; i < BATCH_SIZE; i++ ) + { + assertEquals( i, ids.nextId() ); + } + assertEquals( 1, idSource.calls ); + + // when + long firstIdOfNextBatch = ids.nextId(); + + // then + assertEquals( BATCH_SIZE, firstIdOfNextBatch ); + assertEquals( 2, idSource.calls ); + } + + @Test + public void shouldGiveBackExcessIdsOnClose() throws Exception + { + // given + for ( int i = 0; i < BATCH_SIZE / 2; i++ ) + { + ids.nextId(); + } + + // when + ids.close(); + + // then + assertEquals( BATCH_SIZE - BATCH_SIZE / 2, excessIds.size() ); + for ( long i = BATCH_SIZE / 2; i < BATCH_SIZE; i++ ) + { + assertTrue( excessIds.contains( i ) ); + } + } + + @Test + public void shouldHandleCloseWithNoCurrentBatch() throws Exception + { + // when + ids.close(); + + // then + assertTrue( excessIds.isEmpty() ); + } + + @Test + public void shouldOnlyCloseOnce() throws Exception + { + // given + for ( int i = 0; i < BATCH_SIZE / 2; i++ ) + { + ids.nextId(); + } + + // when + ids.close(); + + // then + for ( long i = BATCH_SIZE / 2; i < BATCH_SIZE; i++ ) + { + assertTrue( excessIds.remove( i ) ); + } + + // and when closing one more time + ids.close(); + + // then + assertTrue( excessIds.isEmpty() ); + } + + @Test + public void shouldContinueThroughEmptyIdBatch() throws Exception + { + // given + IdSequence idSource = mock( IdSequence.class ); + Iterator ranges = asList( + new IdRange( EMPTY_LONG_ARRAY, 0, BATCH_SIZE ), + new IdRange( EMPTY_LONG_ARRAY, BATCH_SIZE, 0 ), + new IdRange( EMPTY_LONG_ARRAY, BATCH_SIZE, BATCH_SIZE ) ).iterator(); + when( idSource.nextIdBatch( anyInt() ) ).thenAnswer( invocation -> ranges.next() ); + RenewableBatchIdSequence ids = new RenewableBatchIdSequence( idSource, BATCH_SIZE, excessIds::add ); + + // when/then + for ( long expectedId = 0; expectedId < BATCH_SIZE * 2; expectedId++ ) + { + assertEquals( expectedId, ids.nextId() ); + } + } + + private static class IdSource implements IdSequence + { + int calls; + long nextId; + + @Override + public IdRange nextIdBatch( int batchSize ) + { + calls++; + try + { + return new IdRange( EMPTY_LONG_ARRAY, nextId, batchSize ); + } + finally + { + nextId += batchSize; + } + } + + @Override + public long nextId() + { + throw new UnsupportedOperationException( "Should not be used" ); + } + } +} diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/ReuseExcessBatchIdsOnRestartIT.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/ReuseExcessBatchIdsOnRestartIT.java new file mode 100644 index 0000000000000..6c82dda4c0f41 --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/ReuseExcessBatchIdsOnRestartIT.java @@ -0,0 +1,125 @@ +/* + * 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.store.id; + +import org.junit.Rule; +import org.junit.Test; + +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import org.neo4j.graphdb.Node; +import org.neo4j.graphdb.Transaction; +import org.neo4j.graphdb.factory.GraphDatabaseSettings; +import org.neo4j.test.rule.DatabaseRule; +import org.neo4j.test.rule.EmbeddedDatabaseRule; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static java.lang.Math.toIntExact; +import static java.util.concurrent.TimeUnit.SECONDS; + +public class ReuseExcessBatchIdsOnRestartIT +{ + @Rule + public final DatabaseRule db = new EmbeddedDatabaseRule(); + + // Knowing that ids are grabbed in batches internally we only create one node and later assert + // that the excess ids that were only grabbed, but not used can be reused. + @Test + public void shouldReuseExcessBatchIdsWhichWerentUsedBeforeClose() throws Exception + { + // given + Node firstNode; + try ( Transaction tx = db.beginTx() ) + { + firstNode = db.createNode(); + tx.success(); + } + + // when + db.restartDatabase(); + + Node secondNode; + try ( Transaction tx = db.beginTx() ) + { + secondNode = db.createNode(); + tx.success(); + } + + // then + assertEquals( firstNode.getId() + 1, secondNode.getId() ); + } + + @Test( timeout = 30_000 ) + public void shouldBeAbleToReuseAllIdsInConcurrentCommitsWithRestart() throws Exception + { + // given + int threads = Runtime.getRuntime().availableProcessors(); + int batchSize = Integer.parseInt( GraphDatabaseSettings.record_id_batch_size.getDefaultValue() ); + ExecutorService executor = Executors.newFixedThreadPool( threads ); + boolean[] createdIds = new boolean[threads * batchSize]; + for ( int i = 0; i < threads; i++ ) + { + executor.submit( () -> + { + try ( Transaction tx = db.beginTx() ) + { + for ( int j = 0; j < batchSize / 2; j++ ) + { + int index = toIntExact( db.createNode().getId() ); + createdIds[index] = true; + } + tx.success(); + } + } ); + } + executor.shutdown(); + while ( !executor.awaitTermination( 1, SECONDS ) ) + { // Just wait longer + } + assertFalse( allTrue( createdIds ) ); + + // when/then + db.restartDatabase(); + try ( Transaction tx = db.beginTx() ) + { + while ( !allTrue( createdIds ) ) + { + int index = toIntExact( db.createNode().getId() ); + assert !createdIds[index]; + createdIds[index] = true; + } + tx.success(); + } + } + + private static boolean allTrue( boolean[] values ) + { + for ( boolean value : values ) + { + if ( !value ) + { + return false; + } + } + return true; + } +} diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/IndexWorkSyncTransactionApplicationStressIT.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/IndexWorkSyncTransactionApplicationStressIT.java index 4ed07d203caa3..1707e3dccb18b 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/IndexWorkSyncTransactionApplicationStressIT.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/command/IndexWorkSyncTransactionApplicationStressIT.java @@ -48,6 +48,7 @@ import org.neo4j.kernel.impl.store.NodeStore; import org.neo4j.kernel.impl.transaction.command.Command.NodeCommand; import org.neo4j.kernel.impl.util.Dependencies; +import org.neo4j.storageengine.api.CommandCreationContext; import org.neo4j.storageengine.api.StorageCommand; import org.neo4j.storageengine.api.StorageStatement; import org.neo4j.storageengine.api.TransactionApplicationMode; @@ -147,6 +148,7 @@ private class Worker implements Runnable private final int batchSize; private final IndexProxy index; private int i, base; + private final CommandCreationContext commandCreationContext; Worker( int id, AtomicBoolean end, RecordStorageEngine storageEngine, int batchSize, IndexProxy index ) { @@ -157,6 +159,7 @@ private class Worker implements Runnable this.index = index; NeoStores neoStores = this.storageEngine.testAccessNeoStores(); this.nodeIds = neoStores.getNodeStore(); + this.commandCreationContext = storageEngine.allocateCommandCreationContext(); } @Override @@ -183,6 +186,10 @@ public void run() { throw new RuntimeException( e ); } + finally + { + commandCreationContext.close(); + } } private TransactionToApply createNodeAndProperty( int progress ) throws Exception diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/storeview/NeoStoreIndexStoreViewTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/storeview/NeoStoreIndexStoreViewTest.java index e634e5c1b73c8..8ec8ace6c35f1 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/storeview/NeoStoreIndexStoreViewTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/transaction/state/storeview/NeoStoreIndexStoreViewTest.java @@ -23,8 +23,6 @@ import org.junit.Rule; import org.junit.Test; import org.mockito.InOrder; -import org.mockito.stubbing.Answer; - import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; @@ -59,11 +57,16 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import static org.neo4j.helpers.collection.Iterators.asSet; import static org.neo4j.helpers.collection.Iterators.emptySetOf; @@ -96,11 +99,13 @@ public void before() throws KernelException neoStores = graphDb.getDependencyResolver().resolveDependency( RecordStorageEngine.class ).testAccessNeoStores(); - locks = mock( LockService.class, (Answer) invocation -> - { - Long nodeId = (Long) invocation.getArguments()[0]; - return lockMocks.computeIfAbsent( nodeId, k -> mock( Lock.class ) ); - } ); + locks = mock( LockService.class ); + when( locks.acquireNodeLock( anyLong(), any() ) ).thenAnswer( + invocation -> + { + Long nodeId = (Long) invocation.getArguments()[0]; + return lockMocks.computeIfAbsent( nodeId, k -> mock( Lock.class ) ); + } ); storeView = new NeoStoreIndexStoreView( locks, neoStores ); } @@ -157,7 +162,7 @@ public void shouldLockNodesWhileReadingThem() throws Exception storeScan.run(); // then - assertEquals( "allocated locks: " + lockMocks.keySet(), 2, lockMocks.size() ); + assertThat( "allocated locks: " + lockMocks.keySet(), lockMocks.size(), greaterThanOrEqualTo( 2 ) ); Lock lock0 = lockMocks.get( 0L ); Lock lock1 = lockMocks.get( 1L ); assertNotNull( "Lock[node=0] never acquired", lock0 ); @@ -167,7 +172,6 @@ public void shouldLockNodesWhileReadingThem() throws Exception order.verify( lock0 ).release(); order.verify( locks ).acquireNodeLock( 1, LockService.LockType.READ_LOCK ); order.verify( lock1 ).release(); - order.verifyNoMoreInteractions(); } @Test diff --git a/community/kernel/src/test/java/org/neo4j/test/impl/EphemeralIdGenerator.java b/community/kernel/src/test/java/org/neo4j/test/impl/EphemeralIdGenerator.java index 44f7df8851369..8b50626dc0dba 100644 --- a/community/kernel/src/test/java/org/neo4j/test/impl/EphemeralIdGenerator.java +++ b/community/kernel/src/test/java/org/neo4j/test/impl/EphemeralIdGenerator.java @@ -27,6 +27,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import org.neo4j.collection.primitive.PrimitiveLongCollections; import org.neo4j.kernel.impl.store.id.IdGenerator; import org.neo4j.kernel.impl.store.id.IdGeneratorFactory; import org.neo4j.kernel.impl.store.id.IdRange; @@ -35,12 +36,14 @@ import org.neo4j.kernel.impl.store.id.configuration.IdTypeConfiguration; import org.neo4j.kernel.impl.store.id.configuration.IdTypeConfigurationProvider; +import static java.lang.Integer.min; + public class EphemeralIdGenerator implements IdGenerator { public static class Factory implements IdGeneratorFactory { protected final Map generators = new EnumMap<>( IdType.class ); - private IdTypeConfigurationProvider + private final IdTypeConfigurationProvider idTypeConfigurationProvider = new CommunityIdTypeConfigurationProvider(); @Override @@ -92,7 +95,7 @@ public String toString() } @Override - public long nextId() + public synchronized long nextId() { if ( freeList != null ) { @@ -106,9 +109,19 @@ public long nextId() } @Override - public IdRange nextIdBatch( int size ) + public synchronized IdRange nextIdBatch( int size ) { - throw new UnsupportedOperationException(); + long[] defragIds = PrimitiveLongCollections.EMPTY_LONG_ARRAY; + if ( freeList != null && !freeList.isEmpty() ) + { + defragIds = new long[min( size, freeList.size() )]; + for ( int i = 0; i < defragIds.length; i++ ) + { + defragIds[i] = freeList.poll(); + } + size -= defragIds.length; + } + return new IdRange( defragIds, nextId.getAndAdd( size ), size ); } @Override diff --git a/community/neo4j-harness/src/test/java/org/neo4j/harness/JavaProceduresTest.java b/community/neo4j-harness/src/test/java/org/neo4j/harness/JavaProceduresTest.java index c6f51761e80ce..bdbfc44b4dcb6 100644 --- a/community/neo4j-harness/src/test/java/org/neo4j/harness/JavaProceduresTest.java +++ b/community/neo4j-harness/src/test/java/org/neo4j/harness/JavaProceduresTest.java @@ -25,6 +25,7 @@ import java.util.stream.Stream; +import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.kernel.api.exceptions.ProcedureException; import org.neo4j.procedure.Context; import org.neo4j.procedure.Mode; @@ -180,6 +181,7 @@ public void shouldWorkWithInjectableFromKernelExtensionWithMorePower() throws Th { // When try ( ServerControls server = TestServerBuilders.newInProcessBuilder() + .withConfig( GraphDatabaseSettings.record_id_batch_size, "1" ) .withProcedure( MyProceduresUsingMyCoreAPI.class ).newServer() ) { // Then diff --git a/community/server/src/test/java/org/neo4j/server/rest/web/DatabaseActionsTest.java b/community/server/src/test/java/org/neo4j/server/rest/web/DatabaseActionsTest.java index 81dfa06fab26e..d6f0a052e2616 100644 --- a/community/server/src/test/java/org/neo4j/server/rest/web/DatabaseActionsTest.java +++ b/community/server/src/test/java/org/neo4j/server/rest/web/DatabaseActionsTest.java @@ -43,6 +43,7 @@ import org.neo4j.graphdb.Relationship; import org.neo4j.graphdb.RelationshipType; import org.neo4j.graphdb.Transaction; +import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.graphdb.schema.ConstraintDefinition; import org.neo4j.graphdb.schema.ConstraintType; import org.neo4j.graphdb.schema.IndexDefinition; @@ -100,7 +101,9 @@ public class DatabaseActionsTest @BeforeClass public static void createDb() throws IOException { - graph = (GraphDatabaseFacade) new TestGraphDatabaseFactory().newImpermanentDatabase(); + graph = (GraphDatabaseFacade) new TestGraphDatabaseFactory().newImpermanentDatabaseBuilder() + .setConfig( GraphDatabaseSettings.record_id_batch_size, "1" ) + .newGraphDatabase(); database = new WrappedDatabase( graph ); graphdbHelper = new GraphDbHelper( database ); actions = new TransactionWrappedDatabaseActions( new LeaseManager( Clocks.fakeClock() ), database.getGraph() ); @@ -578,7 +581,7 @@ public void shouldBeAbleToSetRelationshipProperty() throws Exception public void shouldRemoveRelationProperties() throws Exception { long relId = graphdbHelper.createRelationship( "PAIR-PROGRAMS_WITH" ); - Map map = new HashMap(); + Map map = new HashMap<>(); map.put( "foo", "bar" ); map.put( "baz", 22 ); graphdbHelper.setRelationshipProperties( relId, map ); @@ -593,7 +596,7 @@ public void shouldRemoveRelationProperties() throws Exception public void shouldRemoveRelationshipProperty() throws Exception { long relId = graphdbHelper.createRelationship( "PAIR-PROGRAMS_WITH" ); - Map map = new HashMap(); + Map map = new HashMap<>(); map.put( "foo", "bar" ); map.put( "baz", 22 ); graphdbHelper.setRelationshipProperties( relId, map ); @@ -1115,7 +1118,7 @@ public void shouldAddLabelToNode() throws Exception { // GIVEN long node = actions.createNode( null ).getId(); - Collection labels = new ArrayList(); + Collection labels = new ArrayList<>(); String labelName = "Wonk"; labels.add( labelName ); diff --git a/enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/core/state/machines/id/IdRangeIterator.java b/enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/core/state/machines/id/IdRangeIterator.java deleted file mode 100644 index cb5e75e1a769b..0000000000000 --- a/enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/core/state/machines/id/IdRangeIterator.java +++ /dev/null @@ -1,89 +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.causalclustering.core.state.machines.id; - -import org.neo4j.kernel.impl.store.id.IdGeneratorImpl; -import org.neo4j.kernel.impl.store.id.IdRange; - -import java.util.Arrays; - -import static org.neo4j.collection.primitive.PrimitiveLongCollections.EMPTY_LONG_ARRAY; - -public class IdRangeIterator -{ - static IdRangeIterator EMPTY_ID_RANGE_ITERATOR = - new IdRangeIterator( new IdRange( EMPTY_LONG_ARRAY, 0, 0 ) ) - { - @Override - public long next() - { - return VALUE_REPRESENTING_NULL; - } - }; - - static final long VALUE_REPRESENTING_NULL = -1; - private int position = 0; - private final long[] defrag; - private final long start; - private final int length; - - IdRangeIterator( IdRange idRange ) - { - this.defrag = idRange.getDefragIds(); - this.start = idRange.getRangeStart(); - this.length = idRange.getRangeLength(); - } - - public long next() - { - try - { - if ( position < defrag.length ) - { - return defrag[position]; - } - - long candidate = nextRangeCandidate(); - if ( candidate == IdGeneratorImpl.INTEGER_MINUS_ONE ) - { - position++; - candidate = nextRangeCandidate(); - } - return candidate; - } - finally - { - ++position; - } - } - - private long nextRangeCandidate() - { - int offset = position - defrag.length; - return (offset < length) ? (start + offset) : VALUE_REPRESENTING_NULL; - } - - @Override - public String toString() - { - return "IdRangeIterator[start:" + start + ", length:" + length + ", position:" + position + ", defrag:" + - Arrays.toString( defrag ) + "]"; - } -} diff --git a/enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/core/state/machines/id/ReplicatedIdGenerator.java b/enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/core/state/machines/id/ReplicatedIdGenerator.java index f3541d41200a5..e0bb204e54856 100644 --- a/enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/core/state/machines/id/ReplicatedIdGenerator.java +++ b/enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/core/state/machines/id/ReplicatedIdGenerator.java @@ -26,13 +26,15 @@ import org.neo4j.kernel.impl.store.id.IdContainer; import org.neo4j.kernel.impl.store.id.IdGenerator; import org.neo4j.kernel.impl.store.id.IdRange; +import org.neo4j.kernel.impl.store.id.IdRangeIterator; import org.neo4j.kernel.impl.store.id.IdType; import org.neo4j.logging.Log; import org.neo4j.logging.LogProvider; import static java.lang.Math.max; -import static org.neo4j.causalclustering.core.state.machines.id.IdRangeIterator.EMPTY_ID_RANGE_ITERATOR; -import static org.neo4j.causalclustering.core.state.machines.id.IdRangeIterator.VALUE_REPRESENTING_NULL; + +import static org.neo4j.kernel.impl.store.id.IdRangeIterator.EMPTY_ID_RANGE_ITERATOR; +import static org.neo4j.kernel.impl.store.id.IdRangeIterator.VALUE_REPRESENTING_NULL; class ReplicatedIdGenerator implements IdGenerator { @@ -41,7 +43,7 @@ class ReplicatedIdGenerator implements IdGenerator private final ReplicatedIdRangeAcquirer acquirer; private volatile long highId; private volatile IdRangeIterator idQueue = EMPTY_ID_RANGE_ITERATOR; - private IdContainer idContainer; + private final IdContainer idContainer; private final ReentrantLock idContainerLock = new ReentrantLock(); ReplicatedIdGenerator( FileSystemAbstraction fs, File file, IdType idType, long highId, @@ -116,23 +118,35 @@ public synchronized long nextId() return id; } - long nextId = idQueue.next(); + long nextId = idQueue.nextId(); if ( nextId == VALUE_REPRESENTING_NULL ) { - IdAllocation allocation = acquirer.acquireIds( idType ); - - assert allocation.getIdRange().getRangeLength() > 0; - log.debug( "Received id allocation " + allocation + " for " + idType ); - nextId = storeLocally( allocation ); + acquireNextIdBatch(); + nextId = idQueue.nextId(); } highId = max( highId, nextId + 1 ); return nextId; } + private void acquireNextIdBatch() + { + IdAllocation allocation = acquirer.acquireIds( idType ); + + assert allocation.getIdRange().getRangeLength() > 0; + log.debug( "Received id allocation " + allocation + " for " + idType ); + storeLocally( allocation ); + } + @Override - public IdRange nextIdBatch( int size ) + public synchronized IdRange nextIdBatch( int size ) { - throw new UnsupportedOperationException( "Should never be called" ); + IdRange range = idQueue.nextIdBatch( size ); + if ( range.totalSize() == 0 ) + { + acquireNextIdBatch(); + range = idQueue.nextIdBatch( size ); + } + return range; } @Override @@ -188,11 +202,10 @@ private long getReusableId() } } - private long storeLocally( IdAllocation allocation ) + private void storeLocally( IdAllocation allocation ) { setHighId( allocation.getHighestIdInUse() + 1 ); // high id is certainly bigger than the highest id in use - this.idQueue = new IdRangeIterator( respectingHighId( allocation.getIdRange() ) ); - return idQueue.next(); + this.idQueue = respectingHighId( allocation.getIdRange() ).iterator(); } private IdRange respectingHighId( IdRange idRange ) diff --git a/enterprise/causal-clustering/src/test/java/org/neo4j/causalclustering/core/state/machines/id/IdRangeIteratorTest.java b/enterprise/causal-clustering/src/test/java/org/neo4j/causalclustering/core/state/machines/id/IdRangeIteratorTest.java deleted file mode 100644 index 0deba484db5f8..0000000000000 --- a/enterprise/causal-clustering/src/test/java/org/neo4j/causalclustering/core/state/machines/id/IdRangeIteratorTest.java +++ /dev/null @@ -1,83 +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.causalclustering.core.state.machines.id; - -import org.junit.Test; -import org.neo4j.kernel.impl.store.id.IdRange; - -import java.util.HashSet; -import java.util.Set; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -public class IdRangeIteratorTest -{ - @Test - public void shouldReturnValueRepresentingNullIfWeExhaustIdRange() throws Exception - { - // given - int rangeLength = 1024; - IdRangeIterator iterator = new IdRangeIterator( new IdRange( new long[]{}, 0, rangeLength ) ); - - // when - for ( int i = 0; i < rangeLength; i++ ) - { - iterator.next(); - } - - // then - assertEquals( IdRangeIterator.VALUE_REPRESENTING_NULL, iterator.next() ); - } - - @Test - public void shouldNotHaveAnyGaps() throws Exception - { - // given - int rangeLength = 1024; - IdRangeIterator iterator = new IdRangeIterator( new IdRange( new long[]{}, 0, rangeLength ) ); - - // when - Set seenIds = new HashSet<>(); - for ( int i = 0; i < rangeLength; i++ ) - { - seenIds.add( iterator.next() ); - if ( i > 0 ) - { - // then - assertTrue( "Missing id " + (i - 1), seenIds.contains( (long) i - 1 ) ); - } - } - } - - @Test - public void shouldUseDefragIdsFirst() throws Exception - { - // given - int rangeLength = 1024; - IdRangeIterator iterator = new IdRangeIterator( new IdRange( new long[]{7,8,9}, 1024, rangeLength ) ); - - // then - assertEquals(7, iterator.next()); - assertEquals(8, iterator.next()); - assertEquals(9, iterator.next()); - assertEquals(1024, iterator.next()); - } -} diff --git a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/id/HaIdGeneratorFactory.java b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/id/HaIdGeneratorFactory.java index 2afdc02aac06a..882cf8c77304d 100644 --- a/enterprise/ha/src/main/java/org/neo4j/kernel/ha/id/HaIdGeneratorFactory.java +++ b/enterprise/ha/src/main/java/org/neo4j/kernel/ha/id/HaIdGeneratorFactory.java @@ -144,7 +144,7 @@ private class HaIdGenerator implements IdGenerator private final int grabSize; private final IdType idType; private volatile IdGeneratorState state; - private long maxId; + private final long maxId; HaIdGenerator( IdGenerator initialDelegate, File fileName, int grabSize, IdType idType, IdGeneratorState initialState, long maxId ) @@ -333,41 +333,52 @@ public synchronized long nextId() long nextId = nextLocalId(); if ( nextId == IdRangeIterator.VALUE_REPRESENTING_NULL ) { - // If we don't have anymore grabbed ids from master, grab a bunch - try ( Response response = - master.allocateIds( requestContextFactory.newRequestContext(), idType ) ) - { - IdAllocation allocation = response.response(); - log.info( "Received id allocation " + allocation + " from master " + master + " for " + idType ); - nextId = storeLocally( allocation ); - } - catch ( ComException e ) - { - throw new TransientTransactionFailureException( - "Cannot allocate new entity ids from the cluster master. " + - "The master instance is either down, or we have network connectivity problems", e ); - } + askForNextRangeFromMaster(); + nextId = nextLocalId(); } return nextId; } + private void askForNextRangeFromMaster() + { + // If we don't have anymore grabbed ids from master, grab a bunch + try ( Response response = + master.allocateIds( requestContextFactory.newRequestContext(), idType ) ) + { + IdAllocation allocation = response.response(); + log.info( "Received id allocation " + allocation + " from master " + master + " for " + idType ); + storeLocally( allocation ); + } + catch ( ComException e ) + { + throw new TransientTransactionFailureException( + "Cannot allocate new entity ids from the cluster master. " + + "The master instance is either down, or we have network connectivity problems", e ); + } + } + @Override - public IdRange nextIdBatch( int size ) + public synchronized IdRange nextIdBatch( int size ) { - throw new UnsupportedOperationException( "Should never be called" ); + IdRange range = idQueue.nextIdBatch( size ); + if ( range.totalSize() == 0 ) + { + askForNextRangeFromMaster(); + range = idQueue.nextIdBatch( size ); + } + return range; } - private long storeLocally( IdAllocation allocation ) + private void storeLocally( IdAllocation allocation ) { setHighId( allocation.getHighestIdInUse() ); this.defragCount = allocation.getDefragCount(); - this.idQueue = new IdRangeIterator( allocation.getIdRange() ); - return idQueue.next(); + this.idQueue = allocation.getIdRange().iterator(); } private long nextLocalId() { - return this.idQueue.next(); + return this.idQueue.nextId(); } @Override diff --git a/enterprise/ha/src/test/java/org/neo4j/kernel/ha/id/HaIdGeneratorFactoryTest.java b/enterprise/ha/src/test/java/org/neo4j/kernel/ha/id/HaIdGeneratorFactoryTest.java index ef1682fe3dcb5..d3de54a0f19cd 100644 --- a/enterprise/ha/src/test/java/org/neo4j/kernel/ha/id/HaIdGeneratorFactoryTest.java +++ b/enterprise/ha/src/test/java/org/neo4j/kernel/ha/id/HaIdGeneratorFactoryTest.java @@ -237,12 +237,12 @@ public void shouldNotUseForbiddenMinusOneIdFromIdBatches() throws Exception IdRange idRange = new IdRange( defragIds, low, size ); // WHEN - IdRangeIterator iterartor = new IdRangeIterator( idRange ); + IdRangeIterator iterartor = idRange.iterator(); // THEN for ( long id : defragIds ) { - assertEquals( id, iterartor.next() ); + assertEquals( id, iterartor.nextId() ); } int expectedRangeSize = size - 1; // due to the forbidden id @@ -253,11 +253,11 @@ public void shouldNotUseForbiddenMinusOneIdFromIdBatches() throws Exception expectedId++; } - long id = iterartor.next(); + long id = iterartor.nextId(); assertNotEquals( IdGeneratorImpl.INTEGER_MINUS_ONE, id ); assertEquals( expectedId, id ); } - assertEquals( VALUE_REPRESENTING_NULL, iterartor.next() ); + assertEquals( VALUE_REPRESENTING_NULL, iterartor.nextId() ); } @SuppressWarnings( "unchecked" ) diff --git a/enterprise/kernel/src/test/java/org/neo4j/graphdb/store/id/IdReuseTest.java b/enterprise/kernel/src/test/java/org/neo4j/graphdb/store/id/IdReuseTest.java index 640aa2c5863d4..e01eaa32a7c55 100644 --- a/enterprise/kernel/src/test/java/org/neo4j/graphdb/store/id/IdReuseTest.java +++ b/enterprise/kernel/src/test/java/org/neo4j/graphdb/store/id/IdReuseTest.java @@ -31,6 +31,7 @@ import org.neo4j.graphdb.RelationshipType; import org.neo4j.graphdb.ResourceIterator; import org.neo4j.graphdb.Transaction; +import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.helpers.collection.Iterators; import org.neo4j.kernel.impl.enterprise.configuration.EnterpriseEditionSettings; import org.neo4j.kernel.impl.storageengine.impl.recordstorage.id.IdController; @@ -47,7 +48,17 @@ public class IdReuseTest { @Rule public DatabaseRule dbRule = new EnterpriseDatabaseRule() - .withSetting( EnterpriseEditionSettings.idTypesToReuse, IdType.NODE + "," + IdType.RELATIONSHIP ); + .withSetting( EnterpriseEditionSettings.idTypesToReuse, IdType.NODE + "," + IdType.RELATIONSHIP ) + // Set tx id batching to 1 since there are sequential id testing which gets bitten by this inner working: + // - TxA begins and creates node Na w/ label L. Doing so will create inner transaction TxB which creates L + // - TxB completes and will be set as the tx for this thread in the pool (puddle) + // - TxA continues and creates nodes Na and similar Nb, then relationship Ra. Doing so allocates id ranges for NODE/RELATIONSHIP + // - TxA completes and returns TxA back to pool, but TxB is already in that puddle so goes back to delegate pool + // - Next transaction who wants to create a relationship or node in this thread gets TxB + // - TXB has not allocated NODE/RELATIONSHIP id batch, so does so, which is after TxA id batches. + // - The id of next node/relationship created by TxB will not be immediately following the one created in TxA. + // = This is fine but perhaps unexpected and tests asserting on ids being sequential will fail. + .withSetting( GraphDatabaseSettings.record_id_batch_size, "1" ); @Test public void shouldReuseNodeIdsFromRolledBackTransaction() throws Exception diff --git a/tools/src/test/java/org/neo4j/tools/applytx/DatabaseRebuildToolTest.java b/tools/src/test/java/org/neo4j/tools/applytx/DatabaseRebuildToolTest.java index 5d8b672e27232..e82fa29bccefe 100644 --- a/tools/src/test/java/org/neo4j/tools/applytx/DatabaseRebuildToolTest.java +++ b/tools/src/test/java/org/neo4j/tools/applytx/DatabaseRebuildToolTest.java @@ -34,6 +34,7 @@ import org.neo4j.graphdb.PropertyContainer; import org.neo4j.graphdb.Relationship; import org.neo4j.graphdb.Transaction; +import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.io.fs.DefaultFileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.pagecache.PageCache; @@ -195,7 +196,9 @@ private InputStream input( String... strings ) private void databaseWithSomeTransactions( File dir ) { - GraphDatabaseService db = new TestGraphDatabaseFactory().newEmbeddedDatabase( dir ); + GraphDatabaseService db = new TestGraphDatabaseFactory().newEmbeddedDatabaseBuilder( dir ) + .setConfig( GraphDatabaseSettings.record_id_batch_size, "1" ) + .newGraphDatabase(); Node[] nodes = new Node[10]; for ( int i = 0; i < nodes.length; i++ ) {