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.
neo4j · May 26, 2017 · 8af5d02 · 8af5d02
1 parent caa73ff
commit 8af5d02
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 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
@@ -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
@@ -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() );
         }
     }