Merge pull request neo4j#11045 from burqen/3.4-formatcompatibilitytest

More rigid FormatCompatibilityTest

community/index/src/test/java/org/neo4j/index/internal/gbptree/FormatCompatibilityTest.java

@@ -34,6 +34,7 @@
 import java.io.IOException;
 import java.net.URL;
 import java.nio.file.Files;
+import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.List;
 import java.util.zip.ZipEntry;
@@ -44,19 +45,18 @@
 import org.neo4j.io.pagecache.IOLimiter;
 import org.neo4j.io.pagecache.PageCache;
 import org.neo4j.test.rule.PageCacheRule;
+import org.neo4j.test.rule.RandomRule;
 import org.neo4j.test.rule.TestDirectory;
 import org.neo4j.test.rule.fs.DefaultFileSystemRule;
 
 import static java.lang.String.format;
 import static java.util.Arrays.asList;
-
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-
 import static org.neo4j.index.internal.gbptree.SimpleLongLayout.longLayout;
 import static org.neo4j.test.rule.PageCacheRule.config;
 
@@ -70,7 +70,7 @@
 public class FormatCompatibilityTest
 {
     private static final String STORE = "store";
-    private static final int KEY_COUNT = 10_000;
+    private static final int INITIAL_KEY_COUNT = 10_000;
     private static final String CURRENT_FIXED_SIZE_FORMAT_ZIP = "current-format.zip";
     private static final String CURRENT_DYNAMIC_SIZE_FORMAT_ZIP = "current-dynamic-format.zip";
 
@@ -90,13 +90,21 @@ public static List<Object[]> data()
     private final TestDirectory directory = TestDirectory.testDirectory( getClass() );
     private final PageCacheRule pageCacheRule = new PageCacheRule( config().withInconsistentReads( false ) );
     private final DefaultFileSystemRule fsRule = new DefaultFileSystemRule();
+    private final RandomRule random = new RandomRule();
 
     @Rule
-    public final RuleChain chain = RuleChain.outerRule( fsRule ).around( directory ).around( pageCacheRule );
+    public final RuleChain chain = RuleChain.outerRule( random ).around( fsRule ).around( directory ).around( pageCacheRule );
 
     @Test
     public void shouldDetectFormatChange() throws Throwable
     {
+        List<Long> initialKeys = initialKeys();
+        List<Long> keysToAdd = keysToAdd();
+        List<Long> allKeys = new ArrayList<>();
+        allKeys.addAll( initialKeys );
+        allKeys.addAll( keysToAdd );
+        allKeys.sort( Long::compare );
+
         // GIVEN stored tree
         File storeFile = directory.file( STORE );
         try
@@ -111,32 +119,91 @@ public void shouldDetectFormatChange() throws Throwable
         }
         assertTrue( zipName + " seems to be missing from resources directory", fsRule.get().fileExists( storeFile ) );
 
-        // WHEN reading from the tree
-        // THEN everything should work, otherwise there has likely been a format change
         PageCache pageCache = pageCacheRule.getPageCache( fsRule.get() );
         try ( GBPTree<MutableLong,MutableLong> tree =
                 new GBPTreeBuilder<>( pageCache, storeFile, layout ).build() )
         {
             try
             {
-                tree.consistencyCheck();
-                try ( RawCursor<Hit<MutableLong,MutableLong>,IOException> cursor =
-                        tree.seek( new MutableLong( 0 ), new MutableLong( KEY_COUNT ) ) )
                 {
-                    for ( long expectedKey = 0; cursor.next(); expectedKey++ )
+                    // WHEN reading from the tree
+                    // THEN initial keys should be there
+                    tree.consistencyCheck();
+                    try ( RawCursor<Hit<MutableLong,MutableLong>,IOException> cursor =
+                                  tree.seek( layout.key( 0 ), layout.key( Long.MAX_VALUE ) ) )
+                    {
+                        for ( Long expectedKey : initialKeys )
+                        {
+                            assertHit( cursor, expectedKey );
+                        }
+                        assertFalse( cursor.next() );
+                    }
+                }
+
+                {
+                    // WHEN writing more to the tree
+                    // THEN we should not see any format conflicts
+                    try ( Writer<MutableLong,MutableLong> writer = tree.writer() )
+                    {
+                        while ( keysToAdd.size() > 0 )
+                        {
+                            int next = random.nextInt( keysToAdd.size() );
+                            put( writer, keysToAdd.get( next ) );
+                            keysToAdd.remove( next );
+                        }
+                    }
+                }
+
+                {
+                    // WHEN reading from the tree again
+                    // THEN all keys including newly added should be there
+                    tree.consistencyCheck();
+                    try ( RawCursor<Hit<MutableLong,MutableLong>,IOException> cursor =
+                                  tree.seek( layout.key( 0 ), layout.key( 2 * INITIAL_KEY_COUNT ) ) )
+                    {
+                        for ( Long expectedKey : allKeys )
+                        {
+                            assertHit( cursor, expectedKey );
+                        }
+                        assertFalse( cursor.next() );
+                    }
+                }
+
+                {
+                    // WHEN randomly removing half of tree content
+                    // THEN we should not see any format conflicts
+                    try ( Writer<MutableLong,MutableLong> writer = tree.writer() )
+                    {
+                        int size = allKeys.size();
+                        while ( allKeys.size() > size / 2 )
+                        {
+                            int next = random.nextInt( allKeys.size() );
+                            MutableLong key = layout.key( allKeys.get( next ) );
+                            writer.remove( key );
+                            allKeys.remove( next );
+                        }
+                    }
+                }
+
+                {
+                    // WHEN reading from the tree after remove
+                    // THEN we should see everything that is left in the tree
+                    tree.consistencyCheck();
+                    try ( RawCursor<Hit<MutableLong,MutableLong>,IOException> cursor =
+                                  tree.seek( layout.key( 0 ), layout.key( 2 * INITIAL_KEY_COUNT ) ) )
                     {
-                        Hit<MutableLong,MutableLong> hit = cursor.get();
-                        assertEquals( expectedKey, hit.key().longValue() );
-                        assertEquals( value( expectedKey ), hit.value().longValue() );
+                        for ( Long expectedKey : allKeys )
+                        {
+                            assertHit( cursor, expectedKey );
+                        }
+                        assertFalse( cursor.next() );
                     }
-                    assertFalse( cursor.next() );
                 }
             }
             catch ( Throwable t )
             {
-                throw new AssertionError( format(
-                        "If this is the single failing test in this component then this failure is a strong indication that format " +
-                                "has changed without also incrementing TreeNode version(s). Please make necessary format version changes." ), t );
+                throw new AssertionError( "If this is the single failing test in this component then this failure is a strong indication that format " +
+                        "has changed without also incrementing TreeNode version(s). Please make necessary format version changes.", t );
             }
         }
         catch ( MetadataMismatchException e )
@@ -154,11 +221,11 @@ public void shouldDetectFormatChange() throws Throwable
     private void tellDeveloperToCommitThisFormatVersion()
     {
         fail( format( "This is merely a notification to developer. Format has changed and its version has also " +
-                "been properly incremented. A tree with this new format has been generated and should be committed. " +
-                "Please move:%n  %s%ninto %n  %s, %nreplacing the existing file there",
+                        "been properly incremented. A tree with this new format has been generated and should be committed. " +
+                        "Please move:%n  %s%ninto %n  %s, %nreplacing the existing file there",
                 directory.file( zipName ),
                 "<index-module>" + pathify( ".src.test.resources." ) +
-                pathify( getClass().getPackage().getName() + "." ) + zipName ) );
+                        pathify( getClass().getPackage().getName() + "." ) + zipName ) );
     }
 
     private static String pathify( String name )
