Make sure that the page cache warmer does not artificially extend the…

… life of file mappings.

Much of the database expects to be able to close, unmap, and remap files as it pleases. Much of the database code assumes that it has complete control over their files.
The page cache warmer messed with this by grabbing references to the paged files, and thus increasing their reference count, and potentially extending their life beyond what other code intended for those mappings.
This has been fixed by making the `PageCache.listExistingMappings` method no longer incrimenting the paged file reference count, and the page cache warmer now instead anticipates and copes with asynchronously closed paged files.

community/io/src/main/java/org/neo4j/io/pagecache/PageCache.java

@@ -87,10 +87,12 @@ public interface PageCache extends AutoCloseable
     /**
      * List a snapshot of the current file mappings.
      * <p>
-     * The mappings can change as soon as this method returns. However, the returned {@link PagedFile}s will remain
-     * valid even if they are closed elsewhere.
+     * The mappings can change as soon as this method returns.
      * <p>
-     * <strong>NOTE:</strong> The calling code is responsible for closing <em>all</em> the returned paged files.
+     * <strong>NOTE:</strong> The calling code should <em>not</em> close the returned paged files, unless it does so
+     * in collaboration with the code that originally mapped the file. Any reference count in the mapping will
+     * <em>not</em> be incremented by this method, so calling code must be prepared for that the returned
+     * {@link PagedFile}s can be asynchronously closed elsewhere.
      *
      * @throws IOException if page cache has been closed or page eviction problems occur.
      */

community/io/src/main/java/org/neo4j/io/pagecache/impl/FileIsNotMappedException.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2002-2018 "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 <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.io.pagecache.impl;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * Thrown when accessing a {@link org.neo4j.io.pagecache.PageCursor} of a {@link org.neo4j.io.pagecache.PagedFile} that
+ * has been closed.
+ */
+public class FileIsNotMappedException extends IOException
+{
+    public FileIsNotMappedException( File file )
+    {
+        super( "File is not mapped: " + file );
+    }
+}

community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCache.java

@@ -460,8 +460,9 @@ public synchronized List<PagedFile> listExistingMappings() throws IOException
 
         while ( current != null )
         {
+            // Note that we are NOT incrementing the reference count here.
+            // Calling code is expected to be able to deal with asynchronously closed PagedFiles.
             MuninnPagedFile pagedFile = current.pagedFile;
-            pagedFile.incrementRefCount();
             list.add( pagedFile );
             current = current.next;
         }

community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCursor.java

@@ -28,6 +28,7 @@
 import org.neo4j.io.pagecache.PageCursor;
 import org.neo4j.io.pagecache.PageSwapper;
 import org.neo4j.io.pagecache.PagedFile;
+import org.neo4j.io.pagecache.impl.FileIsNotMappedException;
 import org.neo4j.io.pagecache.tracing.PageFaultEvent;
 import org.neo4j.io.pagecache.tracing.PinEvent;
 import org.neo4j.io.pagecache.tracing.cursor.PageCursorTracer;
@@ -426,7 +427,7 @@ private void abortPageFault( Throwable throwable, int[] chunk, long chunkOffset,
         pinEvent.done();
     }
 
-    long assertPagedFileStillMappedAndGetIdOfLastPage()
+    long assertPagedFileStillMappedAndGetIdOfLastPage() throws FileIsNotMappedException
     {
         return pagedFile.getLastPageId();
     }
@@ -435,7 +436,8 @@ long assertPagedFileStillMappedAndGetIdOfLastPage()
 
     protected abstract void convertPageFaultLock( long pageRef );
 
-    protected abstract void pinCursorToPage( long pageRef, long filePageId, PageSwapper swapper );
+    protected abstract void pinCursorToPage( long pageRef, long filePageId, PageSwapper swapper )
+            throws FileIsNotMappedException;
 
     protected abstract boolean tryLockPage( long pageRef );
 

community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPagedFile.java

@@ -32,6 +32,7 @@
 import org.neo4j.io.pagecache.PageSwapper;
 import org.neo4j.io.pagecache.PageSwapperFactory;
 import org.neo4j.io.pagecache.PagedFile;
+import org.neo4j.io.pagecache.impl.FileIsNotMappedException;
 import org.neo4j.io.pagecache.impl.PagedReadableByteChannel;
 import org.neo4j.io.pagecache.impl.PagedWritableByteChannel;
 import org.neo4j.io.pagecache.tracing.FlushEvent;
@@ -204,7 +205,7 @@ public int pageSize()
     }
 
     @Override
-    public long fileSize()
+    public long fileSize() throws FileIsNotMappedException
     {
         final long lastPageId = getLastPageId();
         if ( lastPageId < 0 )
@@ -519,13 +520,12 @@ public void flush() throws IOException
     }
 
     @Override
