Skip to content

Commit

Permalink
Fixing issue with shouldRetry-less reading of tree state pages
Browse files Browse the repository at this point in the history
reading tree state pages means reading two pages, where each is now read
with its own shouldRetry loop.
  • Loading branch information
tinwelint committed Jan 9, 2017
1 parent 8070d18 commit e697f48
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 17 deletions.
Expand Up @@ -401,13 +401,8 @@ private static Pair<TreeState,TreeState> readStatePages( PagedFile pagedFile ) t
Pair<TreeState,TreeState> states; Pair<TreeState,TreeState> states;
try ( PageCursor cursor = pagedFile.io( 0L /*ignored*/, PagedFile.PF_SHARED_READ_LOCK ) ) try ( PageCursor cursor = pagedFile.io( 0L /*ignored*/, PagedFile.PF_SHARED_READ_LOCK ) )
{ {
do states = TreeStatePair.readStatePages(
{ cursor, IdSpace.STATE_PAGE_A, IdSpace.STATE_PAGE_B );
states = TreeStatePair.readStatePages(
cursor, IdSpace.STATE_PAGE_A, IdSpace.STATE_PAGE_B );
}
while ( cursor.shouldRetry() );
checkOutOfBounds( cursor );
} }
return states; return states;
} }
Expand Down
Expand Up @@ -26,6 +26,8 @@


import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PageCursor;


import static org.neo4j.index.gbptree.PageCursorUtil.checkOutOfBounds;

/** /**
* Pair of {@link TreeState}, ability to make decision about which of the two to read and write respectively, * Pair of {@link TreeState}, ability to make decision about which of the two to read and write respectively,
* depending on the {@link TreeState#isValid() validity} and {@link TreeState#stableGeneration()} of each. * depending on the {@link TreeState#isValid() validity} and {@link TreeState#stableGeneration()} of each.
Expand All @@ -35,6 +37,7 @@ class TreeStatePair
/** /**
* Reads the tree state pair, one from each of {@code pageIdA} and {@code pageIdB}, deciding their validity * Reads the tree state pair, one from each of {@code pageIdA} and {@code pageIdB}, deciding their validity
* and returning them as a {@link Pair}. * and returning them as a {@link Pair}.
* do-shouldRetry is managed inside this method because data is read from two pages.
* *
* @param cursor {@link PageCursor} to use when reading. This cursor will be moved to the two pages * @param cursor {@link PageCursor} to use when reading. This cursor will be moved to the two pages
* one after the other, to read their states. * one after the other, to read their states.
Expand All @@ -45,19 +48,24 @@ class TreeStatePair
*/ */
static Pair<TreeState,TreeState> readStatePages( PageCursor cursor, long pageIdA, long pageIdB ) throws IOException static Pair<TreeState,TreeState> readStatePages( PageCursor cursor, long pageIdA, long pageIdB ) throws IOException
{ {
TreeState stateA; TreeState stateA = readStatePage( cursor, pageIdA );
TreeState stateB; TreeState stateB = readStatePage( cursor, pageIdB );

// Use write lock to avoid should retry. Fine because there can be no concurrency at this point anyway

PageCursorUtil.goTo( cursor, "state page A", pageIdA );
stateA = TreeState.read( cursor );

PageCursorUtil.goTo( cursor, "state page B", pageIdB );
stateB = TreeState.read( cursor );
return Pair.of( stateA, stateB ); return Pair.of( stateA, stateB );
} }


private static TreeState readStatePage( PageCursor cursor, long pageIdA ) throws IOException
{
PageCursorUtil.goTo( cursor, "state page", pageIdA );
TreeState state;
do
{
state = TreeState.read( cursor );
}
while ( cursor.shouldRetry() );
checkOutOfBounds( cursor );
return state;
}

/** /**
* @param states the two states to compare. * @param states the two states to compare.
* @return newest (w/ regards to {@link TreeState#stableGeneration()}) {@link TreeState#isValid() valid} * @return newest (w/ regards to {@link TreeState#stableGeneration()}) {@link TreeState#isValid() valid}
Expand Down

0 comments on commit e697f48

Please sign in to comment.