From da4b6ddc0fdd3d6c89193769bc722931c7af8836 Mon Sep 17 00:00:00 2001 From: Anton Persson Date: Thu, 30 Mar 2017 11:29:04 +0200 Subject: [PATCH] Shuffle recovery test load order ... to better model the fact that order between transactions can be different during recovery compared to when they where first applied, as long as there are no dependencies (locks) between them. --- .../internal/gbptree/GBPTreeRecoveryIT.java | 145 ++++++++++++++---- 1 file changed, 112 insertions(+), 33 deletions(-) diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeRecoveryIT.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeRecoveryIT.java index 451d0ed5b296..b8c6eda2d493 100644 --- a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeRecoveryIT.java +++ b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeRecoveryIT.java @@ -26,10 +26,13 @@ import java.io.File; import java.io.IOException; -import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeMap; +import java.util.TreeSet; import java.util.stream.Collectors; import org.neo4j.cursor.RawCursor; @@ -52,7 +55,7 @@ public class GBPTreeRecoveryIT { private static final int PAGE_SIZE = 256; - private static final Action CHECKPOINT = new CheckpointAction(); + private final Action CHECKPOINT = new CheckpointAction(); private final EphemeralFileSystemRule fs = new EphemeralFileSystemRule(); private final TestDirectory directory = TestDirectory.testDirectory( getClass(), fs.get() ); @@ -160,6 +163,7 @@ private void doShouldRecoverFromAnything( boolean replayRecoveryExactlyFromCheck // a tree which has had random updates and checkpoints in it, load generated with specific seed File file = directory.file( "index" ); List load = generateLoad(); + List shuffledLoad = randomCausalAwareShuffle( load ); int lastCheckPointIndex = indexOfLastCheckpoint( load ); { @@ -172,15 +176,15 @@ private void doShouldRecoverFromAnything( boolean replayRecoveryExactlyFromCheck PageCache pageCache = createPageCache(); GBPTree index = createIndex( pageCache, file ); // Execute all actions up to and including last checkpoint ... - execute( load.subList( 0, lastCheckPointIndex + 1 ), index ); + execute( shuffledLoad.subList( 0, lastCheckPointIndex + 1 ), index ); // ... a random amount of the remaining "unsafe" actions ... - int numberOfRemainingActions = load.size() - lastCheckPointIndex - 1; + int numberOfRemainingActions = shuffledLoad.size() - lastCheckPointIndex - 1; int crashFlushIndex = lastCheckPointIndex + random.nextInt( numberOfRemainingActions ) + 1; - execute( load.subList( lastCheckPointIndex + 1, crashFlushIndex ), index ); + execute( shuffledLoad.subList( lastCheckPointIndex + 1, crashFlushIndex ), index ); // ... flush ... pageCache.flushAndForce(); // ... execute the remaining actions - execute( load.subList( crashFlushIndex, load.size() ), index ); + execute( shuffledLoad.subList( crashFlushIndex, shuffledLoad.size() ), index ); // ... and finally crash fs.snapshot( throwing( () -> { @@ -238,10 +242,53 @@ private void doShouldRecoverFromAnything( boolean replayRecoveryExactlyFromCheck } } + /** + * Shuffle actions without breaking causal dependencies, i.e. without affecting the end result + * of the data ending up in the tree. Checkpoints cannot move. + * + * On an integration level with neo4j, this is done because of the nature of how concurrent transactions + * are applied in random order and recovery applies transactions in order of transaction id. + */ + private List randomCausalAwareShuffle( List actions ) + { + Action[] arrayToShuffle = actions.toArray( new Action[actions.size()] ); + int size = arrayToShuffle.length; + int numberOfActionsToShuffle = random.nextInt( size / 2 ); + + for ( int i = 0; i < numberOfActionsToShuffle; i++ ) + { + int actionIndexToMove = random.nextInt( size ); + int stride = random.nextBoolean() ? 1 : -1; + int maxNumberOfSteps = random.nextInt( 10 ) + 1; + + for ( int steps = 0; steps < maxNumberOfSteps; steps++ ) + { + Action actionToMove = arrayToShuffle[actionIndexToMove]; + int actionIndexToSwap = actionIndexToMove + stride; + if ( actionIndexToSwap < 0 || actionIndexToSwap >= size ) + { + break; + } + Action actionToSwap = arrayToShuffle[actionIndexToSwap]; + + if ( actionToMove.hasCausalDependencyWith( actionToSwap ) ) + { + break; + } + + arrayToShuffle[actionIndexToMove] = actionToSwap; + arrayToShuffle[actionIndexToSwap] = actionToMove; + + actionIndexToMove = actionIndexToSwap; + } + } + return Arrays.asList( arrayToShuffle ); + } + private List recoveryActions( List load, int fromIndex ) { return load.subList( fromIndex, load.size() ).stream() - .filter( Action::isRecoverable ) + .filter( action -> !action.isCheckpoint() ) .collect( Collectors.toList() ); } @@ -304,7 +351,7 @@ private static int indexOfLastCheckpoint( List actions ) int lastCheckpoint = -1; for ( Action action : actions ) { - if ( action == CHECKPOINT ) + if ( action.isCheckpoint() ) { lastCheckpoint = i; } @@ -315,7 +362,7 @@ private static int indexOfLastCheckpoint( List actions ) private List generateLoad() { - List actions = new ArrayList<>(); + List actions = new LinkedList<>(); int count = random.intBetween( 300, 1_000 ); boolean hasCheckPoint = false; for ( int i = 0; i < count; i++ ) @@ -389,53 +436,68 @@ private PageCache createPageCache() return pageCacheRule.getPageCache( fs.get() ); } - interface Action + abstract class Action { - void execute( GBPTree index ) throws IOException; - - long[] data(); - - boolean isRecoverable(); - } + long[] data; + Set allKeys; - abstract class RecoverableAction implements Action - { - final long[] data; - - RecoverableAction( long[] data ) + Action( long[] data ) { this.data = data; + this.allKeys = keySet( data ); } - @Override - public long[] data() + long[] data() { return data; } - @Override - public boolean isRecoverable() + abstract void execute( GBPTree index ) throws IOException; + + abstract boolean isCheckpoint(); + + abstract boolean hasCausalDependencyWith( Action other ); + + private Set keySet( long[] data ) { - return true; + Set keys = new TreeSet<>(); + for ( int i = 0; i < data.length; i += 2 ) + { + keys.add( data[i] ); + } + return keys; } } - abstract static class NonRecoverableAction implements Action + abstract class DataAction extends Action { + DataAction( long[] data ) + { + super( data ); + } + @Override - public boolean isRecoverable() + boolean isCheckpoint() { return false; } @Override - public long[] data() + public boolean hasCausalDependencyWith( Action other ) { - return null; + if ( other.isCheckpoint() ) + { + return true; + } + + Set intersection = new TreeSet<>( allKeys ); + intersection.retainAll( other.allKeys ); + + return !intersection.isEmpty(); } } - class InsertAction extends RecoverableAction + class InsertAction extends DataAction { InsertAction( long[] data ) { @@ -457,7 +519,7 @@ public void execute( GBPTree index ) throws IOException } } - class RemoveAction extends RecoverableAction + class RemoveAction extends DataAction { RemoveAction( long[] data ) { @@ -479,12 +541,29 @@ public void execute( GBPTree index ) throws IOException } } - static class CheckpointAction extends NonRecoverableAction + class CheckpointAction extends Action { + CheckpointAction() + { + super( new long[0] ); + } + @Override public void execute( GBPTree index ) throws IOException { index.checkpoint( unlimited() ); } + + @Override + boolean isCheckpoint() + { + return true; + } + + @Override + public boolean hasCausalDependencyWith( Action other ) + { + return true; + } } }