-    public long getLastPageId()
+    public long getLastPageId() throws FileIsNotMappedException
     {
         long state = getHeaderState();
         if ( refCountOf( state ) == 0 )
         {
-            String msg = "File has been unmapped: " + file().getPath();
-            IllegalStateException exception = new IllegalStateException( msg );
+            FileIsNotMappedException exception = new FileIsNotMappedException( file() );
             Exception closedBy = closeStackTrace;
             if ( closedBy != null )
             {

community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnWritePageCursor.java

@@ -22,6 +22,7 @@
 import java.io.IOException;
 
 import org.neo4j.io.pagecache.PageSwapper;
+import org.neo4j.io.pagecache.impl.FileIsNotMappedException;
 import org.neo4j.io.pagecache.tracing.cursor.PageCursorTracer;
 import org.neo4j.io.pagecache.tracing.cursor.context.VersionContextSupplier;
 
@@ -113,7 +114,7 @@ protected void unlockPage( long pageRef )
     }
 
     @Override
-    protected void pinCursorToPage( long pageRef, long filePageId, PageSwapper swapper )
+    protected void pinCursorToPage( long pageRef, long filePageId, PageSwapper swapper ) throws FileIsNotMappedException
     {
         reset( pageRef );
         // Check if we've been racing with unmapping. We want to do this before

community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java

@@ -62,6 +62,7 @@
 import org.neo4j.io.fs.FileSystemAbstraction;
 import org.neo4j.io.fs.OpenMode;
 import org.neo4j.io.fs.StoreChannel;
+import org.neo4j.io.pagecache.impl.FileIsNotMappedException;
 import org.neo4j.io.pagecache.impl.SingleFilePageSwapperFactory;
 import org.neo4j.io.pagecache.randomharness.Record;
 import org.neo4j.io.pagecache.randomharness.StandardRecordFormat;
@@ -1299,11 +1300,26 @@ public void mustListExistingMappings() throws Exception
             List<PagedFile> existingMappings = pageCache.listExistingMappings();
             assertThat( existingMappings.size(), is( 2 ) );
             assertThat( existingMappings, containsInAnyOrder( pf1, pf2 ) );
-            for ( PagedFile existingMapping : existingMappings )
+        }
+    }
+
+    @Test
+    public void listExistingMappingsMustNotIncrementPagedFileReferenceCount() throws Exception
+    {
+        configureStandardPageCache();
+        File file = file( "a" );
+        PagedFile existingMapping;
+        try ( PagedFile pf = pageCache.map( file, filePageSize ) )
+        {
+            existingMapping = pageCache.listExistingMappings().get( 0 );
+            try ( PageCursor cursor = existingMapping.io( 0, PF_SHARED_WRITE_LOCK ) )
             {
-                existingMapping.close();
+                assertTrue( cursor.next() );
             }
         }
+        // Now the mapping should be closed, which is signalled as an illegal state.
+        expectedException.expect( FileIsNotMappedException.class );
+        existingMapping.io( 0, PF_SHARED_WRITE_LOCK ).next();
     }
 
     @Test
@@ -2482,7 +2498,7 @@ public void lastPageIdFromUnmappedFileMustThrow() throws IOException
             file = pf;
         }
 
-        expectedException.expect( IllegalStateException.class );
+        expectedException.expect( FileIsNotMappedException.class );
         file.getLastPageId();
     }
 
@@ -2592,7 +2608,7 @@ public void pagedFileIoMustThrowIfFileIsUnmapped() throws IOException
                 cursor.next();
                 fail( "cursor.next() on unmapped file did not throw" );
             }
-            catch ( IllegalStateException e )
+            catch ( FileIsNotMappedException e )
             {
                 StringWriter out = new StringWriter();
                 e.printStackTrace( new PrintWriter( out ) );
@@ -2620,7 +2636,7 @@ public void writeLockedPageCursorNextMustThrowIfFileIsUnmapped() throws IOExcept
             cursor.next();
             fail( "cursor.next() on unmapped file did not throw" );
         }
-        catch ( IllegalStateException e )
+        catch ( FileIsNotMappedException e )
         {
             StringWriter out = new StringWriter();
             e.printStackTrace( new PrintWriter( out ) );
@@ -2637,7 +2653,7 @@ public void writeLockedPageCursorNextWithIdMustThrowIfFileIsUnmapped() throws IO
         PageCursor cursor = pagedFile.io( 0, PF_SHARED_WRITE_LOCK );
         pagedFile.close();
 
-        expectedException.expect( IllegalStateException.class );
+        expectedException.expect( FileIsNotMappedException.class );
         cursor.next( 1 );
     }
 
@@ -2652,7 +2668,7 @@ public void readLockedPageCursorNextMustThrowIfFileIsUnmapped() throws IOExcepti
         PageCursor cursor = pagedFile.io( 0, PF_SHARED_READ_LOCK );
         pagedFile.close();
 
-        expectedException.expect( IllegalStateException.class );
+        expectedException.expect( FileIsNotMappedException.class );
         cursor.next();
     }
 
@@ -2667,7 +2683,7 @@ public void readLockedPageCursorNextWithIdMustThrowIfFileIsUnmapped() throws IOE
         PageCursor cursor = pagedFile.io( 0, PF_SHARED_READ_LOCK );
         pagedFile.close();
 
