Skip to content

Commit

Permalink
Removes optimization to not checkpoint GBPTree on no changes
Browse files Browse the repository at this point in the history
This solves an issue which would be observed as a tree looking like non-clean
even after a clean db shutdown. The main reason for this is that skipping
the checkpoint would not await the background clean job and so closing the tree
could potentially be done before the clean job was done. This would prevent
the tree from being marked as clean on close.

Read-only tools like consistency checker asserts that no recovery is required
before starting and so would fail on a db containing such a tree.
  • Loading branch information
tinwelint committed Sep 25, 2017
1 parent 581d8ca commit 25d75e7
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 12 deletions.
Expand Up @@ -366,6 +366,36 @@ public void startupState( boolean clean )
* } * }
* </pre> * </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 pageCache {@link PageCache} to use to map index file
* @param indexFile {@link File} containing the actual index * @param indexFile {@link File} containing the actual index
* @param layout {@link Layout} to use in the tree, this must match the existing layout * @param layout {@link Layout} to use in the tree, this must match the existing layout
Expand Down Expand Up @@ -456,6 +486,8 @@ private void initializeAfterCreation( Layout<KEY,VALUE> layout, Consumer<PageCur
// Initialize free-list // Initialize free-list
freeList.initializeAfterCreation(); freeList.initializeAfterCreation();
changesSinceLastCheckpoint = true; changesSinceLastCheckpoint = true;

// Checkpoint to make the created root node stable. Forcing tree state also piggy-backs on this.
checkpoint( IOLimiter.unlimited(), headerWriter ); checkpoint( IOLimiter.unlimited(), headerWriter );
clean = true; clean = true;
} }
Expand Down Expand Up @@ -828,12 +860,6 @@ public void checkpoint( IOLimiter ioLimiter ) throws IOException


private void checkpoint( IOLimiter ioLimiter, Header.Writer headerWriter ) 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 // Flush dirty pages of the tree, do this before acquiring the lock so that writers won't be
// blocked while we do this // blocked while we do this
pagedFile.flushAndForce( ioLimiter ); pagedFile.flushAndForce( ioLimiter );
Expand Down
Expand Up @@ -21,19 +21,34 @@


import org.apache.commons.lang3.tuple.Pair; import org.apache.commons.lang3.tuple.Pair;


import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.io.PrintStream; 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.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 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.ConsistencyChecker.assertOnTreeNode;
import static org.neo4j.index.internal.gbptree.GenerationSafePointerPair.pointer; 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. * 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 TreeNode<KEY,VALUE> node;
private final Layout<KEY,VALUE> layout; private final Layout<KEY,VALUE> layout;
Expand All @@ -48,6 +63,52 @@ class TreePrinter<KEY,VALUE>
this.unstableGeneration = unstableGeneration; 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. * 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. * Let the passed in {@code cursor} point to the root or sub-tree (internal node) of what to print.
Expand All @@ -65,11 +126,8 @@ void printTree( PageCursor cursor, PrintStream out, boolean printValues, boolean
if ( printState ) if ( printState )
{ {
long currentPage = cursor.getCurrentPageId(); long currentPage = cursor.getCurrentPageId();
Pair<TreeState,TreeState> statePair = TreeStatePair.readStatePages( printTreeState( cursor, out );
cursor, IdSpace.STATE_PAGE_A, IdSpace.STATE_PAGE_B );
TreeNode.goTo( cursor, "back to tree node from reading state", currentPage ); TreeNode.goTo( cursor, "back to tree node from reading state", currentPage );
out.println( "StateA: " + statePair.getLeft() );
out.println( "StateB: " + statePair.getRight() );
} }
assertOnTreeNode( cursor ); assertOnTreeNode( cursor );


Expand Down
Expand Up @@ -1209,7 +1209,7 @@ public void shouldCheckpointAfterInitialCreation() throws Exception
} }


@Test @Test
public void shouldNotCheckpointOnCloseIfNoChangesHappened() throws Exception public void shouldNotCheckpointOnClose() throws Exception
{ {
// GIVEN // GIVEN
CheckpointCounter checkpointCounter = new CheckpointCounter(); CheckpointCounter checkpointCounter = new CheckpointCounter();
Expand All @@ -1230,6 +1230,27 @@ public void shouldNotCheckpointOnCloseIfNoChangesHappened() throws Exception
assertEquals( 1, checkpointCounter.count() ); 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();
for ( int i = 0; i < 2; i++ )
{

}
index.checkpoint( unlimited() );

// THEN
assertEquals( 1, checkpointCounter.count() );
}
}

@Test @Test
public void mustNotSeeUpdatesThatWasNotCheckpointed() throws Exception public void mustNotSeeUpdatesThatWasNotCheckpointed() throws Exception
{ {
Expand Down
52 changes: 52 additions & 0 deletions 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 );
}
}

0 comments on commit 25d75e7

Please sign in to comment.