Merge pull request #10120 from tinwelint/3.3-gbptree-always-checkpoint

Removes optimization to not checkpoint GBPTree on no changes

community/index/src/main/java/org/neo4j/index/internal/gbptree/GBPTree.java

@@ -366,6 +366,36 @@ public void startupState( boolean clean )
      *     }
      * </pre>
      *
+     * Expected state after first time tree is opened, where initial state is created:
+     * <ul>
+     * <li>StateA
+     * <ul>
+     * <li>stableGeneration=2</li>
+     * <li>unstableGeneration=3</li>
+     * <li>rootId=3</li>
+     * <li>rootGeneration=2</li>
+     * <li>lastId=4</li>
+     * <li>freeListWritePageId=4</li>
+     * <li>freeListReadPageId=4</li>
+     * <li>freeListWritePos=0</li>
+     * <li>freeListReadPos=0</li>
+     * <li>clean=false</li>
+     * </ul>
+     * <li>StateB
+     * <ul>
+     * <li>stableGeneration=2</li>
+     * <li>unstableGeneration=4</li>
+     * <li>rootId=3</li>
+     * <li>rootGeneration=2</li>
+     * <li>lastId=4</li>
+     * <li>freeListWritePageId=4</li>
+     * <li>freeListReadPageId=4</li>
+     * <li>freeListWritePos=0</li>
+     * <li>freeListReadPos=0</li>
+     * <li>clean=false</li>
+     * </ul>
+     * </ul>
+     *
      * @param pageCache {@link PageCache} to use to map index file
      * @param indexFile {@link File} containing the actual index
      * @param layout {@link Layout} to use in the tree, this must match the existing layout
@@ -456,6 +486,8 @@ private void initializeAfterCreation( Layout<KEY,VALUE> layout, Consumer<PageCur
         // Initialize free-list
         freeList.initializeAfterCreation();
         changesSinceLastCheckpoint = true;
+
+        // Checkpoint to make the created root node stable. Forcing tree state also piggy-backs on this.
         checkpoint( IOLimiter.unlimited(), headerWriter );
         clean = true;
     }
