Skip to content

Commit

Permalink
Appends tree information on exceptions leaving surface
Browse files Browse the repository at this point in the history 
There are many places where exceptions can be thrown from inside internal
code related to GBPTree and its internals. However the surface of GBPTree
is very small and so the approach is to not pass in tree information into
all internal classes and static methods, but instead decorate exceptions
from the small number of places where these exceptions can leave the
surface. This approach is much less intrusive than the alternative.

This is how it was already, but not the entire surface was covered.
Now also SeekCursor decorates outgoing exceptions.
  • Loading branch information
@tinwelint
tinwelint committed Jul 31, 2017
1 parent c8e9c2c commit 64daf97
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 59 deletions.
90 changes: 53 additions & 37 deletions community/index/src/main/java/org/neo4j/index/internal/gbptree/GBPTree.java
View file
Expand Up @@ -46,6 +46,7 @@
import static org.neo4j.index.internal.gbptree.Generation.generation;
import static org.neo4j.index.internal.gbptree.Generation.stableGeneration;
import static org.neo4j.index.internal.gbptree.Generation.unstableGeneration;
import static org.neo4j.index.internal.gbptree.GenerationSafePointer.MIN_GENERATION;
import static org.neo4j.index.internal.gbptree.Header.CARRY_OVER_PREVIOUS_HEADER;
import static org.neo4j.index.internal.gbptree.Header.replace;
import static org.neo4j.index.internal.gbptree.PageCursorUtil.checkOutOfBounds;
Expand Down Expand Up @@ -325,7 +326,7 @@ public void startupState( boolean clean )
* Whether or not this tree has been closed. Accessed and changed solely in
* {@link #close()} to be able to close tree multiple times gracefully.
*/
private boolean closed;
private boolean closed = true;

/**
* True if tree is clean, false if dirty
Expand All @@ -337,6 +338,12 @@ public void startupState( boolean clean )
*/
private final CleanupJob cleaning;

/**
* {@link Consumer} to hand out to others who want to decorate information about this tree
* to exceptions thrown out from its surface.
*/
private final Consumer<Throwable> exceptionDecorator = t -> appendTreeInformation( t );

/**
* Opens an index {@code indexFile} in the {@code pageCache}, creating and initializing it if it doesn't exist.
* If the index doesn't exist it will be created and the {@link Layout} and {@code pageSize} will
Expand Down Expand Up @@ -376,17 +383,19 @@ public GBPTree( PageCache pageCache, File indexFile, Layout<KEY,VALUE> layout, i
{
this.indexFile = indexFile;
this.monitor = monitor;
this.generation = Generation.generation( GenerationSafePointer.MIN_GENERATION, GenerationSafePointer.MIN_GENERATION + 1 );
this.generation = Generation.generation( MIN_GENERATION, MIN_GENERATION + 1 );
long rootId = IdSpace.MIN_TREE_NODE_ID;
setRoot( rootId, Generation.unstableGeneration( generation ) );
this.layout = layout;
this.pagedFile = openOrCreate( pageCache, indexFile, tentativePageSize, layout );
this.bTreeNode = new TreeNode<>( pageSize, layout );
this.freeList = new FreeListIdProvider( pagedFile, pageSize, rootId, FreeListIdProvider.NO_MONITOR );
this.writer = new SingleWriter( new InternalTreeLogic<>( freeList, bTreeNode, layout ) );

try
{
this.pagedFile = openOrCreate( pageCache, indexFile, tentativePageSize, layout );
closed = false;
this.bTreeNode = new TreeNode<>( pageSize, layout );
this.freeList = new FreeListIdProvider( pagedFile, pageSize, rootId, FreeListIdProvider.NO_MONITOR );
this.writer = new SingleWriter( new InternalTreeLogic<>( freeList, bTreeNode, layout ) );

// Create or load state
if ( created )
{
Expand All @@ -408,6 +417,7 @@ public GBPTree( PageCache pageCache, File indexFile, Layout<KEY,VALUE> layout, i
}
catch ( Throwable t )
{
appendTreeInformation( t );
try
{
close();
Expand Down Expand Up @@ -457,7 +467,7 @@ private PagedFile openOrCreate( PageCache pageCache, File indexFile,

try
{
readMeta( indexFile, layout, pagedFile );
readMeta( layout, pagedFile );
pagedFile = mapWithCorrectPageSize( pageCache, indexFile, pagedFile );
return pagedFile;
}
Expand All @@ -481,10 +491,10 @@ private PagedFile openOrCreate( PageCache pageCache, File indexFile,
pageSize = pageSizeForCreation == 0 ? pageCache.pageSize() : pageSizeForCreation;
if ( pageSize > pageCache.pageSize() )
{
throw new MetadataMismatchException( "Tree in " + indexFile.getAbsolutePath() +
" was about to be created with page size:" + pageSize +
", but page cache used to create it has a smaller page size:" +
pageCache.pageSize() + " so cannot be created" );
throw new MetadataMismatchException(
"Was about to create tree with page size %d" +
", but page cache used to create it has a smaller page size %d" +
" so cannot be created", pageSize, pageCache.pageSize() );
}

// We need to create this index
Expand Down Expand Up @@ -613,8 +623,7 @@ private static PageCursor openMetaPageCursor( PagedFile pagedFile, int pfFlags )
return metaCursor;
}

private void readMeta( File indexFile, Layout<KEY,VALUE> layout, PagedFile pagedFile )
throws IOException
private void readMeta( Layout<KEY,VALUE> layout, PagedFile pagedFile ) throws IOException
{
// Read meta
int formatVersion;
Expand All @@ -638,28 +647,31 @@ private void readMeta( File indexFile, Layout<KEY,VALUE> layout, PagedFile paged
}
catch ( CursorException e )
{
throw new MetadataMismatchException( format(
"Tried to open %s, but caught an error while reading meta data. " +
"File is expected to be corrupt, try to rebuild.", indexFile ), e );
throw new MetadataMismatchException( e,
"Tried to open, but caught an error while reading meta data. " +
"File is expected to be corrupt, try to rebuild." );
}

if ( formatVersion != FORMAT_VERSION )
{
throw new MetadataMismatchException( "Tried to open %s with a different format version than " +
"what it was created with. Created with:%d, opened with %d",
indexFile, formatVersion, FORMAT_VERSION );
throw new MetadataMismatchException(
"Tried to open with a different format version than " +
"what it was created with. Created with %d, opened with %d",
formatVersion, FORMAT_VERSION );
}
if ( layoutIdentifier != layout.identifier() )
{
throw new MetadataMismatchException( "Tried to open " + indexFile + " using different layout identifier " +
"than what it was created with. Created with:" + layoutIdentifier + ", opened with " +
layout.identifier() );
throw new MetadataMismatchException(
"Tried to open using different layout identifier " +
"than what it was created with. Created with %d, opened with %d",
layoutIdentifier, layout.identifier() );
}
if ( majorVersion != layout.majorVersion() || minorVersion != layout.minorVersion() )
{
throw new MetadataMismatchException( "Tried to open " + indexFile + " using different layout version " +
"than what it was created with. Created with:" + majorVersion + "." + minorVersion +
", opened with " + layout.majorVersion() + "." + layout.minorVersion() );
throw new MetadataMismatchException(
"Tried to open using different layout version " +
"than what it was created with. Created with %d.%d, opened with %d.%d",
majorVersion, minorVersion, layout.majorVersion(), layout.minorVersion() );
}
}

Expand All @@ -685,10 +697,10 @@ private PagedFile mapWithCorrectPageSize( PageCache pageCache, File indexFile, P
{
if ( pageSize > pageCache.pageSize() )
{
throw new MetadataMismatchException( "Tree in " + indexFile.getAbsolutePath() +
" was created with page size:" + pageSize +
", but page cache used to open it this time has a smaller page size:" +
pageCache.pageSize() + " so cannot be opened" );
throw new MetadataMismatchException(
" was created with page size %d, but page cache used to open it this time " +
"has a smaller page size %d so cannot be opened",
pageSize, pageCache.pageSize() );
}
pagedFile.close();
return pageCache.map( indexFile, pageSize );
Expand Down Expand Up @@ -743,7 +755,8 @@ public RawCursor<Hit<KEY,VALUE>,IOException> seek( KEY fromInclusive, KEY toExcl

// Returns cursor which is now initiated with left-most leaf node for the specified range
return new SeekCursor<>( cursor, bTreeNode, fromInclusive, toExclusive, layout,
stableGeneration, unstableGeneration, generationSupplier, rootCatchup, rootGeneration );
stableGeneration, unstableGeneration, generationSupplier, rootCatchup, rootGeneration,
exceptionDecorator );
}

/**
Expand Down Expand Up @@ -1010,7 +1023,7 @@ public String toString()
stableGeneration( generation ), unstableGeneration( generation ) );
}

private TreeInconsistencyException appendTreeInformation( TreeInconsistencyException e )
private <E extends Throwable> E appendTreeInformation( E e )
{
return Exceptions.withMessage( e, e.getMessage() + " | " + toString() );
}
Expand Down Expand Up @@ -1075,9 +1088,10 @@ void initialize() throws IOException
treeLogic.initialize( cursor );
success = true;
}
catch ( TreeInconsistencyException e )
catch ( Throwable e )
{
throw appendTreeInformation( e );
appendTreeInformation( e );
throw e;
}
finally
{
Expand All @@ -1102,9 +1116,10 @@ public void merge( KEY key, VALUE value, ValueMerger<KEY,VALUE> valueMerger ) th
treeLogic.insert( cursor, structurePropagation, key, value, valueMerger,
stableGeneration, unstableGeneration );
}
catch ( TreeInconsistencyException e )
catch ( Throwable e )
{
throw appendTreeInformation( e );
appendTreeInformation( e );
throw e;
}

if ( structurePropagation.hasRightKeyInsert )
Expand Down Expand Up @@ -1147,9 +1162,10 @@ public VALUE remove( KEY key ) throws IOException
result = treeLogic.remove( cursor, structurePropagation, key, layout.newValue(),
stableGeneration, unstableGeneration );
}
catch ( TreeInconsistencyException e )
catch ( Throwable e )
{
throw appendTreeInformation( e );
appendTreeInformation( e );
throw e;
}

if ( structurePropagation.hasMidChildUpdate )
Expand Down
8 changes: 4 additions & 4 deletions ...unity/index/src/main/java/org/neo4j/index/internal/gbptree/MetadataMismatchException.java
View file
Expand Up @@ -24,13 +24,13 @@
*/
public class MetadataMismatchException extends RuntimeException
{
MetadataMismatchException( String format, Object... args )
MetadataMismatchException( Throwable cause, String format, Object... args )
{
super( String.format( format, args ) );
super( String.format( format, args ), cause );
}

MetadataMismatchException( String message )
MetadataMismatchException( String format, Object... args )
{
super( message );
super( String.format( format, args ) );
}
}
33 changes: 31 additions & 2 deletions community/index/src/main/java/org/neo4j/index/internal/gbptree/SeekCursor.java
View file
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.index.internal.gbptree;

import java.io.IOException;
import java.util.function.Consumer;
import java.util.function.LongSupplier;
import java.util.function.Supplier;

Expand Down Expand Up @@ -362,14 +363,21 @@ class SeekCursor<KEY,VALUE> implements RawCursor<Hit<KEY,VALUE>,IOException>, Hi
*/
private boolean closed;

/**
* Decorator for caught exceptions, adding information about which tree the exception relates to.
*/
private final Consumer<Throwable> exceptionDecorator;

SeekCursor( PageCursor cursor, TreeNode<KEY,VALUE> bTreeNode, KEY fromInclusive, KEY toExclusive,
Layout<KEY,VALUE> layout, long stableGeneration, long unstableGeneration, LongSupplier generationSupplier,
Supplier<Root> rootCatchup, long lastFollowedPointerGeneration ) throws IOException
Supplier<Root> rootCatchup, long lastFollowedPointerGeneration, Consumer<Throwable> exceptionDecorator )
throws IOException
{
this.cursor = cursor;
this.fromInclusive = fromInclusive;
this.toExclusive = toExclusive;
this.layout = layout;
this.exceptionDecorator = exceptionDecorator;
this.exactMatch = layout.compare( fromInclusive, toExclusive ) == 0;
this.stableGeneration = stableGeneration;
this.unstableGeneration = unstableGeneration;
Expand All @@ -386,7 +394,15 @@ class SeekCursor<KEY,VALUE> implements RawCursor<Hit<KEY,VALUE>,IOException>, Hi
this.expectedFirstAfterGoToNext = layout.newKey();
this.firstKeyInNode = layout.newKey();

traverseDownToFirstLeaf();
try
{
traverseDownToFirstLeaf();
}
catch ( Throwable e )
{
exceptionDecorator.accept( e );
throw e;
}
}

