From 953200021a90752efbfe47c5df98f6a414de57a2 Mon Sep 17 00:00:00 2001
From: Anton Persson <anton.persson@neotechnology.com>
Date: Thu, 4 Apr 2019 14:50:08 +0200
Subject: [PATCH] Turn trace tracking of before seeking concurrently

Trace tracking PageCursorTracer used in test is not thread safe but was
accessed concurrently from the two different seeker threads. We now
turn trace tracking off before starting to seek. We only need the trace
tracking to figure out id for leaf nodes.

It is not strictly necessary to turn trace tracking of in the single
threaded case, but we do that to not store a bunch of ids in trace list
that we don't need.
---
 .../index/internal/gbptree/GBPTreeTest.java   | 42 +++++++++++++------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java
index 67b9bfca0dc24..532e25dad960b 100644
--- a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java
+++ b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java
@@ -19,6 +19,7 @@
  */
 package org.neo4j.index.internal.gbptree;
 
+import org.apache.commons.lang3.mutable.MutableBoolean;
 import org.apache.commons.lang3.mutable.MutableLong;
 import org.apache.commons.lang3.tuple.Pair;
 import org.hamcrest.CoreMatchers;
@@ -1631,7 +1632,9 @@ public void mustThrowIfStuckInInfiniteRootCatchup() throws IOException
         // with an exception.
 
         List<Long> trace = new ArrayList<>();
-        PageCache pageCache = pageCacheWithTrace( trace );
+        MutableBoolean onOffSwitch = new MutableBoolean( true );
+        PageCursorTracer pageCursorTracer = trackingPageCursorTracer( trace, onOffSwitch );
+        PageCache pageCache = pageCacheWithTrace( pageCursorTracer );
 
         // Build a tree with root and two children.
         try ( GBPTree<MutableLong,MutableLong> tree = index( pageCache ).build() )
@@ -1640,6 +1643,9 @@ public void mustThrowIfStuckInInfiniteRootCatchup() throws IOException
             treeWithRootSplit( trace, tree );
             long corruptChild = trace.get( 1 );
 
+            // We are not interested in further trace tracking
+            onOffSwitch.setFalse();
+
             // Corrupt the child
             corruptTheChild( pageCache, corruptChild );
 
@@ -1666,7 +1672,9 @@ public void mustThrowIfStuckInInfiniteRootCatchupMultipleConcurrentSeekers() thr
         try
         {
             List<Long> trace = new ArrayList<>();
-            PageCache pageCache = pageCacheWithTrace( trace );
+            MutableBoolean onOffSwitch = new MutableBoolean( true );
+            PageCursorTracer pageCursorTracer = trackingPageCursorTracer( trace, onOffSwitch );
+            PageCache pageCache = pageCacheWithTrace( pageCursorTracer );
 
             // Build a tree with root and two children.
             try ( GBPTree<MutableLong,MutableLong> tree = index( pageCache ).build() )
@@ -1676,6 +1684,9 @@ public void mustThrowIfStuckInInfiniteRootCatchupMultipleConcurrentSeekers() thr
                 long leftChild = trace.get( 1 );
                 long rightChild = trace.get( 2 );
 
+                // Stop trace tracking because we will soon start pinning pages from different threads
+                onOffSwitch.setFalse();
+
                 // Corrupt the child
                 corruptTheChild( pageCache, leftChild );
                 corruptTheChild( pageCache, rightChild );
@@ -1716,6 +1727,22 @@ public void mustThrowIfStuckInInfiniteRootCatchupMultipleConcurrentSeekers() thr
         }
     }
 
+    private DefaultPageCursorTracer trackingPageCursorTracer( List<Long> trace, MutableBoolean onOffSwitch )
+    {
+        return new DefaultPageCursorTracer()
+        {
+            @Override
+            public PinEvent beginPin( boolean writeLock, long filePageId, PageSwapper swapper )
+            {
+                if ( onOffSwitch.isTrue() )
+                {
+                    trace.add( filePageId );
+                }
+                return super.beginPin( writeLock, filePageId, swapper );
+            }
+        };
+    }
+
     private void assertFutureFailsWithTreeInconsistencyException( Future<Object> execute1 ) throws InterruptedException
     {
         try
@@ -1780,18 +1807,9 @@ private void treeWithRootSplit( List<Long> trace, GBPTree<MutableLong,MutableLon
         }
     }
 
-    private PageCache pageCacheWithTrace( List<Long> trace )
+    private PageCache pageCacheWithTrace( PageCursorTracer pageCursorTracer  )
     {
         // A page cache tracer that we can use to see when tree has seen enough updates and to figure out on which page the child sits.Trace( trace );
-        PageCursorTracer pageCursorTracer = new DefaultPageCursorTracer()
-        {
-            @Override
-            public PinEvent beginPin( boolean writeLock, long filePageId, PageSwapper swapper )
-            {
-                trace.add( filePageId );
-                return super.beginPin( writeLock, filePageId, swapper );
-            }
-        };
         PageCursorTracerSupplier pageCursorTracerSupplier = () -> pageCursorTracer;
         return createPageCache( DEFAULT_PAGE_SIZE, pageCursorTracerSupplier );
     }