From 8a393f2aea730cdd267e7ebffb0a17426ad2af8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mattias=20Finn=C3=A9?= Date: Thu, 5 Jul 2018 13:56:35 +0200 Subject: [PATCH] Allows import monitor to notice use of PageCache for caching Caching for temporary importer data structures, that is. This allows user to become aware of the fact that an performance of an import can degrade heavily when that happens. Some amount of information is also provided as to why of/off-heap allocators couldn't be used. --- .../kernel/impl/store/CountsComputer.java | 2 +- .../participant/CountsMigrator.java | 2 +- .../unsafe/impl/batchimport/ImportLogic.java | 7 +- .../cache/ChunkedNumberArrayFactory.java | 8 +- .../batchimport/cache/NumberArrayFactory.java | 65 ++++++++++++--- .../cache/PageCacheArrayFactoryMonitor.java | 72 +++++++++++++++++ .../HumanUnderstandableExecutionMonitor.java | 26 ++++-- .../impl/batchimport/cache/ByteArrayTest.java | 3 +- .../impl/batchimport/cache/IntArrayTest.java | 3 +- .../impl/batchimport/cache/LongArrayTest.java | 3 +- .../cache/NumberArrayFactoryTest.java | 57 ++++++++++--- .../batchimport/cache/NumberArrayTest.java | 8 +- .../PageCacheArrayFactoryMonitorTest.java | 81 +++++++++++++++++++ .../cache/PageCacheLongArrayTest.java | 2 +- 14 files changed, 294 insertions(+), 45 deletions(-) create mode 100644 community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheArrayFactoryMonitor.java create mode 100644 community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheArrayFactoryMonitorTest.java diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CountsComputer.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CountsComputer.java index f2ef15480c103..2785d55558318 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CountsComputer.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CountsComputer.java @@ -61,7 +61,7 @@ public CountsComputer( NeoStores stores, PageCache pageCache ) stores.getNodeStore(), stores.getRelationshipStore(), (int) stores.getLabelTokenStore().getHighId(), (int) stores.getRelationshipTypeTokenStore().getHighId(), - NumberArrayFactory.auto( pageCache, stores.getStoreDir(), true ) ); + NumberArrayFactory.auto( pageCache, stores.getStoreDir(), true, NumberArrayFactory.NO_MONITOR ) ); } public CountsComputer( long lastCommittedTransactionId, NodeStore nodes, RelationshipStore relationships, diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/CountsMigrator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/CountsMigrator.java index 9654bbe09f1dd..085fa032fc62c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/CountsMigrator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/storemigration/participant/CountsMigrator.java @@ -178,7 +178,7 @@ private void rebuildCountsFromScratch( File storeDirToReadFrom, File migrationDi int highLabelId = (int) neoStores.getLabelTokenStore().getHighId(); int highRelationshipTypeId = (int) neoStores.getRelationshipTypeTokenStore().getHighId(); CountsComputer initializer = new CountsComputer( lastTxId, nodeStore, relationshipStore, highLabelId, - highRelationshipTypeId, NumberArrayFactory.auto( pageCache, migrationDir, true ), + highRelationshipTypeId, NumberArrayFactory.auto( pageCache, migrationDir, true, NumberArrayFactory.NO_MONITOR ), progressMonitor ); life.add( new CountsTracker( logProvider, fileSystem, pageCache, config, storeFileBase, EmptyVersionContextSupplier.EMPTY ).setInitializer( initializer ) ); diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/ImportLogic.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/ImportLogic.java index 41d94046f21f4..44f9b5f9b15da 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/ImportLogic.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/ImportLogic.java @@ -51,6 +51,7 @@ import org.neo4j.unsafe.impl.batchimport.cache.NodeRelationshipCache; import org.neo4j.unsafe.impl.batchimport.cache.NodeType; import org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory; +import org.neo4j.unsafe.impl.batchimport.cache.PageCacheArrayFactoryMonitor; import org.neo4j.unsafe.impl.batchimport.cache.idmapping.IdMapper; import org.neo4j.unsafe.impl.batchimport.input.CachedInput; import org.neo4j.unsafe.impl.batchimport.input.Collector; @@ -158,6 +159,7 @@ public void insufficientAvailableMemory( long estimatedCacheSize, long optimalMi private NodeLabelsCache nodeLabelsCache; private long startTime; private InputCache inputCache; + private PageCacheArrayFactoryMonitor numberArrayFactoryMonitor; private NumberArrayFactory numberArrayFactory; private Collector badCollector; private IdMapper idMapper; @@ -195,7 +197,8 @@ public void initialize( Input input ) throws IOException startTime = currentTimeMillis(); inputCache = new InputCache( fileSystem, storeDir, recordFormats, toIntExact( mebiBytes( 1 ) ) ); this.input = CachedInput.cacheAsNecessary( input, inputCache ); - numberArrayFactory = auto( neoStore.getPageCache(), storeDir, config.allowCacheAllocationOnHeap() ); + numberArrayFactoryMonitor = new PageCacheArrayFactoryMonitor(); + numberArrayFactory = auto( neoStore.getPageCache(), storeDir, config.allowCacheAllocationOnHeap(), numberArrayFactoryMonitor ); badCollector = input.badCollector(); // Some temporary caches and indexes in the import idMapper = input.idMapper( numberArrayFactory ); @@ -208,7 +211,7 @@ public void initialize( Input input ) throws IOException nodeRelationshipCache.memoryEstimation( inputEstimates.numberOfNodes() ), idMapper.memoryEstimation( inputEstimates.numberOfNodes() ) ); - dependencies.satisfyDependencies( inputEstimates, idMapper, neoStore, nodeRelationshipCache ); + dependencies.satisfyDependencies( inputEstimates, idMapper, neoStore, nodeRelationshipCache, numberArrayFactoryMonitor ); if ( neoStore.determineDoubleRelationshipRecordUnits( inputEstimates ) ) { diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/ChunkedNumberArrayFactory.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/ChunkedNumberArrayFactory.java index 57d21b43c1aef..803c766b991d7 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/ChunkedNumberArrayFactory.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/ChunkedNumberArrayFactory.java @@ -34,14 +34,14 @@ public class ChunkedNumberArrayFactory extends NumberArrayFactory.Adapter private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - Short.MAX_VALUE; private final NumberArrayFactory delegate; - ChunkedNumberArrayFactory() + ChunkedNumberArrayFactory( Monitor monitor ) { - this( OFF_HEAP, HEAP ); + this( monitor, OFF_HEAP, HEAP ); } - ChunkedNumberArrayFactory( NumberArrayFactory... delegateList ) + ChunkedNumberArrayFactory( Monitor monitor, NumberArrayFactory... delegateList ) { - delegate = new Auto( delegateList ); + delegate = new Auto( monitor, delegateList ); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayFactory.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayFactory.java index b4bb6a23c44b7..04d27f95ff63c 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayFactory.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayFactory.java @@ -24,6 +24,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.function.Function; import org.neo4j.helpers.Exceptions; @@ -42,6 +43,23 @@ */ public interface NumberArrayFactory { + interface Monitor + { + /** + * Notifies about a successful allocation where information about both successful and failed attempts are included. + * + * @param memory amount of memory for this allocation. + * @param successfulFactory the {@link NumberArrayFactory} which proved successful allocating this amount of memory. + * @param attemptedAllocationFailures list of failed attempts to allocate this memory in other allocators. + */ + void allocationSuccessful( long memory, NumberArrayFactory successfulFactory, Iterable attemptedAllocationFailures ); + } + + Monitor NO_MONITOR = ( memory, successfulFactory, attemptedAllocationFailures ) -> + { + // no-op + }; + /** * Puts arrays inside the heap. */ @@ -107,12 +125,12 @@ public String toString() * ({@link #newLongArray(long, long)} and {@link #newIntArray(long, int)} into smaller chunks where * some can live on heap and some off heap. */ - NumberArrayFactory CHUNKED_FIXED_SIZE = new ChunkedNumberArrayFactory(); + NumberArrayFactory CHUNKED_FIXED_SIZE = new ChunkedNumberArrayFactory( NumberArrayFactory.NO_MONITOR ); /** * {@link Auto} factory which uses JVM stats for gathering information about available memory. */ - NumberArrayFactory AUTO_WITHOUT_PAGECACHE = new Auto( OFF_HEAP, HEAP, CHUNKED_FIXED_SIZE ); + NumberArrayFactory AUTO_WITHOUT_PAGECACHE = new Auto( NumberArrayFactory.NO_MONITOR, OFF_HEAP, HEAP, CHUNKED_FIXED_SIZE ); /** * {@link Auto} factory which has a page cache backed number array as final fallback, in order to prevent OOM @@ -121,15 +139,16 @@ public String toString() * @param dir directory where cached files are placed. * @param allowHeapAllocation whether or not to allow allocation on heap. Otherwise allocation is restricted * to off-heap and the page cache fallback. This to be more in control of available space in the heap at all times. + * @param monitor for monitoring successful and failed allocations and which factory was selected. * @return a {@link NumberArrayFactory} which tries to allocation off-heap, then potentially on heap * and lastly falls back to allocating inside the given {@code pageCache}. */ - static NumberArrayFactory auto( PageCache pageCache, File dir, boolean allowHeapAllocation ) + static NumberArrayFactory auto( PageCache pageCache, File dir, boolean allowHeapAllocation, Monitor monitor ) { PageCachedNumberArrayFactory pagedArrayFactory = new PageCachedNumberArrayFactory( pageCache, dir ); - ChunkedNumberArrayFactory chunkedArrayFactory = new ChunkedNumberArrayFactory( + ChunkedNumberArrayFactory chunkedArrayFactory = new ChunkedNumberArrayFactory( monitor, allocationAlternatives( allowHeapAllocation, pagedArrayFactory ) ); - return new Auto( allocationAlternatives( allowHeapAllocation, chunkedArrayFactory ) ); + return new Auto( monitor, allocationAlternatives( allowHeapAllocation, chunkedArrayFactory ) ); } /** @@ -148,17 +167,41 @@ static NumberArrayFactory[] allocationAlternatives( boolean allowHeapAllocation, return result.toArray( new NumberArrayFactory[result.size()] ); } + class AllocationFailure + { + private final Throwable failure; + private final NumberArrayFactory factory; + + AllocationFailure( Throwable failure, NumberArrayFactory factory ) + { + this.failure = failure; + this.factory = factory; + } + + public Throwable getFailure() + { + return failure; + } + + public NumberArrayFactory getFactory() + { + return factory; + } + } + /** * Looks at available memory and decides where the requested array fits best. Tries to allocate the whole * array with the first candidate, falling back to others as needed. */ class Auto extends Adapter { - + private final Monitor monitor; private final NumberArrayFactory[] candidates; - public Auto( NumberArrayFactory... candidates ) + public Auto( Monitor monitor, NumberArrayFactory... candidates ) { + Objects.requireNonNull( monitor ); + this.monitor = monitor; this.candidates = candidates; } @@ -183,6 +226,7 @@ public ByteArray newByteArray( long length, byte[] defaultValue, long base ) private > T tryAllocate( long length, int itemSize, Function allocator ) { + List failures = new ArrayList<>(); OutOfMemoryError error = null; for ( NumberArrayFactory candidate : candidates ) { @@ -190,7 +234,9 @@ private > T tryAllocate( long length, int ite { try { - return allocator.apply( candidate ); + T array = allocator.apply( candidate ); + monitor.allocationSuccessful( length * itemSize, candidate, failures ); + return array; } catch ( ArithmeticException e ) { @@ -208,6 +254,7 @@ private > T tryAllocate( long length, int ite e.addSuppressed( error ); error = e; } + failures.add( new AllocationFailure( e, candidate ) ); } } throw error( length, itemSize, error ); @@ -302,7 +349,6 @@ default ByteArray newByteArray( long length, byte[] defaultValue ) abstract class Adapter implements NumberArrayFactory { - @Override public IntArray newDynamicIntArray( long chunkSize, int defaultValue ) { @@ -320,6 +366,5 @@ public ByteArray newDynamicByteArray( long chunkSize, byte[] defaultValue ) { return new DynamicByteArray( this, chunkSize, defaultValue ); } - } } diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheArrayFactoryMonitor.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheArrayFactoryMonitor.java new file mode 100644 index 0000000000000..82c3148a7993e --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheArrayFactoryMonitor.java @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2002-2018 "Neo4j," + * Neo4j Sweden AB [http://neo4j.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.unsafe.impl.batchimport.cache; + +import java.util.concurrent.atomic.AtomicReference; + +import org.neo4j.io.pagecache.PageCache; + +import static java.lang.String.format; +import static org.neo4j.helpers.Format.bytes; + +public class PageCacheArrayFactoryMonitor implements NumberArrayFactory.Monitor +{ + // This field is designed to allow multiple threads setting it concurrently, where one of those will win and either one is fine + // because this monitor mostly revolves around highlighting the fact that the page cache number array is in use at all. + private final AtomicReference failedFactoriesDescription = new AtomicReference<>(); + + @Override + public void allocationSuccessful( long memory, NumberArrayFactory successfulFactory, + Iterable attemptedAllocationFailures ) + { + if ( successfulFactory instanceof PageCachedNumberArrayFactory ) + { + StringBuilder builder = + new StringBuilder( format( "Memory allocation of %s ended up in page cache, which may impact performance negatively", bytes( memory ) ) ); + attemptedAllocationFailures.forEach( + failure -> builder.append( format( "%n%s: %s", failure.getFactory().toString(), failure.getFailure().toString() ) ) ); + failedFactoriesDescription.compareAndSet( null, builder.toString() ); + } + } + + /** + * Called by user-facing progress monitor at arbitrary points to get information about whether or not there has been + * one or more {@link NumberArrayFactory} allocations backed by the {@link PageCache}, this because it severely affects + * performance. Calling this method clears the failure description, if any. + * + * @return if there have been {@link NumberArrayFactory} allocations backed by the {@link PageCache} since the last call to this method + * then a description of why it was chosen is returned, otherwise {@code null}. + */ + public String pageCacheAllocationOrNull() + { + String failure = failedFactoriesDescription.get(); + try + { + return failure; + } + finally + { + if ( failure != null ) + { + failedFactoriesDescription.compareAndSet( failure, null ); + } + } + } +} diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/staging/HumanUnderstandableExecutionMonitor.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/staging/HumanUnderstandableExecutionMonitor.java index 4f56346dc0164..2d6727eb6d5e1 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/staging/HumanUnderstandableExecutionMonitor.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/staging/HumanUnderstandableExecutionMonitor.java @@ -32,6 +32,7 @@ import org.neo4j.unsafe.impl.batchimport.ScanAndCacheGroupsStage; import org.neo4j.unsafe.impl.batchimport.SparseNodeFirstRelationshipStage; import org.neo4j.unsafe.impl.batchimport.cache.NodeRelationshipCache; +import org.neo4j.unsafe.impl.batchimport.cache.PageCacheArrayFactoryMonitor; import org.neo4j.unsafe.impl.batchimport.cache.idmapping.IdMapper; import org.neo4j.unsafe.impl.batchimport.input.Input; import org.neo4j.unsafe.impl.batchimport.input.Input.Estimates; @@ -70,7 +71,7 @@ public interface ExternalMonitor boolean somethingElseBrokeMyNiceOutput(); } - public static final ExternalMonitor NO_EXTERNAL_MONITOR = () -> false; + static final ExternalMonitor NO_EXTERNAL_MONITOR = () -> false; enum ImportStage { @@ -95,6 +96,7 @@ enum ImportStage private final ExternalMonitor externalMonitor; private DependencyResolver dependencyResolver; private boolean newInternalStage; + private PageCacheArrayFactoryMonitor pageCacheArrayFactoryMonitor; // progress of current stage private long goal; @@ -103,7 +105,7 @@ enum ImportStage private ImportStage currentStage; private long lastReportTime; - public HumanUnderstandableExecutionMonitor( PrintStream out, Monitor monitor, ExternalMonitor externalMonitor ) + HumanUnderstandableExecutionMonitor( PrintStream out, Monitor monitor, ExternalMonitor externalMonitor ) { this.out = out; this.monitor = monitor; @@ -118,6 +120,7 @@ public void initialize( DependencyResolver dependencyResolver ) BatchingNeoStores neoStores = dependencyResolver.resolveDependency( BatchingNeoStores.class ); IdMapper idMapper = dependencyResolver.resolveDependency( IdMapper.class ); NodeRelationshipCache nodeRelationshipCache = dependencyResolver.resolveDependency( NodeRelationshipCache.class ); + pageCacheArrayFactoryMonitor = dependencyResolver.resolveDependency( PageCacheArrayFactoryMonitor.class ); long biggestCacheMemory = estimatedCacheSize( neoStores, nodeRelationshipCache.memoryEstimation( estimates.numberOfNodes() ), @@ -130,7 +133,6 @@ ESTIMATED_NUMBER_OF_RELATIONSHIP_PROPERTIES, count( estimates.numberOfRelationsh ESTIMATED_DISK_SPACE_USAGE, bytes( nodesDiskUsage( estimates, neoStores ) + relationshipsDiskUsage( estimates, neoStores ) + - // TODO also add some padding to include relationship groups? estimates.sizeOfNodeProperties() + estimates.sizeOfRelationshipProperties() ), ESTIMATED_REQUIRED_MEMORY_USAGE, bytes( biggestCacheMemory ) ); out.println(); @@ -216,13 +218,11 @@ else if ( includeStage( execution ) ) private void endPrevious() { - updateProgress( goal ); // previous ended - // TODO print some end stats for this stage? + updateProgress( goal ); } private void initializeNodeImport( Estimates estimates, IdMapper idMapper, BatchingNeoStores neoStores ) { - // TODO how to handle UNKNOWN? long numberOfNodes = estimates.numberOfNodes(); printStageHeader( "(1/4) Node import", ESTIMATED_NUMBER_OF_NODES, count( numberOfNodes ), @@ -340,7 +340,6 @@ private void updateProgress( long progress ) } } - // TODO not quite right this.progress = max( this.progress, progress ); } @@ -378,6 +377,19 @@ private void printDots( int from, int target ) } out.print( dotChar ); current++; + + printPageCacheAllocationWarningIfUsed(); + } + } + + private void printPageCacheAllocationWarningIfUsed() + { + String allocation = pageCacheArrayFactoryMonitor.pageCacheAllocationOrNull(); + if ( allocation != null ) + { + System.err.println(); + System.err.println( "WARNING:" ); + System.err.println( allocation ); } } diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/ByteArrayTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/ByteArrayTest.java index 511d85464b319..3f0a05854774a 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/ByteArrayTest.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/ByteArrayTest.java @@ -41,6 +41,7 @@ import static org.junit.Assert.assertEquals; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.AUTO_WITHOUT_PAGECACHE; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.HEAP; +import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.NO_MONITOR; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.OFF_HEAP; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.auto; @@ -57,7 +58,7 @@ public static Collection> data() throws IOException fixture = prepareDirectoryAndPageCache( ByteArrayTest.class ); PageCache pageCache = fixture.pageCache; File dir = fixture.directory; - NumberArrayFactory autoWithPageCacheFallback = auto( pageCache, dir, true ); + NumberArrayFactory autoWithPageCacheFallback = auto( pageCache, dir, true, NO_MONITOR ); NumberArrayFactory pageCacheArrayFactory = new PageCachedNumberArrayFactory( pageCache, dir ); int chunkSize = LENGTH / ChunkedNumberArrayFactory.MAGIC_CHUNK_COUNT; return Arrays.asList( diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/IntArrayTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/IntArrayTest.java index 43a92d97b141f..260d4736caba5 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/IntArrayTest.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/IntArrayTest.java @@ -37,6 +37,7 @@ import static java.lang.System.currentTimeMillis; import static org.junit.Assert.assertEquals; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.HEAP; +import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.NO_MONITOR; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.OFF_HEAP; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.auto; @@ -106,7 +107,7 @@ public static Collection data() throws IOException fixture = prepareDirectoryAndPageCache( IntArrayTest.class ); PageCache pageCache = fixture.pageCache; File dir = fixture.directory; - NumberArrayFactory autoWithPageCacheFallback = auto( pageCache, dir, true ); + NumberArrayFactory autoWithPageCacheFallback = auto( pageCache, dir, true, NO_MONITOR ); NumberArrayFactory pageCacheArrayFactory = new PageCachedNumberArrayFactory( pageCache, dir ); return Arrays.asList( HEAP, OFF_HEAP, autoWithPageCacheFallback, pageCacheArrayFactory ); } diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/LongArrayTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/LongArrayTest.java index 02dba9e5d9792..a150c24755fb1 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/LongArrayTest.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/LongArrayTest.java @@ -37,6 +37,7 @@ import static java.lang.System.currentTimeMillis; import static org.junit.Assert.assertEquals; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.HEAP; +import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.NO_MONITOR; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.OFF_HEAP; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.auto; @@ -106,7 +107,7 @@ public static Collection data() throws IOException fixture = prepareDirectoryAndPageCache( LongArrayTest.class ); PageCache pageCache = fixture.pageCache; File dir = fixture.directory; - NumberArrayFactory autoWithPageCacheFallback = auto( pageCache, dir, true ); + NumberArrayFactory autoWithPageCacheFallback = auto( pageCache, dir, true, NO_MONITOR ); NumberArrayFactory pageCacheArrayFactory = new PageCachedNumberArrayFactory( pageCache, dir ); return Arrays.asList( HEAP, OFF_HEAP, autoWithPageCacheFallback, pageCacheArrayFactory ); } diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayFactoryTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayFactoryTest.java index d16d211410d5a..7a55b661d9151 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayFactoryTest.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayFactoryTest.java @@ -21,8 +21,10 @@ import org.junit.Test; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -34,6 +36,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.neo4j.helpers.collection.Iterables.single; +import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.NO_MONITOR; public class NumberArrayFactoryTest { @@ -43,7 +47,7 @@ public class NumberArrayFactoryTest public void shouldPickFirstAvailableCandidateLongArray() { // GIVEN - NumberArrayFactory factory = new NumberArrayFactory.Auto( NumberArrayFactory.HEAP ); + NumberArrayFactory factory = new NumberArrayFactory.Auto( NO_MONITOR, NumberArrayFactory.HEAP ); // WHEN LongArray array = factory.newLongArray( KILO, -1 ); @@ -60,7 +64,7 @@ public void shouldPickFirstAvailableCandidateLongArrayWhenSomeDontHaveEnoughMemo // GIVEN NumberArrayFactory lowMemoryFactory = mock( NumberArrayFactory.class ); doThrow( OutOfMemoryError.class ).when( lowMemoryFactory ).newLongArray( anyLong(), anyLong(), anyLong() ); - NumberArrayFactory factory = new NumberArrayFactory.Auto( lowMemoryFactory, NumberArrayFactory.HEAP ); + NumberArrayFactory factory = new NumberArrayFactory.Auto( NO_MONITOR, lowMemoryFactory, NumberArrayFactory.HEAP ); // WHEN LongArray array = factory.newLongArray( KILO, -1 ); @@ -76,9 +80,10 @@ public void shouldPickFirstAvailableCandidateLongArrayWhenSomeDontHaveEnoughMemo public void shouldThrowOomOnNotEnoughMemory() { // GIVEN + FailureMonitor monitor = new FailureMonitor(); NumberArrayFactory lowMemoryFactory = mock( NumberArrayFactory.class ); doThrow( OutOfMemoryError.class ).when( lowMemoryFactory ).newLongArray( anyLong(), anyLong(), anyLong() ); - NumberArrayFactory factory = new NumberArrayFactory.Auto( lowMemoryFactory ); + NumberArrayFactory factory = new NumberArrayFactory.Auto( monitor, lowMemoryFactory ); // WHEN try @@ -89,6 +94,7 @@ public void shouldThrowOomOnNotEnoughMemory() catch ( OutOfMemoryError e ) { // THEN OK + assertFalse( monitor.called ); } } @@ -96,7 +102,8 @@ public void shouldThrowOomOnNotEnoughMemory() public void shouldPickFirstAvailableCandidateIntArray() { // GIVEN - NumberArrayFactory factory = new NumberArrayFactory.Auto( NumberArrayFactory.HEAP ); + FailureMonitor monitor = new FailureMonitor(); + NumberArrayFactory factory = new NumberArrayFactory.Auto( monitor, NumberArrayFactory.HEAP ); // WHEN IntArray array = factory.newIntArray( KILO, -1 ); @@ -105,6 +112,8 @@ public void shouldPickFirstAvailableCandidateIntArray() // THEN assertTrue( array instanceof HeapIntArray ); assertEquals( 12345, array.get( KILO - 10 ) ); + assertEquals( NumberArrayFactory.HEAP, monitor.successfulFactory ); + assertFalse( monitor.attemptedAllocationFailures.iterator().hasNext() ); } @Test @@ -113,7 +122,7 @@ public void shouldPickFirstAvailableCandidateIntArrayWhenSomeDontHaveEnoughMemor // GIVEN NumberArrayFactory lowMemoryFactory = mock( NumberArrayFactory.class ); doThrow( OutOfMemoryError.class ).when( lowMemoryFactory ).newIntArray( anyLong(), anyInt(), anyLong() ); - NumberArrayFactory factory = new NumberArrayFactory.Auto( lowMemoryFactory, NumberArrayFactory.HEAP ); + NumberArrayFactory factory = new NumberArrayFactory.Auto( NO_MONITOR, lowMemoryFactory, NumberArrayFactory.HEAP ); // WHEN IntArray array = factory.newIntArray( KILO, -1 ); @@ -126,35 +135,41 @@ public void shouldPickFirstAvailableCandidateIntArrayWhenSomeDontHaveEnoughMemor } @Test - public void shouldEvenCatchOtherExceptionsAndTryNext() + public void shouldCatchArithmeticExceptionsAndTryNext() { // GIVEN NumberArrayFactory throwingMemoryFactory = mock( NumberArrayFactory.class ); - doThrow( ArithmeticException.class ).when( throwingMemoryFactory ) - .newByteArray( anyLong(), any( byte[].class ), anyLong() ); - NumberArrayFactory factory = new NumberArrayFactory.Auto( throwingMemoryFactory, NumberArrayFactory.HEAP ); + ArithmeticException failure = new ArithmeticException( "This is an artificial failure" ); + doThrow( failure ).when( throwingMemoryFactory ).newByteArray( anyLong(), any( byte[].class ), anyLong() ); + FailureMonitor monitor = new FailureMonitor(); + NumberArrayFactory factory = new NumberArrayFactory.Auto( monitor, throwingMemoryFactory, NumberArrayFactory.HEAP ); + int itemSize = 4; // WHEN - ByteArray array = factory.newByteArray( KILO, new byte[4], 0 ); + ByteArray array = factory.newByteArray( KILO, new byte[itemSize], 0 ); array.setInt( KILO - 10, 0, 12345 ); // THEN verify( throwingMemoryFactory, times( 1 ) ).newByteArray( eq( KILO ), any( byte[].class ), eq( 0L ) ); assertTrue( array instanceof HeapByteArray ); assertEquals( 12345, array.getInt( KILO - 10, 0 ) ); + assertEquals( KILO * itemSize, monitor.memory ); + assertEquals( NumberArrayFactory.HEAP, monitor.successfulFactory ); + assertEquals( throwingMemoryFactory, single( monitor.attemptedAllocationFailures ).getFactory() ); + assertThat( single( monitor.attemptedAllocationFailures ).getFailure().getMessage(), containsString( failure.getMessage() ) ); } @Test public void heapArrayShouldAllowVeryLargeBases() { - NumberArrayFactory factory = new NumberArrayFactory.Auto( NumberArrayFactory.HEAP ); + NumberArrayFactory factory = new NumberArrayFactory.Auto( NO_MONITOR, NumberArrayFactory.HEAP ); verifyVeryLargeBaseSupport( factory ); } @Test public void offHeapArrayShouldAllowVeryLargeBases() { - NumberArrayFactory factory = new NumberArrayFactory.Auto( NumberArrayFactory.OFF_HEAP ); + NumberArrayFactory factory = new NumberArrayFactory.Auto( NO_MONITOR, NumberArrayFactory.OFF_HEAP ); verifyVeryLargeBaseSupport( factory ); } @@ -168,4 +183,22 @@ private void verifyVeryLargeBaseSupport( NumberArrayFactory factory ) assertThat( factory.newIntArray( 10, 1, base ).get( base + 1 ), is( 1 ) ); assertThat( factory.newLongArray( 10, 1, base ).get( base + 1 ), is( 1L ) ); } + + private static class FailureMonitor implements NumberArrayFactory.Monitor + { + private boolean called; + private long memory; + private NumberArrayFactory successfulFactory; + private Iterable attemptedAllocationFailures; + + @Override + public void allocationSuccessful( long memory, NumberArrayFactory successfulFactory, + Iterable attemptedAllocationFailures ) + { + this.memory = memory; + this.successfulFactory = successfulFactory; + this.attemptedAllocationFailures = attemptedAllocationFailures; + this.called = true; + } + } } diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayTest.java index 5e3e2d428e3cd..62492d6d04d08 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayTest.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/NumberArrayTest.java @@ -39,14 +39,14 @@ import org.neo4j.io.pagecache.PageCache; import org.neo4j.test.rule.RandomRule; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; import static java.lang.Integer.max; - +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.AUTO_WITHOUT_PAGECACHE; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.CHUNKED_FIXED_SIZE; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.HEAP; +import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.NO_MONITOR; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.OFF_HEAP; import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.auto; @@ -82,7 +82,7 @@ public static Collection arrays() throws IOException factories.put( "OFF_HEAP", OFF_HEAP ); factories.put( "AUTO_WITHOUT_PAGECACHE", AUTO_WITHOUT_PAGECACHE ); factories.put( "CHUNKED_FIXED_SIZE", CHUNKED_FIXED_SIZE ); - factories.put( "autoWithPageCacheFallback", auto( pageCache, dir, true ) ); + factories.put( "autoWithPageCacheFallback", auto( pageCache, dir, true, NO_MONITOR ) ); factories.put( "PageCachedNumberArrayFactory", new PageCachedNumberArrayFactory( pageCache, dir ) ); for ( Map.Entry entry : factories.entrySet() ) { diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheArrayFactoryMonitorTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheArrayFactoryMonitorTest.java new file mode 100644 index 0000000000000..44e2cbe1e3a9e --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheArrayFactoryMonitorTest.java @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2002-2018 "Neo4j," + * Neo4j Sweden AB [http://neo4j.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.unsafe.impl.batchimport.cache; + +import org.junit.Test; + +import java.io.File; + +import org.neo4j.io.pagecache.PageCache; + +import static java.util.Arrays.asList; +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; +import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.HEAP; +import static org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory.OFF_HEAP; + +public class PageCacheArrayFactoryMonitorTest +{ + private final PageCachedNumberArrayFactory factory = new PageCachedNumberArrayFactory( mock( PageCache.class ), new File( "storeDir" ) ); + private final PageCacheArrayFactoryMonitor monitor = new PageCacheArrayFactoryMonitor(); + + @Test + public void shouldComposeFailureDescriptionForFailedCandidates() + { + // given + monitor.allocationSuccessful( 123, factory, asList( + new NumberArrayFactory.AllocationFailure( new OutOfMemoryError( "OOM1" ), HEAP ), + new NumberArrayFactory.AllocationFailure( new OutOfMemoryError( "OOM2" ), OFF_HEAP ) ) ); + + // when + String failure = monitor.pageCacheAllocationOrNull(); + + // then + assertThat( failure, containsString( "OOM1" ) ); + assertThat( failure, containsString( "OOM2" ) ); + } + + @Test + public void shouldClearFailureStateAfterAccessorCall() + { + // given + monitor.allocationSuccessful( 123, factory, asList( + new NumberArrayFactory.AllocationFailure( new OutOfMemoryError( "OOM1" ), HEAP ), + new NumberArrayFactory.AllocationFailure( new OutOfMemoryError( "OOM2" ), OFF_HEAP ) ) ); + + // when + String failure = monitor.pageCacheAllocationOrNull(); + String secondCall = monitor.pageCacheAllocationOrNull(); + + // then + assertNotNull( failure ); + assertNull( secondCall ); + } + + @Test + public void shouldReturnNullFailureOnNoFailure() + { + // then + assertNull( monitor.pageCacheAllocationOrNull() ); + } +} diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheLongArrayTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheLongArrayTest.java index 18b6c79cee037..66229accfdf8c 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheLongArrayTest.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/cache/PageCacheLongArrayTest.java @@ -65,7 +65,7 @@ public void verifyChunkingArrayWithPageCacheLongArray() { PageCache pageCache = pageCacheRule.getPageCache( fs ); File directory = dir.directory(); - NumberArrayFactory numberArrayFactory = NumberArrayFactory.auto( pageCache, directory, false ); + NumberArrayFactory numberArrayFactory = NumberArrayFactory.auto( pageCache, directory, false, NumberArrayFactory.NO_MONITOR ); try ( LongArray array = numberArrayFactory.newDynamicLongArray( COUNT / 1_000, 0 ) ) { verifyBehaviour( array );