From 564f935964e690cd3a734f33ea5fbbda3d2e7b7c Mon Sep 17 00:00:00 2001 From: Anton Persson Date: Wed, 15 Nov 2017 11:09:59 +0100 Subject: [PATCH] IndexReader return resource iterator So that they can be closed. --- .../api/index/DelegatingIndexReader.java | 4 +- .../schema/NativeSchemaNumberIndexReader.java | 4 +- .../impl/index/schema/NodeValueIterator.java | 5 +- .../schema/fusion/FusionIndexReader.java | 12 +- .../storageengine/api/schema/IndexReader.java | 10 +- .../api/schema/NodeValueIndexProgressor.java | 14 +- .../api/StatementOperationsTestHelper.java | 6 +- .../api/index/inmemory/HashBasedIndex.java | 37 +++-- .../inmemory/InMemoryIndexImplementation.java | 6 +- .../StateHandlingStatementOperationsTest.java | 7 +- .../schema/fusion/FusionIndexReaderTest.java | 70 ++++++++- .../api/schema/DefaultIndexReaderTest.java | 4 +- .../schema/reader/PartitionedIndexReader.java | 16 +- .../impl/schema/reader/SimpleIndexReader.java | 27 +++- .../reader/PartitionedIndexReaderTest.java | 33 +++-- .../PrimitiveLongResourceCollections.java | 139 ++++++++++++++++++ 16 files changed, 308 insertions(+), 86 deletions(-) create mode 100644 community/primitive-collections/src/main/java/org/neo4j/collection/primitive/PrimitiveLongResourceCollections.java diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/index/DelegatingIndexReader.java b/community/kernel/src/main/java/org/neo4j/kernel/api/index/DelegatingIndexReader.java index c707143ad969e..8c6aca84609e9 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/index/DelegatingIndexReader.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/index/DelegatingIndexReader.java @@ -19,7 +19,7 @@ */ package org.neo4j.kernel.api.index; -import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException; import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.storageengine.api.schema.IndexReader; @@ -48,7 +48,7 @@ public IndexSampler createSampler() } @Override - public PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException + public PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException { return delegate.query( predicates ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NativeSchemaNumberIndexReader.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NativeSchemaNumberIndexReader.java index 8ec493bec6f52..de38359384da3 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NativeSchemaNumberIndexReader.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NativeSchemaNumberIndexReader.java @@ -27,7 +27,7 @@ import java.util.HashSet; import java.util.Set; -import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.cursor.RawCursor; import org.neo4j.helpers.ArrayUtil; import org.neo4j.index.internal.gbptree.GBPTree; @@ -114,7 +114,7 @@ public long countIndexedNodes( long nodeId, Value... propertyValues ) } @Override - public PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException + public PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException { NodeValueIterator nodeValueIterator = new NodeValueIterator(); query( nodeValueIterator, IndexOrder.NONE, predicates ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NodeValueIterator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NodeValueIterator.java index f241f9f0d790d..622a908158f6e 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NodeValueIterator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NodeValueIterator.java @@ -21,7 +21,7 @@ import org.neo4j.collection.primitive.PrimitiveLongCollections; import org.neo4j.collection.primitive.PrimitiveLongIterator; -import org.neo4j.graphdb.Resource; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.storageengine.api.schema.IndexProgressor; import org.neo4j.values.storable.Value; @@ -29,7 +29,7 @@ * A {@link IndexProgressor} + {@link IndexProgressor.NodeValueClient} combo presented as a {@link PrimitiveLongIterator}. */ public class NodeValueIterator extends PrimitiveLongCollections.PrimitiveLongBaseIterator - implements IndexProgressor.NodeValueClient, Resource + implements IndexProgressor.NodeValueClient, PrimitiveLongResourceIterator { private boolean closed; private IndexProgressor progressor; @@ -66,6 +66,7 @@ public void close() { closed = true; progressor.close(); + progressor = null; } } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReader.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReader.java index b46bd00ca1486..bc1b546e5c60c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReader.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReader.java @@ -23,8 +23,8 @@ import java.util.Arrays; import java.util.Queue; -import org.neo4j.collection.primitive.PrimitiveLongCollections; -import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceCollections; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.internal.kernel.api.IndexOrder; import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.internal.kernel.api.IndexQuery.ExactPredicate; @@ -80,7 +80,7 @@ public IndexSampler createSampler() } @Override - public PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException + public PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException { if ( predicates.length > 1 ) { @@ -101,9 +101,9 @@ public PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotAp // todo: There will be no ordering of the node ids here. Is this a problem? if ( predicates[0] instanceof ExistsPredicate ) { - PrimitiveLongIterator nativeResult = nativeReader.query( predicates[0] ); - PrimitiveLongIterator luceneResult = luceneReader.query( predicates[0] ); - return PrimitiveLongCollections.concat( nativeResult, luceneResult ); + PrimitiveLongResourceIterator nativeResult = nativeReader.query( predicates[0] ); + PrimitiveLongResourceIterator luceneResult = luceneReader.query( predicates[0] ); + return PrimitiveLongResourceCollections.concat( nativeResult, luceneResult ); } return luceneReader.query( predicates ); diff --git a/community/kernel/src/main/java/org/neo4j/storageengine/api/schema/IndexReader.java b/community/kernel/src/main/java/org/neo4j/storageengine/api/schema/IndexReader.java index dc26b0add37f9..91394fb19905d 100644 --- a/community/kernel/src/main/java/org/neo4j/storageengine/api/schema/IndexReader.java +++ b/community/kernel/src/main/java/org/neo4j/storageengine/api/schema/IndexReader.java @@ -19,8 +19,8 @@ */ package org.neo4j.storageengine.api.schema; -import org.neo4j.collection.primitive.PrimitiveLongCollections; -import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceCollections; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.graphdb.Resource; import org.neo4j.internal.kernel.api.IndexOrder; import org.neo4j.internal.kernel.api.IndexQuery; @@ -48,7 +48,7 @@ public interface IndexReader extends Resource * @param predicates the predicates to query for. * @return the matching entity IDs. */ - PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException; + PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException; /** * Queries the index for the given {@link IndexQuery} predicates. @@ -101,9 +101,9 @@ public IndexSampler createSampler() } @Override - public PrimitiveLongIterator query( IndexQuery[] predicates ) + public PrimitiveLongResourceIterator query( IndexQuery[] predicates ) { - return PrimitiveLongCollections.emptyIterator(); + return PrimitiveLongResourceCollections.emptyIterator(); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/storageengine/api/schema/NodeValueIndexProgressor.java b/community/kernel/src/main/java/org/neo4j/storageengine/api/schema/NodeValueIndexProgressor.java index 3f1152f7e5064..655a800e5707a 100644 --- a/community/kernel/src/main/java/org/neo4j/storageengine/api/schema/NodeValueIndexProgressor.java +++ b/community/kernel/src/main/java/org/neo4j/storageengine/api/schema/NodeValueIndexProgressor.java @@ -19,15 +19,15 @@ */ package org.neo4j.storageengine.api.schema; -import org.neo4j.collection.primitive.PrimitiveLongIterator; -import org.neo4j.graphdb.Resource; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; +import org.neo4j.values.storable.Value; class NodeValueIndexProgressor implements IndexProgressor { - private final PrimitiveLongIterator ids; + private final PrimitiveLongResourceIterator ids; private final NodeValueClient client; - NodeValueIndexProgressor( PrimitiveLongIterator ids, NodeValueClient client ) + NodeValueIndexProgressor( PrimitiveLongResourceIterator ids, NodeValueClient client ) { this.ids = ids; this.client = client; @@ -38,7 +38,7 @@ public boolean next() { while ( ids.hasNext() ) { - if ( client.acceptNode( ids.next(), null ) ) + if ( client.acceptNode( ids.next(), (Value[]) null ) ) { return true; } @@ -49,9 +49,9 @@ public boolean next() @Override public void close() { - if ( ids instanceof Resource ) + if ( ids != null ) { - ((Resource) ids).close(); + ids.close(); } } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/StatementOperationsTestHelper.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/StatementOperationsTestHelper.java index ae7b85f864fd5..a12d3ec5886ab 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/StatementOperationsTestHelper.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/StatementOperationsTestHelper.java @@ -19,11 +19,10 @@ */ package org.neo4j.kernel.impl.api; -import org.neo4j.collection.primitive.PrimitiveLongCollections; +import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.kernel.api.ReadOperations; import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException; import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException; -import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.kernel.api.txstate.TransactionState; import org.neo4j.kernel.impl.api.operations.CountsOperations; import org.neo4j.kernel.impl.api.operations.EntityReadOperations; @@ -46,6 +45,7 @@ import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.neo4j.collection.primitive.PrimitiveLongResourceCollections.emptyIterator; public abstract class StatementOperationsTestHelper { @@ -79,7 +79,7 @@ public static KernelStatement mockedState( final TransactionState txState ) { IndexReader indexReader = mock( IndexReader.class ); when( indexReader.query( isA( IndexQuery.ExactPredicate.class ) ) ) - .thenReturn( PrimitiveLongCollections.emptyIterator() ); + .thenReturn( emptyIterator() ); StorageStatement storageStatement = mock( StorageStatement.class ); when( storageStatement.getIndexReader( any() ) ).thenReturn( indexReader ); when( state.getStoreStatement() ).thenReturn( storageStatement ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/HashBasedIndex.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/HashBasedIndex.java index bcd3e56b5409e..366a13a466f61 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/HashBasedIndex.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/HashBasedIndex.java @@ -28,13 +28,15 @@ import java.util.Map; import java.util.Set; -import org.neo4j.collection.primitive.PrimitiveLongCollections; import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.helpers.collection.Iterables; import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.storageengine.api.schema.IndexSampler; import org.neo4j.values.storable.Value; +import static org.neo4j.collection.primitive.PrimitiveLongCollections.emptyIterator; +import static org.neo4j.collection.primitive.PrimitiveLongCollections.resourceIterator; import static org.neo4j.collection.primitive.PrimitiveLongCollections.toPrimitiveIterator; import static org.neo4j.internal.kernel.api.IndexQuery.IndexQueryType.exact; import static org.neo4j.kernel.impl.api.PropertyValueComparison.COMPARE_VALUES; @@ -73,13 +75,13 @@ synchronized void drop() } @Override - synchronized PrimitiveLongIterator doIndexSeek( Object... propertyValues ) + synchronized PrimitiveLongResourceIterator doIndexSeek( Object... propertyValues ) { Set nodes = data().get( Arrays.asList( propertyValues ) ); - return nodes == null ? PrimitiveLongCollections.emptyIterator() : toPrimitiveIterator( nodes.iterator() ); + return asResource( nodes == null ? emptyIterator() : toPrimitiveIterator( nodes.iterator() ) ); } - private synchronized PrimitiveLongIterator rangeSeekByNumberInclusive( Number lower, Number upper ) + private synchronized PrimitiveLongResourceIterator rangeSeekByNumberInclusive( Number lower, Number upper ) { Set nodeIds = new HashSet<>(); for ( Map.Entry,Set> entry : data.entrySet() ) @@ -96,10 +98,10 @@ private synchronized PrimitiveLongIterator rangeSeekByNumberInclusive( Number lo } } } - return toPrimitiveIterator( nodeIds.iterator() ); + return asResource( toPrimitiveIterator( nodeIds.iterator() ) ); } - private synchronized PrimitiveLongIterator rangeSeekByString( String lower, boolean includeLower, String upper, + private synchronized PrimitiveLongResourceIterator rangeSeekByString( String lower, boolean includeLower, String upper, boolean includeUpper ) { Set nodeIds = new HashSet<>(); @@ -137,28 +139,28 @@ private synchronized PrimitiveLongIterator rangeSeekByString( String lower, bool } } } - return toPrimitiveIterator( nodeIds.iterator() ); + return asResource( toPrimitiveIterator( nodeIds.iterator() ) ); } - private synchronized PrimitiveLongIterator rangeSeekByPrefix( String prefix ) + private synchronized PrimitiveLongResourceIterator rangeSeekByPrefix( String prefix ) { return stringSearch( ( String entry ) -> entry.startsWith( prefix ) ); } - private synchronized PrimitiveLongIterator containsString( String exactTerm ) + private synchronized PrimitiveLongResourceIterator containsString( String exactTerm ) { return stringSearch( ( String entry ) -> entry.contains( exactTerm ) ); } - private PrimitiveLongIterator endsWith( String suffix ) + private PrimitiveLongResourceIterator endsWith( String suffix ) { return stringSearch( ( String entry ) -> entry.endsWith( suffix ) ); } - private synchronized PrimitiveLongIterator scan() + private synchronized PrimitiveLongResourceIterator scan() { Iterable all = Iterables.flattenIterable( data.values() ); - return toPrimitiveIterator( all.iterator() ); + return asResource( toPrimitiveIterator( all.iterator() ) ); } @Override @@ -245,7 +247,7 @@ public synchronized IndexSampler createSampler() } @Override - public PrimitiveLongIterator query( IndexQuery... predicates ) + public PrimitiveLongResourceIterator query( IndexQuery... predicates ) { if ( predicates.length > 1 ) { @@ -296,7 +298,7 @@ private interface StringFilter boolean test( String s ); } - private PrimitiveLongIterator stringSearch( StringFilter filter ) + private PrimitiveLongResourceIterator stringSearch( StringFilter filter ) { Set nodeIds = new HashSet<>(); for ( Map.Entry,Set> entry : data.entrySet() ) @@ -310,7 +312,12 @@ private PrimitiveLongIterator stringSearch( StringFilter filter ) } } } - return toPrimitiveIterator( nodeIds.iterator() ); + return asResource( toPrimitiveIterator( nodeIds.iterator() ) ); + } + + private PrimitiveLongResourceIterator asResource( PrimitiveLongIterator iterator ) + { + return resourceIterator( iterator, null ); } @Override diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/InMemoryIndexImplementation.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/InMemoryIndexImplementation.java index 60918ad819a58..bdb8d1329a5eb 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/InMemoryIndexImplementation.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/InMemoryIndexImplementation.java @@ -21,7 +21,7 @@ import java.util.Set; -import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.helpers.collection.BoundedIterable; import org.neo4j.kernel.api.index.ArrayEncoder; import org.neo4j.storageengine.api.schema.IndexReader; @@ -33,7 +33,7 @@ abstract class InMemoryIndexImplementation implements IndexReader, BoundedIterab abstract void drop(); - public final PrimitiveLongIterator seek( Value... values ) + public final PrimitiveLongResourceIterator seek( Value... values ) { return doIndexSeek( encode( values ) ); } @@ -56,7 +56,7 @@ public final long countIndexedNodes( long nodeId, Value... propertyValues ) protected abstract long doCountIndexedNodes( long nodeId, Object... encode ); - abstract PrimitiveLongIterator doIndexSeek( Object... propertyValue ); + abstract PrimitiveLongResourceIterator doIndexSeek( Object... propertyValue ); abstract boolean doAdd( long nodeId, boolean applyIdempotently, Object... propertyValue ); 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 b1c575695b605..467074463a3d7 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 @@ -27,15 +27,16 @@ import org.neo4j.collection.primitive.PrimitiveLongCollections; import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceCollections; import org.neo4j.cursor.Cursor; import org.neo4j.helpers.collection.Iterables; +import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.kernel.api.AssertOpen; import org.neo4j.kernel.api.DataWriteOperations; import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException; import org.neo4j.kernel.api.explicitindex.AutoIndexOperations; import org.neo4j.kernel.api.explicitindex.AutoIndexing; import org.neo4j.kernel.api.properties.PropertyKeyValue; -import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.kernel.api.schema.LabelSchemaDescriptor; import org.neo4j.kernel.api.schema.SchemaDescriptorFactory; import org.neo4j.kernel.api.schema.constaints.ConstraintDescriptor; @@ -488,7 +489,7 @@ public void indexQueryClosesIndexReader() throws Exception StoreStatement storeStatement = mock( StoreStatement.class ); IndexReader indexReader = mock( IndexReader.class ); - when( indexReader.query( any() ) ).thenReturn( PrimitiveLongCollections.emptyIterator() ); + when( indexReader.query( any() ) ).thenReturn( PrimitiveLongResourceCollections.emptyIterator() ); when( storeStatement.getFreshIndexReader( any() ) ).thenReturn( indexReader ); when( kernelStatement.getStoreStatement() ).thenReturn( storeStatement ); @@ -612,7 +613,7 @@ public void shouldNotDecorateNumberQuerResultsWIthLookupFilterIfIndexHasFullNumb IndexReader indexReader = mock( IndexReader.class ); when( indexReader.hasFullNumberPrecision( any() ) ).thenReturn( true ); when( indexReader.query( any() ) ) - .thenAnswer( invocation -> PrimitiveLongCollections.iterator( nodeId ) ); + .thenAnswer( invocation -> PrimitiveLongResourceCollections.iterator( null, nodeId ) ); when( storeStatement.getFreshIndexReader( any() ) ).thenReturn( indexReader ); when( storeStatement.getIndexReader( any() ) ).thenReturn( indexReader ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReaderTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReaderTest.java index e46f330dfd036..a26f9ef4f26b8 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReaderTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/fusion/FusionIndexReaderTest.java @@ -22,23 +22,27 @@ import org.junit.Before; import org.junit.Test; -import org.neo4j.collection.primitive.Primitive; import org.neo4j.collection.primitive.PrimitiveLongCollections; import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceCollections; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.collection.primitive.PrimitiveLongSet; -import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException; import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.internal.kernel.api.IndexQuery.NumberRangePredicate; import org.neo4j.internal.kernel.api.IndexQuery.StringContainsPredicate; import org.neo4j.internal.kernel.api.IndexQuery.StringPrefixPredicate; import org.neo4j.internal.kernel.api.IndexQuery.StringRangePredicate; import org.neo4j.internal.kernel.api.IndexQuery.StringSuffixPredicate; +import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException; import org.neo4j.kernel.impl.index.schema.NativeSelector; import org.neo4j.storageengine.api.schema.IndexReader; import org.neo4j.values.storable.Value; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -73,6 +77,64 @@ public void closeMustCloseBothNativeAndLucene() throws Exception verify( luceneReader, times( 1 ) ).close(); } + // close iterator + + @Test + public void closeIteratorMustCloseNativeAndLucene() throws Exception + { + // given + PrimitiveLongResourceIterator nativeIter = mock( PrimitiveLongResourceIterator.class ); + PrimitiveLongResourceIterator luceneIter = mock( PrimitiveLongResourceIterator.class ); + when( nativeReader.query( any( IndexQuery.class ) ) ).thenReturn( nativeIter ); + when( luceneReader.query( any( IndexQuery.class ) ) ).thenReturn( luceneIter ); + + // when + fusionIndexReader.query( IndexQuery.exists( PROP_KEY ) ).close(); + + // then + verify( nativeIter, times( 1 ) ).close(); + verify( luceneIter, times( 1 ) ).close(); + } + + @Test + public void closeIteratorMustCloseNativeIfLuceneThrows() throws Exception + { + // given + verifyCloseOtherQueryIteratorWhenOneThrows( nativeReader, luceneReader ); + } + + @Test + public void closeIteratorMustCloseLuceneIfNativeThrows() throws Exception + { + // given + verifyCloseOtherQueryIteratorWhenOneThrows( luceneReader, nativeReader ); + } + + private void verifyCloseOtherQueryIteratorWhenOneThrows( IndexReader succeedingReader, IndexReader throwingReader ) + throws IndexNotApplicableKernelException + { + PrimitiveLongResourceIterator throwingIter = mock( PrimitiveLongResourceIterator.class ); + PrimitiveLongResourceIterator succeedingIter = mock( PrimitiveLongResourceIterator.class ); + when( throwingReader.query( any( IndexQuery.class ) ) ).thenReturn( throwingIter ); + when( succeedingReader.query( any( IndexQuery.class ) ) ).thenReturn( succeedingIter ); + RuntimeException closeException = new RuntimeException( "You shall not close" ); + doThrow( closeException ).when( throwingIter ).close(); + + // when + try + { + fusionIndexReader.query( IndexQuery.exists( PROP_KEY ) ).close(); + fail( "Should have failed" ); + } + catch ( RuntimeException e ) + { + assertSame( closeException, e.getCause() ); + } + + // then + verify( succeedingIter, times( 1 ) ).close(); + } + /* countIndexedNodes */ @Test @@ -200,8 +262,8 @@ public void mustCombineResultFromExistsPredicate() throws Exception { // given IndexQuery.ExistsPredicate exists = IndexQuery.exists( PROP_KEY ); - when( nativeReader.query( exists ) ).thenReturn( Primitive.iterator( 0L, 1L, 3L, 4L, 7L ) ); - when( luceneReader.query( exists ) ).thenReturn( Primitive.iterator( 2L, 5L, 6L ) ); + when( nativeReader.query( exists ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 0L, 1L, 3L, 4L, 7L ) ); + when( luceneReader.query( exists ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 2L, 5L, 6L ) ); // when PrimitiveLongIterator result = fusionIndexReader.query( exists ); diff --git a/community/kernel/src/test/java/org/neo4j/storageengine/api/schema/DefaultIndexReaderTest.java b/community/kernel/src/test/java/org/neo4j/storageengine/api/schema/DefaultIndexReaderTest.java index a1b52d71fac6c..58efd9faea30e 100644 --- a/community/kernel/src/test/java/org/neo4j/storageengine/api/schema/DefaultIndexReaderTest.java +++ b/community/kernel/src/test/java/org/neo4j/storageengine/api/schema/DefaultIndexReaderTest.java @@ -24,7 +24,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; -import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.internal.kernel.api.IndexOrder; import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException; @@ -66,7 +66,7 @@ public IndexSampler createSampler() } @Override - public PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException + public PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException { return null; } diff --git a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/reader/PartitionedIndexReader.java b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/reader/PartitionedIndexReader.java index b9f4ccd81d528..654a8b8c38038 100644 --- a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/reader/PartitionedIndexReader.java +++ b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/reader/PartitionedIndexReader.java @@ -24,8 +24,8 @@ import java.util.function.Function; import java.util.stream.Collectors; -import org.neo4j.collection.primitive.PrimitiveLongCollections; -import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceCollections; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.helpers.TaskCoordinator; import org.neo4j.io.IOUtils; import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException; @@ -66,7 +66,7 @@ public PartitionedIndexReader( List partitionSearchers, } @Override - public PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException + public PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException { try { @@ -84,7 +84,7 @@ public boolean hasFullNumberPrecision( IndexQuery... predicates ) return false; } - private PrimitiveLongIterator innerQuery( IndexReader reader, IndexQuery[] predicates ) + private PrimitiveLongResourceIterator innerQuery( IndexReader reader, IndexQuery[] predicates ) { try { @@ -140,11 +140,11 @@ public void close() } } - private PrimitiveLongIterator partitionedOperation( - Function readerFunction ) + private PrimitiveLongResourceIterator partitionedOperation( + Function readerFunction ) { - return PrimitiveLongCollections.concat( indexReaders.parallelStream() - .map( readerFunction::apply ) + return PrimitiveLongResourceCollections.concat( indexReaders.parallelStream() + .map( readerFunction ) .collect( Collectors.toList() ) ); } } diff --git a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/reader/SimpleIndexReader.java b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/reader/SimpleIndexReader.java index 13f79840ef98d..2bc06550f75e0 100644 --- a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/reader/SimpleIndexReader.java +++ b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/schema/reader/SimpleIndexReader.java @@ -29,7 +29,9 @@ import java.io.IOException; import java.util.Arrays; +import org.neo4j.collection.primitive.PrimitiveLongCollections; import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.helpers.TaskControl; import org.neo4j.helpers.TaskCoordinator; import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException; @@ -88,9 +90,10 @@ public IndexSampler createSampler() } @Override - public PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException + public PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws IndexNotApplicableKernelException { IndexQuery predicate = predicates[0]; + PrimitiveLongIterator result; switch ( predicate.type() ) { case exact: @@ -101,7 +104,8 @@ public PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotAp "Exact followed by another query predicate type is not supported at this moment."; values[i] = ((IndexQuery.ExactPredicate) predicates[i]).value(); } - return seek( values ); + result = seek( values ); + break; case exists: for ( IndexQuery p : predicates ) { @@ -111,31 +115,38 @@ public PrimitiveLongIterator query( IndexQuery... predicates ) throws IndexNotAp "Exists followed by another query predicate type is not supported." ); } } - return scan(); + result = scan(); + break; case rangeNumeric: assertNotComposite( predicates ); IndexQuery.NumberRangePredicate np = (IndexQuery.NumberRangePredicate) predicate; - return rangeSeekByNumberInclusive( np.from(), np.to() ); + result = rangeSeekByNumberInclusive( np.from(), np.to() ); + break; case rangeString: assertNotComposite( predicates ); IndexQuery.StringRangePredicate sp = (IndexQuery.StringRangePredicate) predicate; - return rangeSeekByString( sp.from(), sp.fromInclusive(), sp.to(), sp.toInclusive() ); + result = rangeSeekByString( sp.from(), sp.fromInclusive(), sp.to(), sp.toInclusive() ); + break; case stringPrefix: assertNotComposite( predicates ); IndexQuery.StringPrefixPredicate spp = (IndexQuery.StringPrefixPredicate) predicate; - return rangeSeekByPrefix( spp.prefix() ); + result = rangeSeekByPrefix( spp.prefix() ); + break; case stringContains: assertNotComposite( predicates ); IndexQuery.StringContainsPredicate scp = (IndexQuery.StringContainsPredicate) predicate; - return containsString( scp.contains() ); + result = containsString( scp.contains() ); + break; case stringSuffix: assertNotComposite( predicates ); IndexQuery.StringSuffixPredicate ssp = (IndexQuery.StringSuffixPredicate) predicate; - return endsWith( ssp.suffix() ); + result = endsWith( ssp.suffix() ); + break; default: // todo figure out a more specific exception throw new RuntimeException( "Index query not supported: " + Arrays.toString( predicates ) ); } + return PrimitiveLongCollections.resourceIterator( result, null); } @Override diff --git a/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/reader/PartitionedIndexReaderTest.java b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/reader/PartitionedIndexReaderTest.java index 5b0460d8ce838..35a1f2f7164af 100644 --- a/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/reader/PartitionedIndexReaderTest.java +++ b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/reader/PartitionedIndexReaderTest.java @@ -29,11 +29,12 @@ import java.util.List; import org.neo4j.collection.primitive.PrimitiveLongCollections; +import org.neo4j.collection.primitive.PrimitiveLongResourceCollections; import org.neo4j.collection.primitive.PrimitiveLongSet; import org.neo4j.helpers.TaskCoordinator; +import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException; import org.neo4j.kernel.api.impl.index.partition.PartitionSearcher; -import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.kernel.api.schema.index.IndexDescriptor; import org.neo4j.kernel.api.schema.index.IndexDescriptorFactory; import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig; @@ -86,9 +87,9 @@ public void seekOverAllPartitions() throws Exception PartitionedIndexReader indexReader = createPartitionedReaderFromReaders(); IndexQuery.ExactPredicate query = IndexQuery.exact( 1, "Test" ); - when( indexReader1.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 1 ) ); - when( indexReader2.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 2 ) ); - when( indexReader3.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 3 ) ); + when( indexReader1.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 1 ) ); + when( indexReader2.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 2 ) ); + when( indexReader3.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 3 ) ); PrimitiveLongSet results = PrimitiveLongCollections.asSet( indexReader.query( query ) ); verifyResult( results ); @@ -100,9 +101,9 @@ public void rangeSeekByNumberOverPartitions() throws Exception PartitionedIndexReader indexReader = createPartitionedReaderFromReaders(); IndexQuery.NumberRangePredicate query = IndexQuery.range( 1, 1, true, 2, true ); - when( indexReader1.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 1 ) ); - when( indexReader2.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 2 ) ); - when( indexReader3.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 3 ) ); + when( indexReader1.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 1 ) ); + when( indexReader2.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 2 ) ); + when( indexReader3.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 3 ) ); PrimitiveLongSet results = PrimitiveLongCollections.asSet( indexReader.query( query ) ); @@ -115,9 +116,9 @@ public void rangeSeekByStringOverPartitions() throws Exception PartitionedIndexReader indexReader = createPartitionedReaderFromReaders(); IndexQuery.StringRangePredicate query = IndexQuery.range( 1, "a", false, "b", true ); - when( indexReader1.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 1 ) ); - when( indexReader2.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 2 ) ); - when( indexReader3.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 3 ) ); + when( indexReader1.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 1 ) ); + when( indexReader2.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 2 ) ); + when( indexReader3.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 3 ) ); PrimitiveLongSet results = PrimitiveLongCollections.asSet( indexReader.query( query ) ); @@ -129,9 +130,9 @@ public void rangeSeekByPrefixOverPartitions() throws Exception { PartitionedIndexReader indexReader = createPartitionedReaderFromReaders(); IndexQuery.StringPrefixPredicate query = IndexQuery.stringPrefix( 1, "prefix" ); - when( indexReader1.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 1 ) ); - when( indexReader2.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 2 ) ); - when( indexReader3.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 3 ) ); + when( indexReader1.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 1 ) ); + when( indexReader2.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 2 ) ); + when( indexReader3.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 3 ) ); PrimitiveLongSet results = PrimitiveLongCollections.asSet( indexReader.query( query ) ); verifyResult( results ); @@ -142,9 +143,9 @@ public void scanOverPartitions() throws Exception { PartitionedIndexReader indexReader = createPartitionedReaderFromReaders(); IndexQuery.ExistsPredicate query = IndexQuery.exists( 1 ); - when( indexReader1.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 1 ) ); - when( indexReader2.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 2 ) ); - when( indexReader3.query( query ) ).thenReturn( PrimitiveLongCollections.iterator( 3 ) ); + when( indexReader1.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 1 ) ); + when( indexReader2.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 2 ) ); + when( indexReader3.query( query ) ).thenReturn( PrimitiveLongResourceCollections.iterator( null, 3 ) ); PrimitiveLongSet results = PrimitiveLongCollections.asSet( indexReader.query( query ) ); verifyResult( results ); diff --git a/community/primitive-collections/src/main/java/org/neo4j/collection/primitive/PrimitiveLongResourceCollections.java b/community/primitive-collections/src/main/java/org/neo4j/collection/primitive/PrimitiveLongResourceCollections.java new file mode 100644 index 0000000000000..662b435739878 --- /dev/null +++ b/community/primitive-collections/src/main/java/org/neo4j/collection/primitive/PrimitiveLongResourceCollections.java @@ -0,0 +1,139 @@ +/* + * 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.collection.primitive; + +import java.util.Arrays; + +import org.neo4j.graphdb.Resource; + +import static org.neo4j.collection.primitive.PrimitiveLongCollections.resourceIterator; + +public class PrimitiveLongResourceCollections +{ + public static PrimitiveLongResourceIterator iterator( Resource resource, final long... items ) + { + return resourceIterator( PrimitiveLongCollections.iterator( items ), resource ); + } + + private static final PrimitiveLongResourceIterator EMPTY = new PrimitiveLongBaseResourceIterator( null ) + { + @Override + protected boolean fetchNext() + { + return false; + } + }; + + public static PrimitiveLongResourceIterator emptyIterator() + { + return EMPTY; + } + + public static PrimitiveLongResourceIterator concat( PrimitiveLongResourceIterator... primitiveLongResourceIterators ) + { + return concat( Arrays.asList( primitiveLongResourceIterators ) ); + } + + public static PrimitiveLongResourceIterator concat( Iterable primitiveLongResourceIterators ) + { + return new PrimitiveLongConcatingResourceIterator( primitiveLongResourceIterators ); + } + + public abstract static class PrimitiveLongBaseResourceIterator extends PrimitiveLongCollections.PrimitiveLongBaseIterator + implements PrimitiveLongResourceIterator + { + private Resource resource; + + PrimitiveLongBaseResourceIterator( Resource resource ) + { + this.resource = resource; + } + + @Override + public void close() + { + if ( resource != null ) + { + resource.close(); + resource = null; + } + } + } + + public static class PrimitiveLongConcatingResourceIterator extends PrimitiveLongCollections.PrimitiveLongConcatingIterator + implements PrimitiveLongResourceIterator + { + private final Iterable iterators; + private volatile boolean closed; + + PrimitiveLongConcatingResourceIterator( Iterable iterators ) + { + super( iterators.iterator() ); + this.iterators = iterators; + } + + @Override + protected boolean fetchNext() + { + return !closed && super.fetchNext(); + } + + @Override + public void close() + { + if ( !closed ) + { + closed = true; + closeAll(); + } + } + + // Copied from IOUtils + private void closeAll() + { + Throwable closeThrowable = null; + for ( Resource resource : iterators ) + { + if ( resource != null ) + { + try + { + resource.close(); + } + catch ( Throwable t ) + { + if ( closeThrowable == null ) + { + closeThrowable = t; + } + else + { + closeThrowable.addSuppressed( t ); + } + } + } + } + if ( closeThrowable != null ) + { + throw new RuntimeException( "Exception closing multiple resources", closeThrowable ); + } + } + } +}