From 8af5d029369a6341f6a73b323f7ab02806160d1f Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Fri, 3 Mar 2017 17:25:32 +0100 Subject: [PATCH] Fix ReadPageCursor shouldRetry bug with linked cursors When we checked the lock validity using the stamp from the top-most page cursor, instead of the stamp for the given cursor. This obviously often resulted in the check failing, with no way to get out of the shouldRetry look. The shouldRetry check now currectly uses the lock stamp for the given cursor in the chain, and also makes sure to refresh the lock state of all the cursors in the chain when a restart is necessary. --- .../impl/muninn/MuninnReadPageCursor.java | 19 +++++++++++++++---- .../org/neo4j/io/pagecache/PageCacheTest.java | 4 +++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnReadPageCursor.java b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnReadPageCursor.java index 7f7fab9a4ff4a..353aa4e072713 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnReadPageCursor.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnReadPageCursor.java @@ -97,21 +97,32 @@ protected void releaseCursor() @Override public boolean shouldRetry() throws IOException { - MuninnPageCursor cursor = this; + MuninnReadPageCursor cursor = this; do { long pageRef = cursor.pinnedPageRef; - if ( pageRef != 0 && !pagedFile.validateReadLock( pageRef, lockStamp ) ) + if ( pageRef != 0 && !pagedFile.validateReadLock( pageRef, cursor.lockStamp ) ) { - startRetry(); + startRetryLinkedChain(); return true; } - cursor = cursor.linkedCursor; + cursor = (MuninnReadPageCursor) cursor.linkedCursor; } while ( cursor != null ); return false; } + private void startRetryLinkedChain() throws IOException + { + MuninnReadPageCursor cursor = this; + do + { + cursor.startRetry(); + cursor = (MuninnReadPageCursor) cursor.linkedCursor; + } + while ( cursor != null ); + } + private void startRetry() throws IOException { setOffset( 0 ); diff --git a/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java b/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java index 5f2977ed877ba..c8b954f8f8766 100644 --- a/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java +++ b/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java @@ -4455,7 +4455,7 @@ public void openingLinkedCursorMustCloseExistingLinkedCursor() throws Exception } } - @Test//( timeout = SHORT_TIMEOUT_MILLIS ) + @Test( timeout = SHORT_TIMEOUT_MILLIS ) public void shouldRetryOnParentCursorMustReturnTrueIfLinkedCursorNeedsRetry() throws Exception { generateFileWithRecords( file( "a" ), recordsPerFilePage * 2, recordSize ); @@ -4472,6 +4472,8 @@ public void shouldRetryOnParentCursorMustReturnTrueIfLinkedCursorNeedsRetry() th // parentReader shouldRetry should be true because the linked cursor needs retry assertTrue( parentReader.shouldRetry() ); + // then, the next read should be consistent + assertFalse( parentReader.shouldRetry() ); } }