@@ -186,26 +253,19 @@ private void unzipTo( File storeFile ) throws IOException
 
     private void createAndZipTree( File storeFile ) throws IOException
     {
+        List<Long> initialKeys = initialKeys();
         PageCache pageCache = pageCacheRule.getPageCache( fsRule.get() );
         try ( GBPTree<MutableLong,MutableLong> tree =
-                new GBPTreeBuilder<>( pageCache, storeFile, layout ).build() )
+                      new GBPTreeBuilder<>( pageCache, storeFile, layout ).build() )
         {
-            MutableLong insertKey = new MutableLong();
-            MutableLong insertValue = new MutableLong();
-            int batchSize = KEY_COUNT / 10;
-            for ( int i = 0, key = 0; i < 10; i++ )
+            try ( Writer<MutableLong,MutableLong> writer = tree.writer() )
             {
-                try ( Writer<MutableLong,MutableLong> writer = tree.writer() )
+                for ( Long key : initialKeys )
                 {
-                    for ( int j = 0; j < batchSize; j++, key++ )
-                    {
-                        insertKey.setValue( key );
-                        insertValue.setValue( value( key ) );
-                        writer.put( insertKey, insertValue );
-                    }
+                    put( writer, key );
                 }
-                tree.checkpoint( IOLimiter.unlimited() );
             }
+            tree.checkpoint( IOLimiter.unlimited() );
         }
         zip( storeFile );
     }
@@ -227,4 +287,39 @@ private File zip( File toZip ) throws IOException
         }
         return targetFile;
     }
+
+    private List<Long> initialKeys()
+    {
+        List<Long> initialKeys = new ArrayList<>();
+        for ( long i = 0, key = 0; i < INITIAL_KEY_COUNT; i++, key += 2 )
+        {
+            initialKeys.add( key );
+        }
+        return initialKeys;
+    }
+
+    private List<Long> keysToAdd()
+    {
+        List<Long> keysToAdd = new ArrayList<>();
+        for ( long i = 0, key = 1; i < INITIAL_KEY_COUNT; i++, key += 2 )
+        {
+            keysToAdd.add( key );
+        }
+        return keysToAdd;
+    }
+
+    private void assertHit( RawCursor<Hit<MutableLong,MutableLong>,IOException> cursor, Long expectedKey ) throws IOException
+    {
+        assert cursor.next() : "Had no next when expecting key " + expectedKey;
+        Hit<MutableLong,MutableLong> hit = cursor.get();
+        assertEquals( expectedKey.longValue(), hit.key().longValue() );
+        assertEquals( value( expectedKey ), hit.value().longValue() );
+    }
+
+    private void put( Writer<MutableLong,MutableLong> writer, long key ) throws IOException
+    {
+        MutableLong insertKey = layout.key( key );
+        MutableLong insertValue = layout.value( value( key ) );
+        writer.put( insertKey, insertValue );
+    }
 }