@@ -828,12 +860,6 @@ public void checkpoint( IOLimiter ioLimiter ) throws IOException
 
     private void checkpoint( IOLimiter ioLimiter, Header.Writer headerWriter ) throws IOException
     {
-        if ( !changesSinceLastCheckpoint && headerWriter == CARRY_OVER_PREVIOUS_HEADER )
-        {
-            // No changes has happened since last checkpoint was called, no need to do another checkpoint
-            return;
-        }
-
         // Flush dirty pages of the tree, do this before acquiring the lock so that writers won't be
         // blocked while we do this
         pagedFile.flushAndForce( ioLimiter );

community/index/src/main/java/org/neo4j/index/internal/gbptree/TreePrinter.java

@@ -21,19 +21,34 @@
 
 import org.apache.commons.lang3.tuple.Pair;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
+import java.nio.file.StandardOpenOption;
 
+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.impl.SingleFilePageSwapperFactory;
+import org.neo4j.io.pagecache.impl.muninn.MuninnPageCache;
+import org.neo4j.io.pagecache.tracing.cursor.PageCursorTracerSupplier;
 
 import static java.lang.String.format;
+
+import static org.neo4j.graphdb.config.Configuration.EMPTY;
 import static org.neo4j.index.internal.gbptree.ConsistencyChecker.assertOnTreeNode;
 import static org.neo4j.index.internal.gbptree.GenerationSafePointerPair.pointer;
+import static org.neo4j.io.ByteUnit.kibiBytes;
+import static org.neo4j.io.pagecache.tracing.PageCacheTracer.NULL;
 
 /**
  * Utility class for printing a {@link GBPTree}, either whole or sub-tree.
+ *
+ * @param <KEY> type of keys in the tree.
+ * @param <VALUE> type of values in the tree.
  */
-class TreePrinter<KEY,VALUE>
+public class TreePrinter<KEY,VALUE>
 {
     private final TreeNode<KEY,VALUE> node;
     private final Layout<KEY,VALUE> layout;
@@ -48,6 +63,52 @@ class TreePrinter<KEY,VALUE>
         this.unstableGeneration = unstableGeneration;
     }
 
+    /**
+     * Prints the header, that is tree state and meta information, about the tree present in the given {@code file}.
+     *
+     * @param fs {@link FileSystemAbstraction} where the {@code file} exists.
+     * @param file {@link File} containing the tree to print header for.
+     * @param out {@link PrintStream} to print at.
+     * @throws IOException on I/O error.
+     */
+    public static void printHeader( FileSystemAbstraction fs, File file, PrintStream out ) throws IOException
+    {
+        SingleFilePageSwapperFactory swapper = new SingleFilePageSwapperFactory();
+        swapper.open( fs, EMPTY );
+        try ( PageCache pageCache = new MuninnPageCache( swapper, 100, (int) kibiBytes( 8 ), NULL, PageCursorTracerSupplier.NULL ) )
+        {
+            printHeader( pageCache, file, out );
+        }
+    }
+
+    /**
+     * Prints the header, that is tree state and meta information, about the tree present in the given {@code file}.
+     *
+     * @param pageCache {@link PageCache} able to map tree contained in {@code file}.
+     * @param file {@link File} containing the tree to print header for.
+     * @param out {@link PrintStream} to print at.
+     * @throws IOException on I/O error.
+     */
+    public static void printHeader( PageCache pageCache, File file, PrintStream out ) throws IOException
+    {
+        try ( PagedFile pagedFile = pageCache.map( file, pageCache.pageSize(), StandardOpenOption.READ ) )
+        {
+            try ( PageCursor cursor = pagedFile.io( IdSpace.STATE_PAGE_A, PagedFile.PF_SHARED_READ_LOCK ) )
+            {
+                // TODO add printing of meta information here when that abstraction has been merged.
+                printTreeState( cursor, out );
+            }
+        }
+    }
+
+    private static void printTreeState( PageCursor cursor, PrintStream out ) throws IOException
+    {
+        Pair<TreeState,TreeState> statePair = TreeStatePair.readStatePages(
+                cursor, IdSpace.STATE_PAGE_A, IdSpace.STATE_PAGE_B );
+        out.println( "StateA: " + statePair.getLeft() );
+        out.println( "StateB: " + statePair.getRight() );
+    }
+
     /**
      * Prints a {@link GBPTree} in human readable form, very useful for debugging.
      * Let the passed in {@code cursor} point to the root or sub-tree (internal node) of what to print.
@@ -65,11 +126,8 @@ void printTree( PageCursor cursor, PrintStream out, boolean printValues, boolean
         if ( printState )
         {
             long currentPage = cursor.getCurrentPageId();
-            Pair<TreeState,TreeState> statePair = TreeStatePair.readStatePages(
-                    cursor, IdSpace.STATE_PAGE_A, IdSpace.STATE_PAGE_B );
+            printTreeState( cursor, out );
             TreeNode.goTo( cursor, "back to tree node from reading state", currentPage );
-            out.println( "StateA: " + statePair.getLeft() );
-            out.println( "StateB: " + statePair.getRight() );
         }
         assertOnTreeNode( cursor );
 

community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java

@@ -855,8 +855,7 @@ public void cleanJobShouldLockOutCheckpoint() throws Exception
         CleanJobControlledMonitor monitor = new CleanJobControlledMonitor();
         try ( GBPTree<MutableLong,MutableLong> index = index().with( monitor ).with( cleanupWork ).build() )
         {
-            // WHEN
-            // Cleanup not finished
+            // WHEN cleanup not finished
             Future<?> cleanup = executor.submit( throwing( cleanupWork::start ) );
             monitor.barrier.awaitUninterruptibly();
             index.writer().close();
@@ -871,6 +870,30 @@ public void cleanJobShouldLockOutCheckpoint() throws Exception
         }
     }
 
+    @Test( timeout = 5_000L )
+    public void cleanJobShouldLockOutCheckpointOnNoUpdate() throws Exception
+    {
+        // GIVEN
+        makeDirty();
+
+        RecoveryCleanupWorkCollector cleanupWork = new ControlledRecoveryCleanupWorkCollector();
+        CleanJobControlledMonitor monitor = new CleanJobControlledMonitor();
+        try ( GBPTree<MutableLong,MutableLong> index = index().with( monitor ).with( cleanupWork ).build() )
+        {
+            // WHEN cleanup not finished
+            Future<?> cleanup = executor.submit( throwing( cleanupWork::start ) );
+            monitor.barrier.awaitUninterruptibly();
+
+            // THEN
+            Future<?> checkpoint = executor.submit( throwing( () -> index.checkpoint( IOLimiter.unlimited() ) ) );
+            shouldWait( checkpoint );
+
+            monitor.barrier.release();
+            cleanup.get();
+            checkpoint.get();
+        }
+    }
+
     @Test( timeout = 5_000L )
     public void cleanJobShouldNotLockOutClose() throws Exception
     {
@@ -1209,7 +1232,7 @@ public void shouldCheckpointAfterInitialCreation() throws Exception
     }
 
     @Test
-    public void shouldNotCheckpointOnCloseIfNoChangesHappened() throws Exception
+    public void shouldNotCheckpointOnClose() throws Exception
     {
         // GIVEN
         CheckpointCounter checkpointCounter = new CheckpointCounter();
@@ -1230,6 +1253,23 @@ public void shouldNotCheckpointOnCloseIfNoChangesHappened() throws Exception
         assertEquals( 1, checkpointCounter.count() );
     }
 
+    @Test
+    public void shouldCheckpointEvenIfNoChanges() throws Exception
+    {
+        // GIVEN
+        CheckpointCounter checkpointCounter = new CheckpointCounter();
+
+        // WHEN
+        try ( GBPTree<MutableLong,MutableLong> index = index().with( checkpointCounter ).build() )
+        {
+            checkpointCounter.reset();
+            index.checkpoint( unlimited() );
+
+            // THEN
+            assertEquals( 1, checkpointCounter.count() );
+        }
+    }
+
     @Test
     public void mustNotSeeUpdatesThatWasNotCheckpointed() throws Exception
     {
@@ -1664,7 +1704,7 @@ private void insert( GBPTree<MutableLong,MutableLong> index, long key, long valu
         }
     }
 
-    private void shouldWait( Future<?> future )throws InterruptedException, ExecutionException
+    private void shouldWait( Future<?> future ) throws InterruptedException, ExecutionException
     {
         try
         {

tools/src/main/java/org/neo4j/tools/dump/DumpGBPTree.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2002-2017 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.tools.dump;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.neo4j.index.internal.gbptree.GBPTree;
+import org.neo4j.index.internal.gbptree.TreePrinter;
+import org.neo4j.io.fs.DefaultFileSystemAbstraction;
+
+/**
+ * For now only dumps header, could be made more useful over time.
+ */
+public class DumpGBPTree
+{
+    /**
+     * Dumps stuff about a {@link GBPTree} to console in human readable format.
+     *
+     * @param args arguments.
+     * @throws IOException on I/O error.
+     */
+    public static void main( String[] args ) throws IOException
+    {
+        if ( args.length == 0 )
+        {
+            System.err.println( "File argument expected" );
+            System.exit( 1 );
+        }
+
+        File file = new File( args[0] );
+        System.out.println( "Dumping " + file.getAbsolutePath() );
+        TreePrinter.printHeader( new DefaultFileSystemAbstraction(), file, System.out );
+    }
+}