diff --git a/community/io/src/test/java/org/neo4j/io/pagecache/RecordingPageCacheTracer.java b/community/io/src/test/java/org/neo4j/io/pagecache/RecordingPageCacheTracer.java index d21f83e74e21d..a975620008a28 100644 --- a/community/io/src/test/java/org/neo4j/io/pagecache/RecordingPageCacheTracer.java +++ b/community/io/src/test/java/org/neo4j/io/pagecache/RecordingPageCacheTracer.java @@ -23,6 +23,9 @@ import java.io.File; import java.io.IOException; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingQueue; @@ -37,22 +40,44 @@ public class RecordingPageCacheTracer implements PageCacheTracer { + private final Set> eventTypesToTrace = new HashSet<>(); private final BlockingQueue record = new LinkedBlockingQueue<>(); private CountDownLatch trapLatch; private Matcher trap; - public void pageFaulted( long filePageId, PageSwapper swapper ) + public RecordingPageCacheTracer() { - Fault event = new Fault( swapper, filePageId ); - record.add( event ); - trip( event ); + this( Evict.class, Fault.class ); } - public void evicted( long filePageId, PageSwapper swapper ) + @SafeVarargs + public RecordingPageCacheTracer( Class... eventTypesToTrace ) { - Evict event = new Evict( swapper, filePageId ); - record.add( event ); - trip( event ); + Collections.addAll( this.eventTypesToTrace, eventTypesToTrace ); + } + + private void pageFaulted( long filePageId, PageSwapper swapper ) + { + record( new Fault( swapper, filePageId ) ); + } + + private void evicted( long filePageId, PageSwapper swapper ) + { + record( new Evict( swapper, filePageId ) ); + } + + private void pinned( long filePageId, PageSwapper swapper ) + { + record( new Pin( swapper, filePageId ) ); + } + + private void record( Event event ) + { + if ( eventTypesToTrace.contains( event.getClass() ) ) + { + record.add( event ); + trip( event ); + } } @Override @@ -132,6 +157,7 @@ public void setCachePageId( int cachePageId ) @Override public void done() { + pinned( filePageId, swapper ); } }; } @@ -305,7 +331,7 @@ public String toString() public static class Fault extends Event { - public Fault( PageSwapper io, long pageId ) + private Fault( PageSwapper io, long pageId ) { super( io, pageId ); } @@ -313,7 +339,15 @@ public Fault( PageSwapper io, long pageId ) public static class Evict extends Event { - public Evict( PageSwapper io, long pageId ) + private Evict( PageSwapper io, long pageId ) + { + super( io, pageId ); + } + } + + public static class Pin extends Event + { + private Pin( PageSwapper io, long pageId ) { super( io, pageId ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/pagecache/StandalonePageCacheFactory.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/pagecache/StandalonePageCacheFactory.java index 239a9b9c232c3..0ffe985182aae 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/pagecache/StandalonePageCacheFactory.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/pagecache/StandalonePageCacheFactory.java @@ -46,15 +46,19 @@ public static PageCache createPageCache( FileSystemAbstraction fileSystem ) return createPageCache( fileSystem, Config.defaults() ); } - public static PageCache createPageCache( - FileSystemAbstraction fileSystem, Config config ) + public static PageCache createPageCache( FileSystemAbstraction fileSystem, Config config ) + { + return createPageCache( fileSystem, PageCacheTracer.NULL, config ); + } + + public static PageCache createPageCache( FileSystemAbstraction fileSystem, PageCacheTracer tracer, Config config ) { Config baseConfig = new Config( MapUtil.stringMap( GraphDatabaseSettings.pagecache_memory.name(), "8M" ) ); Config finalConfig = baseConfig.with( config.getParams() ); FormattedLogProvider logProvider = FormattedLogProvider.toOutputStream( System.err ); ConfiguringPageCacheFactory pageCacheFactory = new ConfiguringPageCacheFactory( - fileSystem, finalConfig, PageCacheTracer.NULL, logProvider.getLog( PageCache.class ) ); + fileSystem, finalConfig, tracer, logProvider.getLog( PageCache.class ) ); return pageCacheFactory.getOrCreatePageCache(); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java index e86288035324f..a73c78346ac09 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java @@ -1114,8 +1114,7 @@ public boolean next() record.setId( currentId ); long pageId = pageIdForRecord( currentId ); int offset = offsetForId( currentId ); - if ( pageId == pageCursor.getCurrentPageId() || - pageCursor.next( pageId ) ) + if ( pageCursor.next( pageId ) ) { do { diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java index 48b041f8350fa..cdaf722531864 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java @@ -19,73 +19,167 @@ */ package org.neo4j.kernel.impl.store; +import org.junit.Before; +import org.junit.ClassRule; import org.junit.Test; +import org.junit.rules.RuleChain; import org.mockito.InOrder; -import org.mockito.Mockito; import java.io.File; import java.io.IOException; +import org.neo4j.io.fs.DefaultFileSystemAbstraction; +import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.pagecache.PageCache; +import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PagedFile; +import org.neo4j.io.pagecache.RecordingPageCacheTracer; +import org.neo4j.io.pagecache.RecordingPageCacheTracer.Pin; +import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.impl.store.format.RecordFormat; +import org.neo4j.kernel.impl.store.format.lowlimit.LowLimitV3_0; +import org.neo4j.kernel.impl.store.id.DefaultIdGeneratorFactory; +import org.neo4j.kernel.impl.store.id.IdGenerator; import org.neo4j.kernel.impl.store.id.IdGeneratorFactory; import org.neo4j.kernel.impl.store.id.IdType; -import org.neo4j.kernel.configuration.Config; -import org.neo4j.kernel.impl.store.id.IdGenerator; import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; +import org.neo4j.kernel.impl.store.record.NodeRecord; +import org.neo4j.kernel.impl.store.record.RecordLoad; import org.neo4j.logging.LogProvider; import org.neo4j.logging.NullLogProvider; +import org.neo4j.test.PageCacheRule; +import org.neo4j.test.TargetDirectory; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; +import static org.neo4j.io.pagecache.PagedFile.PF_SHARED_READ_LOCK; +import static org.neo4j.io.pagecache.RecordingPageCacheTracer.Event; +import static org.neo4j.kernel.impl.store.record.Record.NO_NEXT_PROPERTY; +import static org.neo4j.kernel.impl.store.record.Record.NO_NEXT_RELATIONSHIP; +import static org.neo4j.test.TargetDirectory.testDirForTest; public class CommonAbstractStoreTest { - private static final NullLogProvider LOG = NullLogProvider.getInstance(); + private static final int PAGE_SIZE = 32; + private static final int RECORD_SIZE = 10; + private static final int HIGH_ID = 42; + + private final IdGenerator idGenerator = mock( IdGenerator.class ); private final IdGeneratorFactory idGeneratorFactory = mock( IdGeneratorFactory.class ); + private final PagedFile pageFile = mock( PagedFile.class ); private final PageCache pageCache = mock( PageCache.class ); private final Config config = Config.empty(); private final File storeFile = new File( "store" ); private final IdType idType = IdType.RELATIONSHIP; // whatever - @Test - public void shouldCloseStoreFileFirstAndIdGeneratorAfter() throws Throwable + private static final FileSystemAbstraction fs = new DefaultFileSystemAbstraction(); + private static final TargetDirectory.TestDirectory dir = testDirForTest( CommonAbstractStoreTest.class, fs ); + private static final PageCacheRule pageCacheRule = new PageCacheRule(); + + @ClassRule + public static final RuleChain ruleChain = RuleChain.outerRule( dir ).around( pageCacheRule ); + + @Before + public void setUpMocks() throws IOException { - // given - PagedFile storePagedFile = mock( PagedFile.class ); - when( pageCache.map( eq( storeFile ), anyInt() ) ).thenReturn( storePagedFile ); - IdGenerator idGenerator = mock( - IdGenerator.class ); when( idGeneratorFactory.open( any( File.class ), anyInt(), eq( idType ), anyInt(), anyInt() ) ) .thenReturn( idGenerator ); - RecordFormat recordFormat = Mockito.mock( RecordFormat.class ); - CommonAbstractStore store = new TheStore( storeFile, config, idType, idGeneratorFactory, pageCache, LOG, recordFormat ); - store.initialise( false ); - // this is needed to forget all interaction with the mocks during the construction of the store - reset( storePagedFile, idGenerator ); + when( pageFile.pageSize() ).thenReturn( PAGE_SIZE ); + when( pageCache.map( eq( storeFile ), anyInt() ) ).thenReturn( pageFile ); + } - InOrder inOrder = inOrder( storePagedFile, idGenerator ); + @Test + public void shouldCloseStoreFileFirstAndIdGeneratorAfter() throws Throwable + { + // given + TheStore store = newStore(); + InOrder inOrder = inOrder( pageFile, idGenerator ); // when store.close(); // then - inOrder.verify( storePagedFile, times( 1 ) ).close(); + inOrder.verify( pageFile, times( 1 ) ).close(); inOrder.verify( idGenerator, times( 1 ) ).close(); } - private static class TheStore extends CommonAbstractStore + @Test + public void recordCursorCallsNextOnThePageCursor() throws IOException + { + TheStore store = newStore(); + long recordId = 4; + long pageIdForRecord = store.pageIdForRecord( recordId ); + + PageCursor pageCursor = mock( PageCursor.class ); + when( pageCursor.getCurrentPageId() ).thenReturn( pageIdForRecord ); + when( pageCursor.next( anyInt() ) ).thenReturn( true ); + + RecordCursor cursor = store.newRecordCursor( new TheRecord( -1 ) ); + cursor.init( recordId, RecordLoad.FORCE, pageCursor ); + + cursor.next( recordId ); + + InOrder order = inOrder( pageCursor ); + order.verify( pageCursor ).next( pageIdForRecord ); + order.verify( pageCursor ).shouldRetry(); + } + + @Test + public void recordCursorPinsEachPageItReads() throws Exception { - public TheStore( File fileName, Config configuration, IdType idType, IdGeneratorFactory idGeneratorFactory, - PageCache pageCache, LogProvider logProvider, RecordFormat recordFormat ) + File storeFile = dir.file( "a" ); + RecordingPageCacheTracer tracer = new RecordingPageCacheTracer( Pin.class ); + PageCache pageCache = pageCacheRule.getPageCache( fs, tracer, Config.empty() ); + + try ( NodeStore store = new NodeStore( storeFile, Config.empty(), new DefaultIdGeneratorFactory( fs ), + pageCache, NullLogProvider.getInstance(), null, LowLimitV3_0.RECORD_FORMATS ) ) + { + store.initialise( true ); + assertNull( tracer.tryObserve( Event.class ) ); + + NodeRecord record = store.newRecord(); + record.setId( 0 ); + record.initialize( true, NO_NEXT_PROPERTY.intValue(), false, NO_NEXT_RELATIONSHIP.intValue(), 42 ); + store.updateRecord( record ); + assertNotNull( tracer.tryObserve( Pin.class ) ); + assertNull( tracer.tryObserve( Event.class ) ); + + long pageId = store.pageIdForRecord( record.getId() ); + try ( RecordCursor cursor = store.newRecordCursor( store.newRecord() ); + PageCursor pageCursor = store.storeFile.io( pageId, PF_SHARED_READ_LOCK ) ) + { + assertTrue( pageCursor.next( pageId ) ); + cursor.init( record.getId(), RecordLoad.NORMAL, pageCursor ); + assertTrue( cursor.next() ); + assertNotNull( tracer.tryObserve( Pin.class ) ); + assertNull( tracer.tryObserve( Event.class ) ); + } + } + } + + @SuppressWarnings( "unchecked" ) + private TheStore newStore() + { + RecordFormat recordFormat = mock( RecordFormat.class ); + LogProvider log = NullLogProvider.getInstance(); + TheStore store = new TheStore( storeFile, config, idType, idGeneratorFactory, pageCache, log, recordFormat ); + store.initialise( false ); + return store; + } + + private static class TheStore extends CommonAbstractStore + { + TheStore( File fileName, Config configuration, IdType idType, IdGeneratorFactory idGeneratorFactory, + PageCache pageCache, LogProvider logProvider, RecordFormat recordFormat ) { super( fileName, configuration, idType, idGeneratorFactory, pageCache, logProvider, "TheType", recordFormat, NoStoreHeaderFormat.NO_STORE_HEADER_FORMAT, "v1" ); @@ -99,18 +193,26 @@ protected void initialiseNewStoreFile( PagedFile file ) throws IOException @Override protected int determineRecordSize() { - return 10; + return RECORD_SIZE; } @Override public long scanForHighId() { - return 42; + return HIGH_ID; } @Override - public void accept( Processor processor, AbstractBaseRecord record ) throws Exception + public void accept( Processor processor, TheRecord record ) throws FAILURE + { + } + } + + private static class TheRecord extends AbstractBaseRecord + { + TheRecord( long id ) { + super( id ); } } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/PropertyRecordTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/PropertyRecordTest.java index ebc733d1d32f1..17cc162281bcc 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/PropertyRecordTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/PropertyRecordTest.java @@ -57,8 +57,8 @@ public void addingDuplicatePropertyBlockShouldOverwriteExisting() record.setPropertyBlock( blockB ); // Then the record should only contain a single block, because blockB overwrote blockA - List propertyBlocks = Iterables.asList( (Iterable)record ); - assertThat( propertyBlocks, hasItem( blockB )); + List propertyBlocks = Iterables.asList( (Iterable) record ); + assertThat( propertyBlocks, hasItem( blockB ) ); assertThat( propertyBlocks, hasSize( 1 ) ); } @@ -119,7 +119,43 @@ public void shouldBeAbleToRemoveBlocksDuringIteration() throws Exception assertFalse( iterator.hasNext() ); // and THEN there should only be the non-removed blocks left - assertEquals( blocks, Iterables.asSet( (Iterable) record ) ); + assertEquals( blocks, Iterables.asSet( record ) ); + } + + @Test + public void addLoadedBlock() + { + PropertyRecord record = new PropertyRecord( 42 ); + + addBlock( record, 1, 2 ); + addBlock( record, 3, 4 ); + + List blocks = Iterables.asList( record ); + assertEquals( 2, blocks.size() ); + assertEquals( 1, blocks.get( 0 ).getKeyIndexId() ); + assertEquals( 2, blocks.get( 0 ).getSingleValueInt() ); + assertEquals( 3, blocks.get( 1 ).getKeyIndexId() ); + assertEquals( 4, blocks.get( 1 ).getSingleValueInt() ); + } + + @Test + public void addLoadedBlockFailsWhenTooManyBlocksAdded() + { + PropertyRecord record = new PropertyRecord( 42 ); + + addBlock( record, 1, 2 ); + addBlock( record, 3, 4 ); + addBlock( record, 5, 6 ); + addBlock( record, 7, 8 ); + + try + { + addBlock( record, 9, 10 ); + fail( "Exception expected " ); + } + catch ( Throwable ignored ) + { + } } private void assertIteratorRemoveThrowsIllegalState( Iterator iterator ) @@ -133,4 +169,14 @@ private void assertIteratorRemoveThrowsIllegalState( Iterator ite { // OK } } + + private static void addBlock( PropertyRecord record, int key, int value ) + { + PropertyBlock block = new PropertyBlock(); + PropertyStore.encodeValue( block, key, value, null, null ); + for ( long valueBlock : block.getValueBlocks() ) + { + record.addLoadedBlock( valueBlock ); + } + } } diff --git a/community/kernel/src/test/java/org/neo4j/test/PageCacheRule.java b/community/kernel/src/test/java/org/neo4j/test/PageCacheRule.java index a37d5d5a5e807..d0fb872a5c24a 100644 --- a/community/kernel/src/test/java/org/neo4j/test/PageCacheRule.java +++ b/community/kernel/src/test/java/org/neo4j/test/PageCacheRule.java @@ -30,6 +30,7 @@ import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.pagecache.PageCache; +import org.neo4j.io.pagecache.tracing.PageCacheTracer; import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.impl.pagecache.StandalonePageCacheFactory; @@ -56,6 +57,11 @@ public PageCache getPageCache( FileSystemAbstraction fs ) } public PageCache getPageCache( FileSystemAbstraction fs, Config config ) + { + return getPageCache( fs, PageCacheTracer.NULL, config ); + } + + public PageCache getPageCache( FileSystemAbstraction fs, PageCacheTracer tracer, Config config ) { if ( pageCache != null ) { @@ -70,7 +76,7 @@ public PageCache getPageCache( FileSystemAbstraction fs, Config config ) } } - pageCache = StandalonePageCacheFactory.createPageCache( fs, config ); + pageCache = StandalonePageCacheFactory.createPageCache( fs, tracer, config ); if ( automaticallyProduceInconsistentReads ) {