/**
Expand Down Expand Up @@ -473,6 +489,19 @@ else if ( !saneRead() )

@Override
public boolean next() throws IOException
{
try
{
return internalNext();
}
catch ( Throwable e )
{
exceptionDecorator.accept( e );
throw e;
}
}

private boolean internalNext() throws IOException
{
while ( true )
{
Expand Down
5 changes: 0 additions & 5 deletions ...nity/index/src/main/java/org/neo4j/index/internal/gbptree/TreeInconsistencyException.java
View file
Expand Up @@ -28,9 +28,4 @@ public class TreeInconsistencyException extends RuntimeException
{
super( String.format( format, args ) );
}

TreeInconsistencyException( String message )
{
super( message );
}
}
9 changes: 5 additions & 4 deletions community/index/src/main/java/org/neo4j/index/internal/gbptree/TreeNode.java
View file
Expand Up @@ -104,13 +104,14 @@ class TreeNode<KEY,VALUE>

if ( internalMaxKeyCount < 2 )
{
throw new MetadataMismatchException( "For layout " + layout + " a page size of " + pageSize +
" would only fit " + internalMaxKeyCount + " internal keys, minimum is 2" );
throw new MetadataMismatchException(
"For layout %s a page size of %d would only fit %d internal keys, minimum is 2",
layout, pageSize, internalMaxKeyCount );
}
if ( leafMaxKeyCount < 2 )
{
throw new MetadataMismatchException( "A page size of " + pageSize + " would only fit " +
leafMaxKeyCount + " leaf keys, minimum is 2" );
throw new MetadataMismatchException( "A page size of %d would only fit leaf keys, minimum is 2",
pageSize, leafMaxKeyCount );
}
}

Expand Down
1 change: 0 additions & 1 deletion community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java
View file
Expand Up @@ -173,7 +173,6 @@ public long identifier()
};
try ( GBPTree<MutableLong,MutableLong> ignored = index().with( otherLayout ).build() )
{

fail( "Should not load" );
}
catch ( MetadataMismatchException e )
Expand Down

0 comments on commit 64daf97

Please sign in to comment.