Skip to content

Commit

Permalink
Turn trace tracking of before seeking concurrently
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
burqen committed Apr 9, 2019
1 parent b8f70c3 commit 9532000
Showing 1 changed file with 30 additions and 12 deletions.
Expand Up @@ -19,6 +19,7 @@
*/ */
package org.neo4j.index.internal.gbptree; 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.mutable.MutableLong;
import org.apache.commons.lang3.tuple.Pair; import org.apache.commons.lang3.tuple.Pair;
import org.hamcrest.CoreMatchers; import org.hamcrest.CoreMatchers;
Expand Down Expand Up @@ -1631,7 +1632,9 @@ public void mustThrowIfStuckInInfiniteRootCatchup() throws IOException
// with an exception. // with an exception.


List<Long> trace = new ArrayList<>(); 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. // Build a tree with root and two children.
try ( GBPTree<MutableLong,MutableLong> tree = index( pageCache ).build() ) try ( GBPTree<MutableLong,MutableLong> tree = index( pageCache ).build() )
Expand All @@ -1640,6 +1643,9 @@ public void mustThrowIfStuckInInfiniteRootCatchup() throws IOException
treeWithRootSplit( trace, tree ); treeWithRootSplit( trace, tree );
long corruptChild = trace.get( 1 ); long corruptChild = trace.get( 1 );


// We are not interested in further trace tracking
onOffSwitch.setFalse();

// Corrupt the child // Corrupt the child
corruptTheChild( pageCache, corruptChild ); corruptTheChild( pageCache, corruptChild );


Expand All @@ -1666,7 +1672,9 @@ public void mustThrowIfStuckInInfiniteRootCatchupMultipleConcurrentSeekers() thr
try try
{ {
List<Long> trace = new ArrayList<>(); 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. // Build a tree with root and two children.
try ( GBPTree<MutableLong,MutableLong> tree = index( pageCache ).build() ) try ( GBPTree<MutableLong,MutableLong> tree = index( pageCache ).build() )
Expand All @@ -1676,6 +1684,9 @@ public void mustThrowIfStuckInInfiniteRootCatchupMultipleConcurrentSeekers() thr
long leftChild = trace.get( 1 ); long leftChild = trace.get( 1 );
long rightChild = trace.get( 2 ); long rightChild = trace.get( 2 );


// Stop trace tracking because we will soon start pinning pages from different threads
onOffSwitch.setFalse();

// Corrupt the child // Corrupt the child
corruptTheChild( pageCache, leftChild ); corruptTheChild( pageCache, leftChild );
corruptTheChild( pageCache, rightChild ); corruptTheChild( pageCache, rightChild );
Expand Down Expand Up @@ -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 private void assertFutureFailsWithTreeInconsistencyException( Future<Object> execute1 ) throws InterruptedException
{ {
try try
Expand Down Expand Up @@ -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 ); // 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; PageCursorTracerSupplier pageCursorTracerSupplier = () -> pageCursorTracer;
return createPageCache( DEFAULT_PAGE_SIZE, pageCursorTracerSupplier ); return createPageCache( DEFAULT_PAGE_SIZE, pageCursorTracerSupplier );
} }
Expand Down

0 comments on commit 9532000

Please sign in to comment.