From e697f4833f911f01a1fa40d790fc604caa3c84ca Mon Sep 17 00:00:00 2001
From: Mattias Persson <mattias@neotechnology.com>
Date: Tue, 20 Dec 2016 18:49:59 +0100
Subject: [PATCH] Fixing issue with shouldRetry-less reading of tree state
 pages

reading tree state pages means reading two pages, where each is now read
with its own shouldRetry loop.
---
 .../java/org/neo4j/index/gbptree/GBPTree.java |  9 ++----
 .../neo4j/index/gbptree/TreeStatePair.java    | 28 ++++++++++++-------
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/community/index/src/main/java/org/neo4j/index/gbptree/GBPTree.java b/community/index/src/main/java/org/neo4j/index/gbptree/GBPTree.java
index 039ba82835cb..6fe9eb8adcf5 100644
--- a/community/index/src/main/java/org/neo4j/index/gbptree/GBPTree.java
+++ b/community/index/src/main/java/org/neo4j/index/gbptree/GBPTree.java
@@ -401,13 +401,8 @@ private static Pair<TreeState,TreeState> readStatePages( PagedFile pagedFile ) t
         Pair<TreeState,TreeState> states;
         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 );
-            }
-            while ( cursor.shouldRetry() );
-            checkOutOfBounds( cursor );
+            states = TreeStatePair.readStatePages(
+                    cursor, IdSpace.STATE_PAGE_A, IdSpace.STATE_PAGE_B );
         }
         return states;
     }
diff --git a/community/index/src/main/java/org/neo4j/index/gbptree/TreeStatePair.java b/community/index/src/main/java/org/neo4j/index/gbptree/TreeStatePair.java
index 720c6dfea394..b003cfdbe04b 100644
--- a/community/index/src/main/java/org/neo4j/index/gbptree/TreeStatePair.java
+++ b/community/index/src/main/java/org/neo4j/index/gbptree/TreeStatePair.java
@@ -26,6 +26,8 @@
 
 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,
  * depending on the {@link TreeState#isValid() validity} and {@link TreeState#stableGeneration()} of each.
@@ -35,6 +37,7 @@ class TreeStatePair
     /**
      * Reads the tree state pair, one from each of {@code pageIdA} and {@code pageIdB}, deciding their validity
      * 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
      * one after the other, to read their states.
@@ -45,19 +48,24 @@ class TreeStatePair
      */
     static Pair<TreeState,TreeState> readStatePages( PageCursor cursor, long pageIdA, long pageIdB ) throws IOException
     {
-        TreeState stateA;
-        TreeState stateB;
-
-        // 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 );
+        TreeState stateA = readStatePage( cursor, pageIdA );
+        TreeState stateB = readStatePage( cursor, pageIdB );
         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.
      * @return newest (w/ regards to {@link TreeState#stableGeneration()}) {@link TreeState#isValid() valid}