Move goTo from InternalTreeLogic to TreeNode

neo4j · Dec 4, 2016 · 4805cbf · 4805cbf
1 parent b141504
commit 4805cbf
Show file tree

Hide file tree

Showing 5 changed files with 129 additions and 52 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
@@ -321,10 +321,7 @@ public RawCursor<Hit<KEY,VALUE>,IOException> seek( KEY fromInclusive, KEY toExcl
             {
                 PointerChecking.checkChildPointer( childId );
 
-                if ( !cursor.next( childId ) )
-                {
-                    throw new IllegalStateException( "Couldn't go to child " + childId );
-                }
+                bTreeNode.goTo( cursor, childId, stableGeneration, unstableGeneration );
             }
         }
         while ( isInternal && keyCount > 0 );
@@ -400,10 +397,7 @@ public IndexWriter<KEY,VALUE> writer( IndexWriter.Options options ) throws IOExc
 
     private void goToRoot( PageCursor cursor ) throws IOException
     {
-        if ( !cursor.next( rootId ) )
-        {
-            throw new IllegalStateException( "Could not go to root " + rootId );
-        }
+        bTreeNode.goTo( cursor, rootId, stableGeneration, unstableGeneration );
     }
 
     private void checkOutOfBounds( PageCursor cursor )

diff --git a/community/index/src/main/java/org/neo4j/index/gbptree/InternalTreeLogic.java b/community/index/src/main/java/org/neo4j/index/gbptree/InternalTreeLogic.java
@@ -151,11 +151,11 @@ void insert( PageCursor cursor, StructurePropagation<KEY> structurePropagation,
         long childId = bTreeNode.childAt( cursor, pos, stableGeneration, unstableGeneration );
         PointerChecking.checkChildPointer( childId );
 
-        goTo( cursor, childId, stableGeneration, unstableGeneration );
+        bTreeNode.goTo( cursor, childId, stableGeneration, unstableGeneration );
 
         insert( cursor, structurePropagation, key, value, valueMerger, options, stableGeneration, unstableGeneration );
 
-        goTo( cursor, currentId, stableGeneration, unstableGeneration );
+        bTreeNode.goTo( cursor, currentId, stableGeneration, unstableGeneration );
 
         if ( structurePropagation.hasNewGen )
         {
@@ -252,7 +252,7 @@ private void splitInternal( PageCursor cursor, StructurePropagation<KEY> structu
 
         {   // Update new right
             // NOTE: don't include middle
-            goTo( cursor, newRight, stableGeneration, unstableGeneration );
+            bTreeNode.goTo( cursor, newRight, stableGeneration, unstableGeneration );
             bTreeNode.initializeInternal( cursor, stableGeneration, unstableGeneration );
             bTreeNode.setRightSibling( cursor, oldRight, stableGeneration, unstableGeneration );
             bTreeNode.setLeftSibling( cursor, current, stableGeneration, unstableGeneration );
@@ -272,13 +272,13 @@ private void splitInternal( PageCursor cursor, StructurePropagation<KEY> structu
         // Update old right with new left sibling (newRight)
         if ( oldRight != TreeNode.NO_NODE_FLAG )
         {
-            goTo( cursor, oldRight, stableGeneration, unstableGeneration );
+            bTreeNode.goTo( cursor, oldRight, stableGeneration, unstableGeneration );
             bTreeNode.setLeftSibling( cursor, newRight, stableGeneration, unstableGeneration );
         }
 
         // Update left node
         // Move cursor back to left
-        goTo( cursor, fullNode, stableGeneration, unstableGeneration );
+        bTreeNode.goTo( cursor, fullNode, stableGeneration, unstableGeneration );
         bTreeNode.setKeyCount( cursor, middlePos );
         if ( pos < middlePos )
         {
@@ -363,7 +363,7 @@ private void insertInLeaf( PageCursor cursor, StructurePropagation<KEY> structur
      * @param newValue value to be inserted (in association with key)
      * @param keyCount number of keys in this leaf (it was already read anyway)
      * @param options options for this insert
-     * @throws IOException if cursor.next( newRight ) fails
+     * @throws IOException on cursor failure
      */
     private void splitLeaf( PageCursor cursor, StructurePropagation<KEY> structurePropagation,
             KEY newKey, VALUE newValue, int keyCount, IndexWriter.Options options,
@@ -460,7 +460,7 @@ private void splitLeaf( PageCursor cursor, StructurePropagation<KEY> structurePr
         }
 
         {   // Update new right
-            goTo( cursor, newRight, stableGeneration, unstableGeneration );
+            bTreeNode.goTo( cursor, newRight, stableGeneration, unstableGeneration );
             bTreeNode.initializeLeaf( cursor, stableGeneration, unstableGeneration );
             bTreeNode.setRightSibling( cursor, oldRight, stableGeneration, unstableGeneration );
             bTreeNode.setLeftSibling( cursor, current, stableGeneration, unstableGeneration );
@@ -472,12 +472,12 @@ private void splitLeaf( PageCursor cursor, StructurePropagation<KEY> structurePr
         // Update old right with new left sibling (newRight)
         if ( oldRight != TreeNode.NO_NODE_FLAG )
         {
-            goTo( cursor, oldRight, stableGeneration, unstableGeneration );
+            bTreeNode.goTo( cursor, oldRight, stableGeneration, unstableGeneration );
             bTreeNode.setLeftSibling( cursor, newRight, stableGeneration, unstableGeneration );
         }
 
         // Update left child
-        goTo( cursor, current, stableGeneration, unstableGeneration );
+        bTreeNode.goTo( cursor, current, stableGeneration, unstableGeneration );
         bTreeNode.setKeyCount( cursor, middlePos );
         // If pos < middle. Write shifted values to left node. Else, don't write anything.
         if ( pos < middlePos )
@@ -529,11 +529,11 @@ VALUE remove( PageCursor cursor, StructurePropagation<KEY> structurePropagation,
 
         long currentId = cursor.getCurrentPageId();
         long childId = bTreeNode.childAt( cursor, pos, stableGeneration, unstableGeneration );
-        goTo( cursor, childId, stableGeneration, unstableGeneration );
+        bTreeNode.goTo( cursor, childId, stableGeneration, unstableGeneration );
 
         VALUE result = remove( cursor, structurePropagation, key, into, stableGeneration, unstableGeneration );
 
-        goTo( cursor, currentId, stableGeneration, unstableGeneration );
+        bTreeNode.goTo( cursor, currentId, stableGeneration, unstableGeneration );
         if ( structurePropagation.hasNewGen )
         {
             structurePropagation.hasNewGen = false;
@@ -643,40 +643,20 @@ private void createUnstableVersionIfNeeded( PageCursor cursor, StructurePropagat
         long rightSibling = bTreeNode.rightSibling( cursor, stableGeneration, unstableGeneration );
         if ( leftSibling != TreeNode.NO_NODE_FLAG )
         {
-            goTo( cursor, leftSibling, stableGeneration, unstableGeneration );
+            bTreeNode.goTo( cursor, leftSibling, stableGeneration, unstableGeneration );
             bTreeNode.setRightSibling( cursor, newGenId, stableGeneration, unstableGeneration );
         }
         if ( rightSibling != TreeNode.NO_NODE_FLAG )
         {
-            goTo( cursor, rightSibling, stableGeneration, unstableGeneration );
+            bTreeNode.goTo( cursor, rightSibling, stableGeneration, unstableGeneration );
             bTreeNode.setLeftSibling( cursor, newGenId, stableGeneration, unstableGeneration );
         }
 
         // Leave cursor at new tree node
-        goTo( cursor, newGenId, stableGeneration, unstableGeneration );
+        bTreeNode.goTo( cursor, newGenId, stableGeneration, unstableGeneration );
 
         // Propagate structure change
         structurePropagation.hasNewGen = true;
         structurePropagation.left = newGenId;
     }
-
-    private void goTo( PageCursor cursor, long childId, long stableGeneration, long unstableGeneration )
-            throws IOException
-    {
-        if ( !cursor.next( childId ) )
-        {
-            throw new IllegalStateException( "Could not go to " + childId );
-        }
-        verifyGen( cursor, stableGeneration, unstableGeneration );
-    }
-
-    private void verifyGen( PageCursor cursor, long stableGeneration, long unstableGeneration )
-    {
-        long gen = bTreeNode.gen( cursor );
-        if ( ( gen > stableGeneration && gen < unstableGeneration ) || gen > unstableGeneration )
-        {
-            throw new IllegalStateException( "Reached a node with generation=" + gen +
-                    ", stableGeneration=" + stableGeneration + ", unstableGeneration=" + unstableGeneration );
-        }
-    }
 }
diff --git a/community/index/src/main/java/org/neo4j/index/gbptree/SeekCursor.java b/community/index/src/main/java/org/neo4j/index/gbptree/SeekCursor.java
@@ -139,12 +139,9 @@ public boolean next() throws IOException
             {
                 if ( bTreeNode.isNode( rightSibling ) )
                 {
-                    if ( !cursor.next( rightSibling ) )
-                    {
-                        // TODO: Check if rightSibling is within expected range before calling next.
-                        // TODO: Possibly by getting highest expected from IdProvider
-                        throw new IllegalStateException( "Could not go to right sibling " + rightSibling );
-                    }
+                    // TODO: Check if rightSibling is within expected range before calling next.
+                    // TODO: Possibly by getting highest expected from IdProvider
+                    bTreeNode.goTo( cursor, rightSibling, stableGeneration, unstableGeneration );
                     pos = -1;
                     reread = true;
                     continue; // in the outer loop, with the position reset to the beginning of the right sibling

diff --git a/community/index/src/main/java/org/neo4j/index/gbptree/TreeNode.java b/community/index/src/main/java/org/neo4j/index/gbptree/TreeNode.java
@@ -19,6 +19,7 @@
  */
 package org.neo4j.index.gbptree;
 
+import java.io.IOException;
 import java.util.Comparator;
 import java.util.function.Consumer;
 
@@ -370,6 +371,26 @@ int readChildrenWithInsertRecordInPosition( PageCursor cursor, Consumer<PageCurs
                 childSize(), childOffset( 0 ), into );
     }
 
+    void goTo( PageCursor cursor, long childId, long stableGeneration, long unstableGeneration )
+            throws IOException
+    {
+        if ( !cursor.next( childId ) )
+        {
+            throw new IllegalStateException( "Could not go to " + childId );
+        }
+        verifyGen( cursor, stableGeneration, unstableGeneration );
+    }
+
+    private void verifyGen( PageCursor cursor, long stableGeneration, long unstableGeneration )
+    {
+        long gen = gen( cursor );
+        if ( ( gen > stableGeneration && gen < unstableGeneration ) || gen > unstableGeneration )
+        {
+            throw new IllegalStateException( "Reached a node with generation=" + gen +
+                    ", stableGeneration=" + stableGeneration + ", unstableGeneration=" + unstableGeneration );
+        }
+    }
+
     /**
      * Leaves cursor on same page as when called. No guarantees on offset.
      * <p>

diff --git a/community/index/src/test/java/org/neo4j/index/gbptree/TreeNodeTest.java b/community/index/src/test/java/org/neo4j/index/gbptree/TreeNodeTest.java
@@ -20,34 +20,43 @@
 package org.neo4j.index.gbptree;
 
 import org.apache.commons.lang3.mutable.MutableLong;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 
+import java.io.IOException;
+
 import org.neo4j.io.pagecache.PageCursor;
 import org.neo4j.test.rule.RandomRule;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-
-import static org.neo4j.index.gbptree.ByteArrayPageCursor.wrap;
 import static org.neo4j.index.gbptree.TreeNode.NO_NODE_FLAG;
 
 public class TreeNodeTest
 {
     private static final int STABLE_GENERATION = 1;
-    private static final int UNSTABLE_GENERATION = 2;
+    private static final int CRASH_GENERATION = 2;
+    private static final int UNSTABLE_GENERATION = 3;
+    private static final int HIGH_GENERATION = 4;
 
     private static final int PAGE_SIZE = 512;
-    private final PageCursor cursor = wrap( new byte[PAGE_SIZE], 0, PAGE_SIZE );
+    private final PageCursor cursor = new PageAwareByteArrayCursor( PAGE_SIZE );
     private final Layout<MutableLong,MutableLong> layout = new SimpleLongLayout();
     private final TreeNode<MutableLong,MutableLong> node = new TreeNode<>( PAGE_SIZE, layout );
     private final byte[] tmp = new byte[PAGE_SIZE];
 
     @Rule
     public final RandomRule random = new RandomRule();
 
+    @Before
+    public void prepareCursor() throws IOException
+    {
+        cursor.next();
+    }
+
     @Test
     public void shouldInitializeLeaf() throws Exception
     {
@@ -410,6 +419,82 @@ public void shouldReadAndInsertChildren() throws Exception
         assertEquals( 3, node.childAt( cursor, 2, STABLE_GENERATION, UNSTABLE_GENERATION ) );
     }
 
+    @Test
+    public void shouldThrowWhenGoToCrashGenLeaf() throws Exception
+    {
+        // GIVEN
+        node.initializeLeaf( cursor, STABLE_GENERATION, CRASH_GENERATION );
+
+        try
+        {
+            // WHEN
+            node.goTo( cursor, cursor.getCurrentPageId(), STABLE_GENERATION, UNSTABLE_GENERATION );
+            fail( "Expected throw" );
+        }
+        catch ( IllegalStateException e )
+        {
+            // THEN
+            // Good
+        }
+    }
+
+    @Test
+    public void shouldThrowWhenGoToCrashGenInternal() throws Exception
+    {
+        // GIVEN
+        node.initializeInternal( cursor, STABLE_GENERATION, CRASH_GENERATION );
+
+        try
+        {
+            // WHEN
+            node.goTo( cursor, cursor.getCurrentPageId(), STABLE_GENERATION, UNSTABLE_GENERATION );
+            fail( "Expected throw" );
+        }
+        catch ( IllegalStateException e )
+        {
+            // THEN
+            // Good
+        }
+    }
+
+    @Test
+    public void shouldThrowWhenGoToHighGenLeaf() throws Exception
+    {
+        // GIVEN
+        node.initializeLeaf( cursor, STABLE_GENERATION, HIGH_GENERATION );
+
+        try
+        {
+            // WHEN
+            node.goTo( cursor, cursor.getCurrentPageId(), STABLE_GENERATION, UNSTABLE_GENERATION );
+            fail( "Expected throw" );
+        }
+        catch ( IllegalStateException e )
+        {
+            // THEN
+            // Good
+        }
+    }
+
+    @Test
+    public void shouldThrowWhenGoToHighGenInternal() throws Exception
+    {
+        // GIVEN
+        node.initializeInternal( cursor, STABLE_GENERATION, HIGH_GENERATION );
+
+        try
+        {
+            // WHEN
+            node.goTo( cursor, cursor.getCurrentPageId(), STABLE_GENERATION, UNSTABLE_GENERATION );
+            fail( "Expected throw" );
+        }
+        catch ( IllegalStateException e )
+        {
+            // THEN
+            // Good
+        }
+    }
+
     @Test
     public void shouldInsertAndRemoveRandomKeysAndValues() throws Exception
     {