-        expectedException.expect( IllegalStateException.class );
+        expectedException.expect( FileIsNotMappedException.class );
         cursor.next( 1 );
     }
 
@@ -2716,7 +2732,7 @@ public void advancingPessimisticReadLockingCursorAfterUnmappingMustThrow() throw
 
         fork( $close( pagedFile ) ).join();
 
-        expectedException.expect( IllegalStateException.class );
+        expectedException.expect( FileIsNotMappedException.class );
         cursor.next();
     }
 
@@ -2740,7 +2756,7 @@ public void advancingOptimisticReadLockingCursorAfterUnmappingMustThrow() throws
             cursor.next();
             fail( "Advancing the cursor should have thrown" );
         }
-        catch ( IllegalStateException e )
+        catch ( FileIsNotMappedException e )
         {
             // Yay!
         }
@@ -2770,7 +2786,7 @@ public void readingAndRetryingOnPageWithOptimisticReadLockingAfterUnmappingMustN
             cursor.next();
             fail( "Advancing the cursor should have thrown" );
         }
-        catch ( IllegalStateException e )
+        catch ( FileIsNotMappedException e )
         {
             // Yay!
         }

enterprise/causal-clustering/src/test/java/org/neo4j/io/pagecache/impl/muninn/VersionContextTrackingIT.java

@@ -36,6 +36,7 @@
 import org.neo4j.io.pagecache.PageCache;
 import org.neo4j.io.pagecache.PageSwapper;
 import org.neo4j.io.pagecache.PagedFile;
+import org.neo4j.io.pagecache.impl.FileIsNotMappedException;
 import org.neo4j.io.pagecache.tracing.cursor.PageCursorTracer;
 import org.neo4j.io.pagecache.tracing.cursor.context.EmptyVersionContextSupplier;
 import org.neo4j.kernel.impl.factory.GraphDatabaseFacade;
@@ -188,6 +189,7 @@ protected void convertPageFaultLock( long pageRef )
 
         @Override
         protected void pinCursorToPage( long pageRef, long filePageId, PageSwapper swapper )
+                throws FileIsNotMappedException
         {
             delegate.pinCursorToPage( pageRef, filePageId, swapper );
         }

enterprise/kernel/src/main/java/org/neo4j/kernel/impl/pagecache/PageCacheWarmer.java

@@ -44,6 +44,7 @@
 import org.neo4j.io.pagecache.PageCache;
 import org.neo4j.io.pagecache.PageCursor;
 import org.neo4j.io.pagecache.PagedFile;
+import org.neo4j.io.pagecache.impl.FileIsNotMappedException;
 import org.neo4j.kernel.impl.transaction.state.NeoStoreFileListing;
 import org.neo4j.scheduler.JobScheduler;
 import org.neo4j.storageengine.api.StoreFileMetadata;
@@ -106,21 +107,14 @@ public synchronized Resource addFilesTo( Collection<StoreFileMetadata> coll ) th
             return Resource.EMPTY;
         }
         List<PagedFile> files = pageCache.listExistingMappings();
-        try
+        for ( PagedFile file : files )
         {
-            for ( PagedFile file : files )
+            File profileFile = profileOutputFileFinal( file );
+            if ( fs.fileExists( profileFile ) )
             {
-                File profileFile = profileOutputFileFinal( file );
-                if ( fs.fileExists( profileFile ) )
-                {
-                    coll.add( new StoreFileMetadata( profileFile, 1, false ) );
-                }
+                coll.add( new StoreFileMetadata( profileFile, 1, false ) );
             }
         }
-        finally
-        {
-            IOUtils.closeAll( files );
-        }
         return Resource.EMPTY;
     }
 
@@ -147,16 +141,16 @@ public synchronized OptionalLong reheat() throws IOException
     {
         long pagesLoaded = 0;
         List<PagedFile> files = pageCache.listExistingMappings();
-        try
+        for ( PagedFile file : files )
         {
-            for ( PagedFile file : files )
+            try
             {
                 pagesLoaded += reheat( file );
             }
-        }
-        finally
-        {
-            IOUtils.closeAll( files );
+            catch ( FileIsNotMappedException ignore )
+            {
+                // The database is allowed to map and unmap files while we are trying to heat it up.
+            }
         }
         return stopped ? OptionalLong.empty() : OptionalLong.of( pagesLoaded );
     }
@@ -212,16 +206,16 @@ public synchronized OptionalLong profile() throws IOException
         // profiling in parallel is just not worth it.
         long pagesInMemory = 0;
         List<PagedFile> files = pageCache.listExistingMappings();
-        try
+        for ( PagedFile file : files )
         {
-            for ( PagedFile file : files )
+            try
             {
                 pagesInMemory += profile( file );
             }
-        }
-        finally
-        {
-            IOUtils.closeAll( files );
+            catch ( FileIsNotMappedException ignore )
+            {
+                // The database is allowed to map and unmap files while we are profiling the page cache.
+            }
         }
         pageCache.reportEvents();
         return stopped ? OptionalLong.empty() : OptionalLong.of( pagesInMemory );